From e14f1e79d2f24e96468c9d7334d4087e10ba06a3 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 13 2017 14:10:29 +0000
Subject: [PATCH 1/4] Ensure people with ticket metadata can edit the custom fields.


---

diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html
index 5fccec2..892a985 100644
--- a/pagure/templates/issue.html
+++ b/pagure/templates/issue.html
@@ -379,7 +379,7 @@
               </p>
             </div>
           </fieldset>
-          {% if authenticated and g.repo_admin %}
+          {% if authenticated and g.repo_user %}
             <fieldset class="form-group issue-custom-form">
               <label for="field"><strong>{{ field.name }}</strong></label>
               {% if field.key_type == 'list' %}

From 95877ad743d0ab26873ed57c81fe0d59d28ba676 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 13 2017 14:10:29 +0000
Subject: [PATCH 2/4] Add unit-tests ensuring people with ticket access to view and edit custom fields


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

---

diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py
index fd42b0a..b328ea0 100644
--- a/tests/test_pagure_flask_ui_issues.py
+++ b/tests/test_pagure_flask_ui_issues.py
@@ -689,6 +689,126 @@ class PagureFlaskIssuestests(tests.Modeltests):
 
     @patch('pagure.lib.git.update_git')
     @patch('pagure.lib.notify.send_email')
+    def test_view_issue_custom_field_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')
+
+        # Add user 'foo' with ticket access on repo
+        repo = pagure.lib.get_project(self.session, 'test')
+        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()
+
+        # Set some custom fields
+        repo = pagure.lib.get_project(self.session, 'test')
+        msg = pagure.lib.set_custom_key_fields(
+            self.session,
+            repo,
+            ['bugzilla', 'upstream', 'reviewstatus'],
+            ['link', 'boolean', 'list'],
+            ['unused data for non-list type', '', 'ack, nack ,  needs review'],
+            [None, None, None])
+        self.session.commit()
+        self.assertEqual(msg, 'List of custom fields updated')
+
+        # User with no rights
+        user = tests.FakeUser()
+        with tests.user_set(pagure.APP, user):
+            output = self.app.get('/test/issue/1')
+            self.assertEqual(output.status_code, 200)
+            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)
+            # user no ACLs = no take action/button
+            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 no ACLs = no metadata form
+            self.assertNotIn(
+                '<input                  class="form-control" '
+                'name="bugzilla" id="bugzilla"/>',output.data)
+            self.assertNotIn(
+                '<select class="form-control" name="reviewstatus" '
+                'id="reviewstatus>',output.data)
+            self.assertNotIn(
+                '<input type="checkbox"                   '
+                'class="form-control" name="upstream" id="upstream"/>',
+                output.data)
+
+        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)
+            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)
+
+            # user has ticket == Sees the metadata
+            self.assertIn(
+                '<input                  class="form-control" '
+                'name="bugzilla" id="bugzilla"/>',output.data)
+            self.assertIn(
+                '<select class="form-control" name="reviewstatus" '
+                'id="reviewstatus>',output.data)
+            self.assertIn(
+                '<input type="checkbox"                   '
+                'class="form-control" name="upstream" id="upstream"/>',
+                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 bcb5c114804fa927d401914217ef617647af7232 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 13 2017 14:10:29 +0000
Subject: [PATCH 3/4] Fix logic around merge_pull_request and diff_pull_request


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

---

diff --git a/pagure/lib/git.py b/pagure/lib/git.py
index 020dad3..4172180 100644
--- a/pagure/lib/git.py
+++ b/pagure/lib/git.py
@@ -1188,7 +1188,7 @@ def merge_pull_request(
     # Update the start and stop commits in the DB, one last time
     diff_commits = diff_pull_request(
         session, request, fork_obj, PagureRepo(parentpath),
-        requestfolder=request_folder, with_diff=False)[0]
+        requestfolder=request_folder, with_diff=False)
 
     if request.project.settings.get(
             'Enforce_signed-off_commits_in_pull-request', False):
@@ -1535,7 +1535,7 @@ def diff_pull_request(
     if with_diff:
         return (diff_commits, diff)
     else:
-        diff_commits
+        return diff_commits
 
 
 def get_git_tags(project):
diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py
index df2b502..45befbc 100644
--- a/pagure/ui/fork.py
+++ b/pagure/ui/fork.py
@@ -302,7 +302,7 @@ def request_pull_patch(repo, requestid, username=None, namespace=None):
             diff_commits = pagure.lib.git.diff_pull_request(
                 SESSION, request, repo_obj, orig_repo,
                 requestfolder=APP.config['REQUESTS_FOLDER'],
-                with_diff=False)[0]
+                with_diff=False)
         except pagure.exceptions.PagureException as err:
             flask.flash(err.message, 'error')
             return flask.redirect(flask.url_for(

From 14e8263a8b6e528be16878b209953a06176259d8 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Apr 13 2017 14:46:05 +0000
Subject: [PATCH 4/4] Create the projects before creating their API tokens


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

---

diff --git a/tests/test_pagure_flask_api_group.py b/tests/test_pagure_flask_api_group.py
index 8d82405..b0fbd45 100644
--- a/tests/test_pagure_flask_api_group.py
+++ b/tests/test_pagure_flask_api_group.py
@@ -92,6 +92,7 @@ class PagureFlaskApiGroupTests(tests.Modeltests):
             Test the api_view_group method of the flask api with an
             authenticated user. The tested group has one member.
         """
+        tests.create_projects(self.session)
         tests.create_tokens(self.session)
         headers = {'Authorization': 'token aaabbbcccddd'}
         output = self.app.get('/api/0/group/some_group', headers=headers)
@@ -158,7 +159,9 @@ class PagureFlaskApiGroupTests(tests.Modeltests):
             self.session, user.username, group, user.username, True)
         self.session.commit()
 
+        tests.create_projects(self.session)
         tests.create_tokens(self.session)
+
         headers = {'Authorization': 'token aaabbbcccddd'}
         output = self.app.get('/api/0/group/some_group', headers=headers)
         self.assertEqual(output.status_code, 200)