New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Analysis Module: Implement Analysis Mutation to setup a new analysis #1516

Open

sauravsapkota wants to merge 8 commits into develop from feature/add-mutation-for-analysis

Contributor

sauravsapkota commented Jul 24, 2024 •

edited

Loading

Addresses

Analysis Module: Implement Analysis Mutation to setup a new analysis #1519

Changes

Convert REST API for Analysis CRUD to GraphQL mutation

This PR doesn't introduce any:

This PR contains valid:

tests
permission checks (tests here too)
translations

sauravsapkota force-pushed the feature/add-mutation-for-analysis branch from bbcda71 to 12ce2e0 Compare

July 24, 2024 09:55

sauravsapkota changed the title ~~WIP: Convert REST API for Analysis creation to GraphQL mutation~~ WIP: Convert REST API for Analysis CRUD to GraphQL mutation

sudan45 reviewed

View reviewed changes

apps/analysis/mutation.py Outdated Show resolved Hide resolved

sudan45 requested changes

View reviewed changes

apps/analysis/mutation.py Outdated Show resolved Hide resolved

sauravsapkota force-pushed the feature/add-mutation-for-analysis branch 2 times, most recently from 752e009 to b3ea86f Compare

July 25, 2024 06:34

sauravsapkota changed the title ~~WIP: Convert REST API for Analysis CRUD to GraphQL mutation~~ Convert REST API for Analysis CRUD to GraphQL mutation

sauravsapkota requested a review from sudan45

July 25, 2024 12:04

sauravsapkota marked this pull request as ready for review

July 25, 2024 12:04

sauravsapkota changed the title ~~Convert REST API for Analysis CRUD to GraphQL mutation~~ Analysis Module: Implement Analysis Mutation to setup a new analysis

sauravsapkota requested review from thenav56, Rup-Narayan-Rajbanshi, susilnem and a team

July 26, 2024 04:38

sudan45 requested changes

View reviewed changes

apps/analysis/mutation.py Outdated Show resolved Hide resolved

apps/analysis/mutation.py Outdated Show resolved Hide resolved

susilnem reviewed

View reviewed changes

apps/analysis/mutation.py Outdated Show resolved Hide resolved

susilnem reviewed

View reviewed changes

apps/analysis/mutation.py Show resolved Hide resolved

susilnem reviewed

View reviewed changes

apps/analysis/mutation.py Show resolved Hide resolved

susilnem requested changes

View reviewed changes

Member

susilnem left a comment

Let's verify.

Rup-Narayan-Rajbanshi reviewed

View reviewed changes

apps/analysis/mutation.py Outdated Show resolved Hide resolved

thenav56 requested changes

View reviewed changes

apps/analysis/mutation.py Show resolved Hide resolved

apps/analysis/mutation.py Outdated Show resolved Hide resolved

apps/analysis/serializers.py Show resolved Hide resolved

Rup-Narayan-Rajbanshi reviewed

View reviewed changes

apps/analysis/mutation.py Outdated Show resolved Hide resolved

sauravsapkota force-pushed the feature/add-mutation-for-analysis branch from db630f4 to d68451c Compare

July 29, 2024 11:46

sauravsapkota requested review from Rup-Narayan-Rajbanshi, sudan45, susilnem and thenav56

July 29, 2024 14:00

sauravsapkota changed the title ~~Analysis Module: Implement Analysis Mutation to setup a new analysis~~ WIP: Analysis Module: Implement Analysis Mutation to setup a new analysis

sauravsapkota force-pushed the feature/add-mutation-for-analysis branch from d68451c to b505c2c Compare

July 30, 2024 06:46

sauravsapkota changed the title ~~WIP: Analysis Module: Implement Analysis Mutation to setup a new analysis~~ Analysis Module: Implement Analysis Mutation to setup a new analysis

sauravsapkota force-pushed the feature/add-mutation-for-analysis branch from b505c2c to 2f66047 Compare

July 30, 2024 11:53

thenav56 requested changes

View reviewed changes

apps/analysis/serializers.py Outdated Show resolved Hide resolved

apps/analysis/serializers.py Outdated Show resolved Hide resolved

schema.graphql

+                startDate: Date
+                endDate: Date!
+                clonedFrom: ID
+                analysisPillar: [AnalysisPillarGqlInputType!]

Member

thenav56 Jul 31, 2024

@subinasr @AdityaKhatri @sudan45 @sauravsapkota

Looking at this behavior, should we continue with this or modify the logic?

Basically, right now, while editing Analysis, one can remove one or multiple Analysis Pillar with one click.
Let discuss more on this.

Member

AdityaKhatri Aug 2, 2024

@thenav56, I think this is fine for now. I'll create an issue to handle this behavior in the frontend.
the-deep/client#2981

Let me know what you think.

Contributor Author

sauravsapkota Aug 6, 2024 •

edited

Loading

Remove WritableNestedModelSerializer
Write Custom logic to save and update AnalysisPillar
@thenav56 @AdityaKhatri @subinasr @sudan45 @susilnem. Please check once.

apps/analysis/tests/test_mutations.py

+                          projectId=self.project_without_af.id,
+                      )
+                      self.force_login(self.non_member_user)

Member

thenav56 Jul 31, 2024

We need to use member_user?

apps/analysis/tests/test_mutations.py Outdated Show resolved Hide resolved

