PR#1016: frontend: user should be able to pin co-maintained projects - copr/copr

copr / copr

#1016 frontend: user should be able to pin co-maintained projects

Merged 4 years ago by praiskup. Opened 4 years ago by schlupov.

copr/ schlupov/copr pin_others_projects into master

frontend: user should be able to pin co-maintained projects

Silvie Chlupova • 4 years ago

ac5ffe5

frontend/coprs_frontend/coprs/logic/complex_logic.py

file modified

+6 -1

		`@@ -8,6 +8,7 @@`
		`from .. import db`
		`from .builds_logic import BuildsLogic`
		`from copr_common.enums import StatusEnum`
		`+ from coprs import helpers`
		`from coprs import models`
		`from coprs import exceptions`
		`from coprs.exceptions import ObjectNotFound, ActionInProgressException`
		`@@ -235,7 +236,11 @@`
		`for group in user.user_groups:`
		`coprs.extend(CoprsLogic.get_multiple_by_group_id(group.id).all())`

		`- return coprs`
		`+ coprs += [perm.copr for perm in user.copr_permissions if`
		`+ perm.get_permission("admin") == helpers.PermissionEnum("approved") or`
		`+ perm.get_permission("builder") == helpers.PermissionEnum("approved")]`
		`+`
		`+ return set(coprs)`


		`class ProjectForking(object):`

schlupov commented 4 years ago

Fixes: #1002

praiskup commented on line 8 of frontend/coprs_frontend/coprs/views/user_ns/user_general.py 4 years ago

shouldn't we rather fix get_coprs_permissible_by_user function?

rebased onto 6e5f8b49637390895a3231c56ddb66cc52392676

4 years ago

schlupov commented 4 years ago

I agree I moved the code into get_coprs_permissible_by_user.

praiskup commented on line 8 of frontend/coprs_frontend/coprs/logic/complex_logic.py 4 years ago

I think there's some risk that the 'coprs' array can already contain some of those ...?

praiskup commented 4 years ago

Thinking about it ... what if we added also projects with builder permission, not only 'admin' permission?

rebased onto aa854bf08501370d1a1f862925c89a9fe217c696

4 years ago

schlupov commented 4 years ago

I wasn't sure about builder permissions so I added just admin permissions for now however it can be changed if you think that users should be able to pin projects in which they have builder permissions.
I added a condition to check that into 'coprs' array is being added copr that is not already in it.

praiskup commented on line 8 of frontend/coprs_frontend/coprs/logic/complex_logic.py 4 years ago

I'd propose to switch coprs from list() to set() in this case.

frostyx commented on line 7 of frontend/coprs_frontend/coprs/logic/complex_logic.py 4 years ago

Please use helpers.PermissionEnum instead of 2 here.

frostyx commented on line 8 of frontend/coprs_frontend/coprs/logic/complex_logic.py 4 years ago

Also, this IMHO looks unnecessarily too complicated. I would merge the filtering and coprs list construction into just one step, e.g.

coprs += [perm.copr for perm in user.copr_permissions
          if perm.get_permission("admin") == 2
          and perm.copr not in coprs]

rebased onto 863e43660055dcc5d2d37dc6884aebf7e38970d2

4 years ago

rebased onto ac5ffe5

4 years ago

frostyx commented 4 years ago

+1, Initially I was a bit worried about changing the return type to the set, but the only place where we use this method is render_pinned_projects and that one immediately converts it back to list by sorting it, so there should be no problem.

praiskup commented 4 years ago