Skip to content
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.

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

CDSA invalidation #233

Merged
merged 54 commits into from
Sep 29, 2023
Merged

CDSA invalidation #233

merged 54 commits into from
Sep 29, 2023

Conversation

amstilp
Copy link
Contributor

@amstilp amstilp commented Aug 30, 2023

WIP

  • Add a new model to track AgreementMajorVersions.
  • Add detail views for AgreementVersions and AgreementMajorVersions.
  • Add a status field to SignedAgreements with choices Active, Lapsed, or Withdrawn.
  • Modify access audits for SignedAgreements and CDSAWorkspaces to take SignedAgreement status field into account.
  • Allow a user to invalidate an AgreementMajorVersion. This also sets the status of any associated SignedAgreements to Lapsed.
  • Only show records associated with Active SignedAgreements in the records tables.

Closes #195

This is to handle a page for agreement versions - it's a little
clearer.
This view shows the set of signed agreements that exist with this
agreement version.
Add a table for AgreementVersion as well as a detail view and a url.
Add a link in the navbar to this view.
This is not a true detail view, but displays information about
the major_version associated with one or more agreement_versions.
Add a url, a view, a template, and tests.
Add a get_major_version_absolute_url method to AgreementVersion.
Link to this url in relevant tables and detail pages.
@amstilp amstilp added the wip label Aug 30, 2023
@codecov
Copy link

codecov bot commented Aug 30, 2023

Codecov Report

Merging #233 (20257e5) into main (c78c198) will increase coverage by 0.16%.
Report is 18 commits behind head on main.
The diff coverage is 99.92%.

❗ Current head 20257e5 differs from pull request most recent head 3dd387f. Consider uploading reports for the commit 3dd387f to get more accurate results

@@            Coverage Diff             @@
##             main     #233      +/-   ##
==========================================
+ Coverage   97.81%   97.98%   +0.16%     
==========================================
  Files         208      219      +11     
  Lines       14488    15652    +1164     
==========================================
+ Hits        14172    15336    +1164     
  Misses        316      316              
Files Coverage Δ
primed/cdsa/admin.py 100.00% <100.00%> (ø)
primed/cdsa/audit/signed_agreement_audit.py 100.00% <100.00%> (+0.97%) ⬆️
primed/cdsa/audit/workspace_audit.py 100.00% <100.00%> (ø)
primed/cdsa/forms.py 100.00% <100.00%> (ø)
...entmajorversion_historicalagreementmajorversion.py 100.00% <100.00%> (ø)
...ions/0003_agreementversion_add_major_version_fk.py 100.00% <100.00%> (ø)
.../migrations/0004_populate_agreementmajorversion.py 100.00% <100.00%> (ø)
...005_agreementversion_remove_field_major_version.py 100.00% <100.00%> (ø)
...ersion_rename_major_version_fk_to_major_version.py 100.00% <100.00%> (ø)
...tions/0007_alter_agreementversion_major_version.py 100.00% <100.00%> (ø)
... and 21 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

amstilp added 20 commits August 31, 2023 09:27
Add a model definition, a factory, and tests. This model will be
used as a foreign key in AgreementVersion.
This fk will be populated in a following data migration using the
values in major_version.
This data migration does two things:
1. Create an AgreementMajorVersion record for each unique major_version
   in AgreementVersion.
2. Set major_version_fk to the appropriate AgreementMajorVersion
   instance for each AgreementVersion instance.

Also add tests for the migration. Add the django-test-migrations
package to the requirements file for local development, since it
is used in tests.
Remove the previous major_version field and rename the temporary
major_version_fk field to major_version. Note that a lot of the code
will be broken since we didn't udpate any of it yet for these changes.
Rename the constraint, since mysql/mariadb will likely complain if
we don't.
Instead of only migrating from before the data migration to immediately
after, migrate from the first migration before any of the changes
(initial) to the last migration in the set (0007) in tests. This is
a better test of what is actually happening (and what we actually
want to happen).
Instead of having AgreementMajorVersion be a template view that
operates on a non-existent model class, use a detail view that
pulls and instance of the AgreementMajorVersion model. Add a
get_absolute_url method to AgreementMajorVersion, and remove the
get_major_version_absolute_url method from AgreementVersion since
it is no longer necessary. Update tables and templates as necessary
for the removal of this method.
Also modify the table dictionaries/table columns to use accessors
instead of passing each value in the table dictionary (eg, signed
agreement types can be derived from the signed_agreement using an
accessor instead of passed separately).
This field indicates if the version is a valid CDSA or a deprecated
CDSA. Also display this information in the AgreementVersionTable
as a column as well on various detail pages as a pill.
We don't need the custom managers and overhead that comes along with
the StatusModel for AgreementMajorVersion, since we can't access the
model manager methods via the foreign key relationships and the
AgreementMajorVersion model is rarely used directly.
Rename BooleanCheckColumn to BooleanIconColumn, which is a more
representative of what the column is now doing. Add an init argument
that allows the user to specify if they would like icons to be
displayed for "False" values. For the AgreementVersionTable, show
the false icon.
Modify the SignedAgreementForm version queryset to include only
AgreementVersions whose major version is valid. Add a test to check
this.
This update also refactored the code and the tests quite a bit,
which makes it a little easier to read (hopefully). Add more tests.
Similar to the SignedAgreement audit. Expand tests.
This field will indicate if a signed agreement is active or replaced
(or other statuses as added in the future).
In addition to checking whether the agreement version is valid,
the SignedAgreementAudit now checks whether the agreement is active.
In addition to checking if the workspace has a valid CDSA, also check
that that CDSA is active.
Add a view that updates the status of a SignedAgreement. Add three
different urls that use this view, one for each agreement type. Note
that the the different urls need to pass the correct agreement_type
as a kwarg to the view. I decided to do this because then the urls
are more consistent with the detail views, eg:
cdsa/signed_agreements/members/1 (for the detail view)
and
cdsa/signed_agreements/members/1/update (for the update view)

Also add a form that is used in this view, tests for the form, and
tests for the view.
This status is intended to be used for SignedAgreements whose
major version became invalid but were not withdrawn.
If the major version associated with a signed agreement is not valid
but the SignedAgreement is still active, still allow that
SignedAgreement to have access to the CDSA (and any workspaces associated
with that DataAffiliate CDSA's study). We decided that the view to
invalidate an AgreementMajorVersion instance would also set the
status to LAPSED for all SignedAgreements associated with that version.
This gives us additional flexibility if we need in the future (eg, we
can change a LAPSED SignedAgreement to ACTIVE if we need to extend
their access for some reason). Therefore, we only need to consider
the status of the SignedAgreement (and its associated primary, if
applicable).
This view invalidates a given AgreementMajorVerison instance and
also sets the status of all associated agreements to Lapsed.
This button is only shown if the user has edit permission and the
version is valid.
Only records associated with an active SignedAgreement are shown
in the tables.
Instead of hardcoding the CDSA group name as "PRIMED_CDSA", use the
name specified in the settings file.
@amstilp
Copy link
Contributor Author

amstilp commented Sep 25, 2023

Some questions:

  • Should we indicate whether a CDSAWorkspace has an active signed agreement on the CDSAWorkspace table page?

amstilp and others added 3 commits September 29, 2023 13:52
This is no longer needed, since we use the AgreementMajorVersion
foreign key to determine validity.
@amstilp amstilp merged commit 72c13d1 into main Sep 29, 2023
5 checks passed
@amstilp amstilp deleted the feature/cdsa-invalidation branch September 29, 2023 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow CDSAs to be invalidated/closed/expired/etc
1 participant