Allow marking projects as "archived" #17005

facutuesca · 2024-10-30T18:46:46Z

What is this?

Following the discussion here, this PR shows what the implementation could look like for adding "status markers" to projects.

Specifically, this PR allows changing a project's status to "Archived".

Details

The implementation is straightforward, since a Project.lifecycle_status field already exists (to mark a project as quarantined). This change adds a new "archived" value for that field, and adds the UI so that users can archive and unarchive their projects.

Even though the issue mentioned above proposes more statuses ("deprecated", "finished", etc), those can be ambiguous and overlapping, and would require further discussion to define clearly. "Archived", however, is already widely used on GitHub repos and its meaning should be easy for users to understand:

You can archive a repository to make it read-only for all users and indicate that it's no longer actively maintained. You can also unarchive repositories that have been archived

TODO

Actually restrict file uploads when project is Archived
Add admin UI to archive/unarchive projects

Screenshots

Project settings

Project landing page

cc @woodruffw @miketheman

miketheman

Generally, looking good, thanks for scoping it down to a single lifecycle status.

I think something good to add to the owner-facing management page would be a suggestion to publish a new release with a readme update or something to point folks where to go next.
Prior ideas have been to allow user-submitted text in the form, but I like that less, since then we have to figure out where to store that.

I think a warning modal popup confirmation might also be in order when archiving would be a good idea to prevent misclicks.

This also doesn't appear to block users from making further releases to an archived project - is that intentionally excluded? In my head, once Archived, it's in read-only mode - similar to GitHub's Archived.

warehouse/templates/manage/project/settings.html

miketheman · 2024-11-19T18:29:33Z

I also just realized - this probably needs some added notation in the Admin UI/ability to change there as well. I didn't see anything in scope here yet.

Signed-off-by: Facundo Tuesca <[email protected]>

facutuesca · 2024-11-25T22:59:26Z

@miketheman

I think something good to add to the owner-facing management page would be a suggestion to publish a new release with a readme update or something to point folks where to go next.
Prior ideas have been to allow user-submitted text in the form, but I like that less, since then we have to figure out where to store that.

I think a warning modal popup confirmation might also be in order when archiving would be a good idea to prevent misclicks.>

I changed the UI to add the suggestion and the modal popup. Since these are specific to archiving a project, I changed the UI so that instead of a dropdown with the backend statuses, the user sees two buttons to archive or unarchive their project (see the updated screenshots)

This also doesn't appear to block users from making further releases to an archived project - is that intentionally excluded? In my head, once Archived, it's in read-only mode - similar to GitHub's Archived.

I'll add the restrictions once the rest is ready. I was thinking to only disallow uploads for now, WDYT?

this probably needs some added notation in the Admin UI/ability to change there as well. I didn't see anything in scope here yet.

What should the UI for this look?

Signed-off-by: Facundo Tuesca <[email protected]>

facutuesca · 2024-11-26T16:05:48Z

Pushed a commit to disallow uploads on archived projects

Signed-off-by: Facundo Tuesca <[email protected]>

facutuesca · 2024-11-26T21:13:40Z

Added admin UI to archive/unarchive projects:

miketheman

Coming along nicely, thanks for pushing forward on this. I had some notes inline, mostly structural in nature, but some questions as well - nothing too "heavy".

Let me know if anything isn't clear!

miketheman · 2024-12-10T16:15:32Z

tests/unit/packaging/test_models.py

+        owner2 = DBRoleFactory.create(project=project)
+
+        # Maintainers should not appear in the ACLs, since they only have
+        # upload permissions, and anchived projects don't allow upload


typo:

Suggested change

# upload permissions, and anchived projects don't allow upload

# upload permissions, and archived projects don't allow upload

miketheman · 2024-12-10T16:17:22Z

tests/unit/packaging/test_models.py

+        DBRoleFactory.create(project=project, role_name="Maintainer")
+        DBRoleFactory.create(project=project, role_name="Maintainer")


minor optimization, useful if same params and return value not used

Suggested change

DBRoleFactory.create(project=project, role_name="Maintainer")

DBRoleFactory.create(project=project, role_name="Maintainer")

DBRoleFactory.create_batch(2, project=project, role_name="Maintainer")

miketheman · 2024-12-10T22:34:47Z

warehouse/utils/project.py

+    if (
+        project.lifecycle_status is None
+        or project.lifecycle_status == LifecycleStatus.QuarantineExit
+    ):
+        project.lifecycle_status = LifecycleStatus.Archived
+        project.record_event(
+            tag=EventTag.Project.ProjectArchiveEnter,
+            request=request,
+            additional={
+                "submitted_by": request.user.username,
+            },
+        )
+        request.session.flash("Project archived", queue="success")
+    else:
+        request.session.flash(
+            f"Cannot archive project with status {project.lifecycle_status}",
+            queue="error",
+        )


style: Consider early-exit inversion of the logic, like so (pseudocode):

if not <conditions that must be met>: flash "cannot archive", error return do the work flash "done", success

