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

Track 'archived' status on ActiveProject model. Ref #2148 #2149

Merged
merged 7 commits into from
Dec 15, 2023

Conversation

tompollard
Copy link
Member

@tompollard tompollard commented Dec 1, 2023

The ArchivedProject stores information about projects that are either deleted (by the creator) or rejected (by an editor). As discussed in #2148, there are some major issues with the ArchivedProject object. I think there is agreement that it should be removed.

This pull request moves us towards being able to delete the ArchivedProject object, by:

  • adding a new archived status to the ActiveProject object
  • moving existing logic for archiving projects to use this archived status.

The change touches a lot of files, so the pull request looks bigger than it is. All we are doing is replacing the references to ArchivedProject with references to ActiveProject.submission_status=ARCHIVED.

After this pull request is merged, ArchivedProject objects will continue to exist in the database, but they will not be visible to users and they cannot be created.

Some next steps would be to:

  1. Decide how to deal with existing Archived Projects (possibly we could get away with simply deleting them, and then removing the model.
  2. Remove all GenericForeignKeys and never touch them again!
  3. Improve project logging. It would be helpful to have an event log that tracks the status of projects as they move from creation -> review -> publication/rejection/deletion.
  4. Come up with an approach for dealing with the files for archived projects. When can they be deleted and when do they need to be saved?

There is one bug (that I'm aware of) that this change creates: if a new version of a project is created and then archived, it is not possible to create a new version of that project. I think this is a fairly minor issue, and I would like to address in a later pull request.

tompollard added a commit that referenced this pull request Dec 1, 2023
The archive_datetime column was added in #2149. The migration appeared to be looking for the column, and so this is a retrospective fix to allow the build to proceed.
tompollard added a commit that referenced this pull request Dec 1, 2023
The archive_datetime column was added in #2149. The migration appeared to be looking for the column, and so this is a retrospective fix to allow the build to proceed.
tompollard added a commit that referenced this pull request Dec 1, 2023
The archive_datetime column was added in #2149. The migration appeared to be looking for the column, and so this is a retrospective fix to allow the build to proceed.
tompollard added a commit that referenced this pull request Dec 1, 2023
The archive_datetime column was added in #2149. The migration appeared to be looking for the column, and so this is a retrospective fix to allow the build to proceed.
@tompollard tompollard changed the title Track 'archived' status on ActiveProject model. Ref #2148 [WIP] Track 'archived' status on ActiveProject model. Ref #2148 Dec 1, 2023
@tompollard
Copy link
Member Author

@bemoody this is now ready for review. Sorry for the churn!

@bemoody
Copy link
Collaborator

bemoody commented Dec 5, 2023

This does not delete the files when it's expected to (when the author deletes the project without submitting it.)

Also note that this makes the content of both deleted and rejected projects permanently visible to authors and editors (if you know the URL.) I'm not sure if that's what you intended. It seems dangerous.

@tompollard
Copy link
Member Author

This does not delete the files when it's expected to (when the author deletes the project without submitting it.)

I have now added the clear_files=True argument for projects that are deleted by the user, for consistency with current behavior. Independently of this PR it would be good to decide on a clear policy for deleting files.

Also note that this makes the content of both deleted and rejected projects permanently visible to authors and editors (if you know the URL.) I'm not sure if that's what you intended. It seems dangerous.

I have updated the access policy for authors to prevent access to archived projects (using the implementation that you suggested - thanks!). I have not restricted access to editors, because I think it is probably useful for them to be able to review content of archived content.

@bemoody
Copy link
Collaborator

bemoody commented Dec 6, 2023

This prevents you from creating a new project once you have ten archived proejects.

@tompollard
Copy link
Member Author

tompollard commented Dec 6, 2023

This prevents you from creating a new project once you have ten archived projects.

Good catch! I have moved the MAX_SUBMITTABLE_PROJECTS variable to the .env file to make it customizable. Excluding the ARCHIVED_PROJECTS from count is awkward because of the generic foreign keys that link the Author object to both ArchivedProject and ActiveProject models:

n_submitting = Author.objects.filter(user=user, is_submitting=True,
content_type=ContentType.objects.get_for_model(ActiveProject)).count()

I implemented the alternative approach that we discussed (first get keys of all active projects except those with SubmissionStatus.ARCHIVED, then filter using the IDs).

@tompollard
Copy link
Member Author

@kshalot please could you confirm that deprecating ArchivedProject will not cause any issues for Health Data Nexus?

@tompollard
Copy link
Member Author

@bemoody Karol has confirmed that this change will not cause any issues for Health Data Nexus. Please could you go ahead and merge?

@bemoody bemoody merged commit ce72593 into dev Dec 15, 2023
11 checks passed
@bemoody
Copy link
Collaborator

bemoody commented Dec 15, 2023

TThanks for your work on this! Could you please raise an issue or issues about the next steps you mentioned above?

@tompollard tompollard deleted the tp/archive_activeproject branch December 15, 2023 17:52
@tompollard
Copy link
Member Author

Could you please raise an issue or issues about the next steps you mentioned above?

Thanks Benjamin, will do!

tompollard added a commit that referenced this pull request Sep 25, 2024
This pull request removes the unused `ArchivedProject` model.

We now track the "Archived" status of projects on the ActiveProject
model (see: #2149).

In #2170 we migrated all
ArchivedProject content to ActiveProjects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants