Add a requires_study_review field for DataAffiliateAgreements #509
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
amstilp
commented
Mar 15, 2024
•
amstilp
commented
- Add requires_study_review field to DataAffiliateAgreement
- Move the is_primary field to MemberAgreement and DataAffiliateAgreement, which makes it easier to clean other fields that depend on this field. This involves writing some custom migrations to populate is_primary
- Move the custom cleaning for is_primary, requires_study_review and additional_limitations to DataAffiliateAgreement.
- Update code and forms to handle the is_primary change
- Show requires_study_review indicators in tables and on detail pages, where appropriate.
Add tests in the DataAffiliateCreate view to verify that you can't create a DataAffiliate component with the additional_limitations or requires_study_review=True. These tests are failing - the clean method isn't working because it is essentially cleaning two models together, and the DataAffiliateAgreement form instance doesn't have the signed_agreement field set before the form/formset are saved.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #509 +/- ##
==========================================
+ Coverage 98.65% 98.66% +0.01%
==========================================
Files 286 291 +5
Lines 22454 22773 +319
==========================================
+ Hits 22151 22470 +319
Misses 303 303 ☔ View full report in Codecov by Sentry. |
This will make the forms easier to process, and also makes some sense because the NonDataAffiliate type does not differentiate between primary and components - they are all primary.
Move the additional_limitation and rqeuires_study_review Validation errors to the specific fields, not the NON_FIELD_ERRORS. This provides better behavior on the forms.
Include an indicator of whether requires_study_review is true on the CDSAWorkspace and DataAFfiliateAgreement detail pages.
With is_primary being moved to the agreement type classes, the detail pages needed to be fixed to show it properly.
Add a field to the DataAffiliateAgreementTable and the CDSAWorkspace Table to indicate whether study review is required.
This concept no longer exists, since NonDataAffiliateAgreement does not have an is_primary field. They are all primary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.