From 7aaf05739ba702274cf0ee73df530b08c57b579c Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Mar 27 2018 12:28:55 +0000
Subject: [PATCH 1/2] Fix the user's requests page


We used to include the projects in which an user had opened at one point
a pull-request in, this lead to a lot of false positive.
Now we're being strickter about when retrieving the list of projects
and adding the pull-requests that the used opened in a second step.

So the logic is something like:
- Get all the projects the user is involved in (direct access or via a
  group)
- Get all the PRs in these projects + all the PRs opened by that user
- Group them all into one final query that is then ordered/filtered as
  desired

Fixes https://pagure.io/pagure/issue/2930

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

---

diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py
index 382fa29..e3ceade 100644
--- a/pagure/lib/__init__.py
+++ b/pagure/lib/__init__.py
@@ -3839,7 +3839,7 @@ def get_pull_request_of_user(
         )
     )
     sub_q2 = session.query(
-        model.Project.id
+        sqlalchemy.distinct(model.Project.id)
     ).filter(
         # User got commit right
         sqlalchemy.and_(
@@ -3853,7 +3853,7 @@ def get_pull_request_of_user(
         )
     )
     sub_q3 = session.query(
-        model.Project.id
+        sqlalchemy.distinct(model.Project.id)
     ).filter(
         # User created a group that has commit right
         sqlalchemy.and_(
@@ -3869,7 +3869,7 @@ def get_pull_request_of_user(
         )
     )
     sub_q4 = session.query(
-        model.Project.id
+        sqlalchemy.distinct(model.Project.id)
     ).filter(
         # User is part of a group that has commit right
         sqlalchemy.and_(
@@ -3885,22 +3885,31 @@ def get_pull_request_of_user(
             )
         )
     )
-    sub_q5 = session.query(
-        model.Project.id
+
+    projects = projects.union(sub_q2).union(sub_q3).union(sub_q4)
+
+    query = session.query(
+        sqlalchemy.distinct(model.PullRequest.uid)
     ).filter(
+        model.PullRequest.project_id.in_(projects.subquery())
+    )
+
+    query_2 = session.query(
+        sqlalchemy.distinct(model.PullRequest.uid)
+    ).filter(
+        # User open the PR
         sqlalchemy.and_(
-            model.Project.id == model.PullRequest.project_id,
             model.PullRequest.user_id == model.User.id,
             model.User.user == username
         )
     )
 
-    projects = projects.union(sub_q2).union(sub_q3).union(sub_q4).union(sub_q5)
+    final_sub = query.union(query_2)
 
     query = session.query(
         model.PullRequest
     ).filter(
-        model.PullRequest.project_id.in_(projects.subquery())
+        model.PullRequest.uid.in_(final_sub.subquery())
     ).order_by(
         model.PullRequest.date_created.desc()
     )

From a264131e0f640e934e4fd08330aa72f6922f0196 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Mar 27 2018 12:28:55 +0000
Subject: [PATCH 2/2] Add unit-tests ensuring the fix to the user's requests page stays


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

---

diff --git a/tests/test_pagure_flask_ui_app.py b/tests/test_pagure_flask_ui_app.py
index 7bf65db..33f617c 100644
--- a/tests/test_pagure_flask_ui_app.py
+++ b/tests/test_pagure_flask_ui_app.py
@@ -1327,6 +1327,64 @@ class PagureFlaskApptests(tests.Modeltests):
         'pagure.lib.git.update_git', MagicMock(return_value=True))
     @patch(
         'pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_view_my_requests_pr_in_another_project(self):
+        """Test the view_user_requests endpoint when the user opened a PR
+        in another project. """
+        # Pingou creates the PR on test
+        tests.create_projects(self.session)
+        repo = pagure.lib._get_project(self.session, 'test')
+        req = pagure.lib.new_pull_request(
+            session=self.session,
+            repo_from=repo,
+            branch_from='dev',
+            repo_to=repo,
+            branch_to='master',
+            title='test pull-request #1',
+            user='pingou',
+            requestfolder=None,
+        )
+        self.session.commit()
+        self.assertEqual(req.id, 1)
+        self.assertEqual(req.title, 'test pull-request #1')
+
+        # foo creates the PR on test
+        repo = pagure.lib._get_project(self.session, 'test')
+        req = pagure.lib.new_pull_request(
+            session=self.session,
+            repo_from=repo,
+            branch_from='dev',
+            repo_to=repo,
+            branch_to='master',
+            title='test pull-request #2',
+            user='foo',
+            requestfolder=None,
+        )
+        self.session.commit()
+        self.assertEqual(req.id, 2)
+        self.assertEqual(req.title, 'test pull-request #2')
+
+        # Check pingou's PR list
+        output = self.app.get('/user/pingou/requests')
+        self.assertEqual(output.status_code, 200)
+        self.assertIn('test pull-request #1', output.data)
+        self.assertIn('test pull-request #2', output.data)
+        self.assertEqual(
+            output.data.count('<tr class="pr-status pr-status-open"'),
+            2)
+
+        # Check foo's PR list
+        output = self.app.get('/user/foo/requests')
+        self.assertEqual(output.status_code, 200)
+        self.assertNotIn('test pull-request #1', output.data)
+        self.assertIn('test pull-request #2', output.data)
+        self.assertEqual(
+            output.data.count('<tr class="pr-status pr-status-open"'),
+            1)
+
+    @patch(
+        'pagure.lib.git.update_git', MagicMock(return_value=True))
+    @patch(
+        'pagure.lib.notify.send_email', MagicMock(return_value=True))
     def test_view_my_requests_against_another_project(self):
         """Test the view_user_requests endpoint when there is a PR opened
         by me against a project I do not have rights on. """