sauravsapkota force-pushed the feature/add-mutation-for-analysis branch from 2f66047 to 682f97e Compare

July 31, 2024 08:03

sauravsapkota requested a review from thenav56

July 31, 2024 08:20

thenav56 requested changes

View reviewed changes

Member

thenav56 left a comment

Almost done

apps/analysis/tests/test_mutations.py Show resolved Hide resolved

apps/analysis/tests/test_mutations.py Outdated

Comment on lines 1418 to 1429

+                      self.assertEqual(
+                          content['data']['project']['analysisCreate']['result']['title'],
+                          minput['analysisData']['title']
+                      )
+                      self.assertEqual(
+                          content['data']['project']['analysisCreate']['result']['teamLead']['id'],
+                          str(self.member_user.id)
+                      )
+                      self.assertEqual(
+                          content['data']['project']['analysisCreate']['result']['endDate'],
+                          str(minput['analysisData']['endDate'])
+                      )

Member

thenav56 Aug 6, 2024

Suggested change

      
                    self.assertEqual(
          
                        content['data']['project']['analysisCreate']['result']['title'],
          
                        minput['analysisData']['title']
          
                    )
          
                    self.assertEqual(
          
                        content['data']['project']['analysisCreate']['result']['teamLead']['id'],
          
                        str(self.member_user.id)
          
                    )
          
                    self.assertEqual(
          
                        content['data']['project']['analysisCreate']['result']['endDate'],
          
                        str(minput['analysisData']['endDate'])
          
                    )
          
                    analysis_resp_data = content['data']['project']['analysisCreate']['result']
          
                    self.assertEqual(analysis_resp_data['title'], minput['analysisData']['title'])
          
                    self.assertEqual(analysis_resp_data['teamLead']['id'], str(self.member_user.id))
          
                    self.assertEqual(analysis_resp_data['endDate'], str(minput['analysisData']['endDate']))

apps/analysis/tests/test_mutations.py Outdated

Comment on lines 1443 to 1452

+                      minput = dict(
+                          analysisData=dict(
+                              title='Test Analysis', teamLead=self.member_user.id, endDate='2020-01-01'
+                          ),
+                          projectId=self.project_with_af.id,
+                      )
+                      self.force_login(self.member_user)
+                      content = self.query_check(self.CREATE_MUTATION, variables=minput)
+                      self.assertIsNotNone(content['data']['project']['analysisCreate']['result']['id'])

Member

thenav56 Aug 6, 2024

Let's just use factory here.

apps/analysis/tests/test_mutations.py Outdated

Comment on lines 1492 to 1501

+                      minput = dict(
+                          analysisData=dict(
+                              title='Test Analysis', teamLead=self.member_user.id, endDate='2020-01-01'
+                          ),
+                          projectId=self.project_with_af.id,
+                      )
+                      self.force_login(self.member_user)
+                      content = self.query_check(self.CREATE_MUTATION, variables=minput)
+                      self.assertIsNotNone(content['data']['project']['analysisCreate']['result']['id'])

Member

thenav56 Aug 6, 2024

Let's just use factory here.

susilnem requested changes

View reviewed changes

Member

susilnem left a comment

Minor changes

apps/analysis/tests/test_mutations.py Show resolved Hide resolved

apps/analysis/tests/test_mutations.py Outdated Show resolved Hide resolved

sauravsapkota force-pushed the feature/add-mutation-for-analysis branch 2 times, most recently from 05d71a4 to cf8a770 Compare

August 16, 2024 11:22

Rup-Narayan-Rajbanshi reviewed

View reviewed changes

apps/analysis/serializers.py Outdated Show resolved Hide resolved

apps/analysis/serializers.py Show resolved Hide resolved

thenav56 force-pushed the feature/add-mutation-for-analysis branch from cf8a770 to 56f7199 Compare

August 22, 2024 05:13

sudan45 force-pushed the feature/add-mutation-for-analysis branch from 56f7199 to 9ec96c9 Compare

October 1, 2024 09:21

AdityaKhatri force-pushed the feature/add-mutation-for-analysis branch from 9ec96c9 to 355a5f5 Compare

October 16, 2024 09:45

sudan45 force-pushed the feature/add-mutation-for-analysis branch 2 times, most recently from e8a506e to 49a7bc8 Compare

October 18, 2024 09:15

AdityaKhatri mentioned this pull request

Add/analysis overview query the-deep/client#2970

Open

11 tasks

sauravsapkota and others added 7 commits

November 7, 2024 09:57


          Add Graphql mutation for Analysis CRUD

463019c


          Rename analyis field into analysis_id

709acce


          Add test cases for Analysis Mutation

81d1834


          Change filter genericscalartype into filtertype

39a7ef0


          Change filter genericscalartype into filtertype

d60a690


          Remove required False in analyisis pillar analyis

53e87c1


          Add analysis pillar delete mutation

a5f03bb

AdityaKhatri force-pushed the feature/add-mutation-for-analysis branch from cb86e92 to a5f03bb Compare

November 7, 2024 04:12


          Refactor

fd94b93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Rup-Narayan-Rajbanshi Rup-Narayan-Rajbanshi left review comments

AdityaKhatri AdityaKhatri left review comments

susilnem susilnem requested changes

thenav56 thenav56 requested changes

sudan45 Awaiting requested review from sudan45

Requested changes must be addressed to merge this pull request.

Labels

None yet