From 6866d1fa08c96a9a44046076d22958942dfbbe59 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 10 2017 08:11:04 +0000
Subject: [PATCH 1/4] Allow people with ticket access to take and drop issues


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

---

diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html
index bd7e60c..5fccec2 100644
--- a/pagure/templates/issue.html
+++ b/pagure/templates/issue.html
@@ -717,7 +717,7 @@ function try_async_comment(form) {
 {% endif %}
 <script type="text/javascript">
 
-{% if authenticated and g.repo_admin %}
+{% if authenticated and g.repo_user %}
 function take_issue(){
   var _url = "{{ url_for('api_ns.api_assign_issue',
             repo=repo.name, namespace=repo.namespace, username=username,
@@ -739,7 +739,7 @@ function take_issue(){
 {% endif %}
 
 {% if authenticated and (
-    g.repo_admin
+    g.repo_user
     or issue.user.user == g.fas_user.username
     or issue.assignee.user == g.fas_user.username) %}
 function drop_issue(){

From 8c9b3b87b73b2aa08a67e2dd2cf31516cb3bc48e Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 10 2017 08:11:04 +0000
Subject: [PATCH 2/4] Small code style change


---

diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index 8324180..5e13af0 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -94,8 +94,9 @@ def _check_issue_access_repo_commiter(issue):
     :raises pagure.exceptions.APIError: when access denied
     """
     if (
-        issue.private and
-        not is_repo_committer(issue.project) and (
+        issue.private
+        and not is_repo_committer(issue.project)
+        and (
             not api_authenticated() or
             not issue.user.user == flask.g.fas_user.username
         )

From d326ca5b3249e51e9ef9d6699cb7f871ee3f01d8 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 10 2017 08:11:04 +0000
Subject: [PATCH 3/4] Add unit-tests ensuring people with ticket access can action the take button


---

diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py
index 6cb32b2..dc63e2a 100644
--- a/tests/test_pagure_flask_ui_issues.py
+++ b/tests/test_pagure_flask_ui_issues.py
@@ -536,6 +536,12 @@ class PagureFlaskIssuestests(tests.Modeltests):
             self.assertFalse(
                 '<a href="/login/">Login</a> to comment on this ticket.'
                 in output.data)
+            # Not author nor admin = No take
+            self.assertNotIn('function take_issue(){',output.data)
+            self.assertNotIn('function drop_issue(){',output.data)
+            self.assertNotIn(
+                '<button class="btn btn-sm pull-xs-right" id="take-btn"',
+                output.data)
 
         user.username = 'pingou'
         with tests.user_set(pagure.APP, user):
@@ -604,6 +610,85 @@ class PagureFlaskIssuestests(tests.Modeltests):
 
     @patch('pagure.lib.git.update_git')
     @patch('pagure.lib.notify.send_email')
+    def test_view_issue_user_ticket(self, p_send_email, p_ugt):
+        """ Test the view_issue endpoint. """
+        p_send_email.return_value = True
+        p_ugt.return_value = True
+
+        output = self.app.get('/foo/issue/1')
+        self.assertEqual(output.status_code, 404)
+
+        tests.create_projects(self.session)
+        tests.create_projects_git(
+            os.path.join(self.path), bare=True)
+
+        output = self.app.get('/test/issue/1')
+        self.assertEqual(output.status_code, 404)
+
+        # Create issues to play with
+        repo = pagure.lib.get_project(self.session, 'test')
+        msg = pagure.lib.new_issue(
+            session=self.session,
+            repo=repo,
+            title='Test issue',
+            content='We should work on this',
+            user='pingou',
+            ticketfolder=None
+        )
+        self.session.commit()
+        self.assertEqual(msg.title, 'Test issue')
+
+        output = self.app.get('/test/issue/1')
+        self.assertEqual(output.status_code, 200)
+        # Not authentified = No edit
+        self.assertNotIn(
+            '<a class="btn btn-primary btn-sm" href="/test/issue/1/edit" '
+            'title="Edit this issue">',
+            output.data)
+        self.assertTrue(
+            '<a href="/login/?next=http%3A%2F%2Flocalhost%2Ftest%2Fissue%2F1">'
+            'Login</a>\n            to comment on this ticket.'
+            in output.data)
+
+        # Create issues to play with
+        repo = pagure.lib.get_project(self.session, 'test')
+
+        # Add user 'foo' with ticket access on repo
+        msg = pagure.lib.add_user_to_project(
+            self.session,
+            repo,
+            new_user='foo',
+            user='pingou',
+            access='ticket',
+        )
+        self.assertEqual(msg, 'User added')
+        self.session.commit()
+
+        user = tests.FakeUser(username='foo')
+        with tests.user_set(pagure.APP, user):
+            output = self.app.get('/test/issue/1')
+            self.assertEqual(output.status_code, 200)
+            # Not author nor admin = No edit
+            self.assertNotIn(
+                '<a class="btn btn-primary btn-sm" '
+                'href="/test/issue/1/edit" title="Edit this issue">',
+                output.data)
+            self.assertNotIn(
+                '<button class="btn btn-danger btn-sm" type="submit"',
+                output.data)
+            self.assertNotIn('title="Delete this ticket">', output.data)
+            self.assertFalse(
+                '<a href="/login/">Login</a> to comment on this ticket.'
+                in output.data)
+            # user has ticket = take ok
+            self.assertIn('function take_issue(){',output.data)
+            self.assertIn('function drop_issue(){',output.data)
+            self.assertIn(
+                '<button class="btn btn-sm pull-xs-right" id="take-btn"',
+                output.data)
+
+    @patch('pagure.lib.git.update_git')
+    @patch('pagure.lib.notify.send_email')
     def test_view_issue_non_ascii_milestone(self, p_send_email, p_ugt):
         """ Test the view_issue endpoint with non-ascii milestone. """
         p_send_email.return_value = True

From 9e0b8e9fdc4646aaae828b5db07e9b72f52c2842 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 10 2017 08:38:48 +0000
Subject: [PATCH 4/4] pep8 fixes and add a .flake8 configuration file


---

diff --git a/.flake8 b/.flake8
new file mode 100644
index 0000000..711bb18
--- /dev/null
+++ b/.flake8
@@ -0,0 +1,2 @@
+[flake8]
+ignore = W503
diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index 5e13af0..5a49842 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -97,8 +97,8 @@ def _check_issue_access_repo_commiter(issue):
         issue.private
         and not is_repo_committer(issue.project)
         and (
-            not api_authenticated() or
-            not issue.user.user == flask.g.fas_user.username
+            not api_authenticated()
+            or not issue.user.user == flask.g.fas_user.username
         )
     ):
         raise pagure.exceptions.APIError(
diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py
index dc63e2a..fd42b0a 100644
--- a/tests/test_pagure_flask_ui_issues.py
+++ b/tests/test_pagure_flask_ui_issues.py
@@ -537,8 +537,8 @@ class PagureFlaskIssuestests(tests.Modeltests):
                 '<a href="/login/">Login</a> to comment on this ticket.'
                 in output.data)
             # Not author nor admin = No take
-            self.assertNotIn('function take_issue(){',output.data)
-            self.assertNotIn('function drop_issue(){',output.data)
+            self.assertNotIn('function take_issue(){', output.data)
+            self.assertNotIn('function drop_issue(){', output.data)
             self.assertNotIn(
                 '<button class="btn btn-sm pull-xs-right" id="take-btn"',
                 output.data)
@@ -681,8 +681,8 @@ class PagureFlaskIssuestests(tests.Modeltests):
                 '<a href="/login/">Login</a> to comment on this ticket.'
                 in output.data)
             # user has ticket = take ok
-            self.assertIn('function take_issue(){',output.data)
-            self.assertIn('function drop_issue(){',output.data)
+            self.assertIn('function take_issue(){', output.data)
+            self.assertIn('function drop_issue(){', output.data)
             self.assertIn(
                 '<button class="btn btn-sm pull-xs-right" id="take-btn"',
                 output.data)
diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py
index 39784cc..9c5a6b2 100644
--- a/tests/test_pagure_lib.py
+++ b/tests/test_pagure_lib.py
@@ -3580,9 +3580,9 @@ class PagureLibtests(tests.Modeltests):
         self.assertRaises(
             pagure.exceptions.AccessLevelNotFound,
             pagure.lib.get_project_users,
-                self.session,
-                project_obj=project,
-                access='owner',
+            self.session,
+            project_obj=project,
+            access='owner',
         )
 
         # Let's add a new user to the project, 'foo'

