PR#4966: Introduce the collaborator_project_groups mapping - pagure

pagure

#4966 Introduce the collaborator_project_groups mapping

Merged 3 years ago by pingou. Opened 3 years ago by pingou.

fix_collaborator_groups into master

Adjust a test so the logic matches its description

Pierre-Yves Chibon • 3 years ago

7f8ef43

Small code style change

Pierre-Yves Chibon • 3 years ago

1671c2d

Introduce the collaborator_project_groups mapping

Pierre-Yves Chibon • 3 years ago

f48c7d6

pagure/lib/model.py

file modified

+12 -3

		`@@ -487,6 +487,16 @@`
		`)`

		`collaborator_groups = relation(`
		`+ "PagureGroup",`
		`+ secondary="projects_groups",`
		`+ primaryjoin="projects.c.id==projects_groups.c.project_id",`
		`+ secondaryjoin="and_(pagure_group.c.id==projects_groups.c.group_id,\`
		`+ projects_groups.c.access=='collaborator')",`
		`+ order_by="PagureGroup.group_name.asc()",`
		`+ viewonly=True,`
		`+ )`
		`+`
		`+ collaborator_project_groups = relation(`
		`"ProjectGroup",`
		`primaryjoin="and_(projects.c.id==projects_groups.c.project_id,\`
		`projects_groups.c.access=='collaborator')",`
		`@@ -1000,9 +1010,8 @@`
		`elif access == "collaborator":`
		`committers = set(self.committer_groups)`
		`admins = set(self.admin_groups)`
		`- return list(`
		`- set(self.collaborator_groups) - committers - admins`
		`- )`
		`+ collaborators = set(self.collaborator_groups)`
		`+ return list(collaborators - committers - admins)`
		`elif access == "ticket":`
		`committers = set(self.committer_groups)`
		`admins = set(self.admin_groups)`

pagure/utils.py

file modified

+1 -1

		`@@ -311,7 +311,7 @@`
		`return True`

		`# If they are in a group that has commit access -> maybe`
		`- for project_group in repo_obj.collaborator_groups:`
		`+ for project_group in repo_obj.collaborator_project_groups:`
		`if project_group.group.group_name in usergroups:`
		`# if branch is None when the user tries to read,`
		`# so we'll allow that`

tests/test_pagure_flask_api_project.py

file modified

+23 -2

		`@@ -1010,6 +1010,27 @@`
		`)`
		`self.session.commit()`

		`+ # Add a collaborator group`
		`+ msg = pagure.lib.query.add_group(`
		`+ self.session,`
		`+ group_name="some_group",`
		`+ display_name="Some Group",`
		`+ description=None,`
		`+ group_type="bar",`
		`+ user="pingou",`
		`+ is_admin=False,`
		`+ blacklist=[],`
		`+ )`
		`+ pagure.lib.query.add_group_to_project(`
		`+ session=self.session,`
		`+ project=project,`
		`+ new_group="some_group",`
		`+ user="pingou",`
		`+ access="collaborator",`
		`+ branches="features/*",`
		`+ )`
		`+ self.session.commit()`
		`+`
		`# Existing project`
		`output = self.app.get("/api/0/test")`
		`self.assertEqual(output.status_code, 200)`
		`@@ -1019,7 +1040,7 @@`
		`expected_data = {`
		`"access_groups": {`
		`"admin": [],`
		`- "collaborator": [],`
		`+ "collaborator": ["some_group"],`
		`"commit": [],`
		`"ticket": [],`
		`},`
		`@@ -4294,7 +4315,7 @@`
		`project,`
		`new_user="foo",`
		`user="pingou",`
		`- access="ticket",`
		`+ access="collaborator",`
		`)`
		`self.session.commit()`

pingou commented 3 years ago

We need two mappings on a regular basis. One linking a project to its
group of collaborator and pointing to the PagureGroup objects directly.
This is used for example to retrieve the list of collaborator groups.
The one mapping is between a project and the corresponding ProjectGroup
for collaborators which is where the "branches" that the group is
restricted to is stored.

So we now have both mappings available and we need to be careful to use
the proper one, but pagure will quickly indicate (ie: crash) if one
uses the wrong one and tries to access either the group name or the
branches attribute.

Fixes https://pagure.io/fedora-infrastructure/issue/9247

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

rebased onto b126ebb1cdbc6b26f72a7391e8b43322d0e73e35

3 years ago

rebased onto f48c7d6

3 years ago