thus communicate early on the exit clause, continue normally otherwise.
Removes one layer of indentation for "the work", and makes it simpler to create another if/else when necessary with fewer logical jumps to make.

Similar applies to unarchive_project

miketheman · 2024-12-10T22:43:08Z

warehouse/packaging/models.py

        for publisher in self.oidc_publishers:
-            acls.append((Allow, f"oidc:{publisher.id}", [Permissions.ProjectsUpload]))
+            if self.lifecycle_status != LifecycleStatus.Archived:
+                # Only allow uploads in non-archived projects
+                acls.append(
+                    (Allow, f"oidc:{publisher.id}", [Permissions.ProjectsUpload])
+                )


optimize: Adding the if will now happen on each iteration of the loop per publisher, but the project's lifecycle happens once, probably can shortcut the loop instead by placing the if outside.
Most projects only have a single publisher, but just in case, save the iteration if unnecessary.

miketheman · 2024-12-10T22:52:02Z

warehouse/packaging/models.py

+                current_permissions = [
+                    p for p in current_permissions if p != Permissions.ProjectsUpload
+                ]


optimize: could this be instead?

Suggested change

current_permissions = [

p for p in current_permissions if p != Permissions.ProjectsUpload

]

current_permissions.remove(Permissions.ProjectsUpload)

I think each branch of current_permissions ends up with ProjectsUpload, and this might be faster than a list comprehension?

facutuesca requested a review from a team as a code owner October 30, 2024 18:46

woodruffw added the UX/UI design, user experience, user interface label Nov 19, 2024

facutuesca force-pushed the add-archived-project-status branch from 78ef39e to f22daf0 Compare November 19, 2024 16:40

miketheman reviewed Nov 19, 2024

View reviewed changes

warehouse/templates/manage/project/settings.html Outdated Show resolved Hide resolved

facutuesca force-pushed the add-archived-project-status branch 2 times, most recently from 365bced to 0a59e39 Compare November 25, 2024 22:50

feat: allow marking projects as archived

2c2853b

Signed-off-by: Facundo Tuesca <[email protected]>

facutuesca force-pushed the add-archived-project-status branch from 0a59e39 to 2c2853b Compare November 25, 2024 22:56

disallow uploads on archived projects

83193e3

Signed-off-by: Facundo Tuesca <[email protected]>

facutuesca added 3 commits November 26, 2024 22:09

remove dead code

ce387c2

Signed-off-by: Facundo Tuesca <[email protected]>

move archive functions

2f3fa08

Signed-off-by: Facundo Tuesca <[email protected]>

add admin UI to archive/unarchive projects

5f07641

Signed-off-by: Facundo Tuesca <[email protected]>

facutuesca requested a review from miketheman November 26, 2024 21:13

woodruffw assigned facutuesca Dec 3, 2024

miketheman requested changes Dec 10, 2024

View reviewed changes

miketheman added the HTML requires change to HTML files label Dec 10, 2024

demeringo mentioned this pull request Dec 13, 2024

Remove sdk publication workflows (or document their purpose) Boavizta/boaviztapi#356

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow marking projects as "archived" #17005

Allow marking projects as "archived" #17005

facutuesca commented Oct 30, 2024 •

edited

Loading

miketheman left a comment

miketheman commented Nov 19, 2024

facutuesca commented Nov 25, 2024

facutuesca commented Nov 26, 2024

facutuesca commented Nov 26, 2024

miketheman left a comment

miketheman Dec 10, 2024

miketheman Dec 10, 2024

miketheman Dec 10, 2024

miketheman Dec 10, 2024

miketheman Dec 10, 2024

	# upload permissions, and anchived projects don't allow upload
	# upload permissions, and archived projects don't allow upload

		DBRoleFactory.create(project=project, role_name="Maintainer")
		DBRoleFactory.create(project=project, role_name="Maintainer")

	DBRoleFactory.create(project=project, role_name="Maintainer")
	DBRoleFactory.create(project=project, role_name="Maintainer")
	DBRoleFactory.create_batch(2, project=project, role_name="Maintainer")

Allow marking projects as "archived" #17005

Are you sure you want to change the base?

Allow marking projects as "archived" #17005

Conversation

facutuesca commented Oct 30, 2024 • edited Loading

What is this?

Details

TODO

Screenshots

Project settings

Project landing page

miketheman left a comment

Choose a reason for hiding this comment

miketheman commented Nov 19, 2024

facutuesca commented Nov 25, 2024

facutuesca commented Nov 26, 2024

facutuesca commented Nov 26, 2024

miketheman left a comment

Choose a reason for hiding this comment

miketheman Dec 10, 2024

Choose a reason for hiding this comment

miketheman Dec 10, 2024

Choose a reason for hiding this comment

miketheman Dec 10, 2024

Choose a reason for hiding this comment

miketheman Dec 10, 2024

Choose a reason for hiding this comment

miketheman Dec 10, 2024

Choose a reason for hiding this comment

facutuesca commented Oct 30, 2024 •

edited

Loading