From d91182e9bd01582c3ba1671a758038819943fb9b Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Tue, 18 Aug 2020 13:33:21 +0200
Subject: [PATCH 5/9] Introduce the collaborator_project_groups mapping

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>
---
 pagure/lib/model.py                    | 10 ++++++++++
 pagure/utils.py                        |  2 +-
 tests/test_pagure_flask_api_project.py | 23 ++++++++++++++++++++++-
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/pagure/lib/model.py b/pagure/lib/model.py
index 325a5452..4eea9f58 100644
--- a/pagure/lib/model.py
+++ b/pagure/lib/model.py
@@ -487,6 +487,16 @@ class Project(BASE):
     )
 
     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')",
diff --git a/pagure/utils.py b/pagure/utils.py
index 24a0ce4d..915ba499 100644
--- a/pagure/utils.py
+++ b/pagure/utils.py
@@ -311,7 +311,7 @@ def is_repo_collaborator(repo_obj, refname, username=None, session=None):
                     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
diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py
index 3c364be2..51dd8e9f 100644
--- a/tests/test_pagure_flask_api_project.py
+++ b/tests/test_pagure_flask_api_project.py
@@ -1010,6 +1010,27 @@ class PagureFlaskApiProjecttests(tests.Modeltests):
         )
         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 @@ class PagureFlaskApiProjecttests(tests.Modeltests):
         expected_data = {
             "access_groups": {
                 "admin": [],
-                "collaborator": [],
+                "collaborator": ["some_group"],
                 "commit": [],
                 "ticket": [],
             },
-- 
2.26.2