pagure

Clone

#3865 Fix letting the user who opened the ticket close it
Merged 6 years ago by pingou. Opened 6 years ago by pingou.
fix_closing  into  master

file modified
+5 -4 
@@ -289,10 +289,11 @@ 
 

          else:
 

              _log.debug("  Using template at: %s", templ)
 

  

-     # There is a risk for a race-condition here between when the repo is created
 

-     # and when the README gets added. However, this risk is small enough that we
 

-     # will keep this as is for now (esp since it fixes the situation where
 

-     # deleting the project raised an error if it was in the middle of the lock)
 

+     # There is a risk for a race-condition here between when the repo is
 

+     # created and when the README gets added. However, this risk is small
 

+     # enough that we will keep this as is for now (esp since it fixes the
 

+     # situation where deleting the project raised an error if it was in the
 

+     # middle of the lock)
 

      try:
 

          with project.lock("WORKER"):
 

              pagure.lib.git.create_project_repos(

file modified
+10 -6 
@@ -18,12 +18,15 @@ 
 

  {% block repo %}
 

  <div class="d-flex align-items-start">
 

      <h4 class="ml-1">
 

-         {% if g.authenticated and (g.repo_user or open_access) %}
 

+       {% if g.authenticated and (g.repo_user or open_access or g.fas_user.username == issue.user.user) %}
 

          <form action="{{ url_for('ui_ns.update_issue', username=username,
 

          namespace=repo.namespace, repo=repo.name, issueid=issueid)
 

          }}" method="post" class="hidden" id="changestatusform">
 

+         {{form.csrf_token}}
 

          <input type="hidden" id="statusform_status" name="status" value=""/>
 

          <input type="hidden" id="statusform_close_status" name="close_status" value=""/>
 

+       {% endif %}
 

+       {% if g.authenticated and (g.repo_user or open_access) %}
 

          <input type="hidden" name="tag" value="{{ issue.tags_text | join(',') }}"/>
 

          <input type="hidden" name="depending" value="{{ issue.depending_text | join(',') }}"/>
 

          <input type="hidden" name="blocking" value="{{ issue.blocking_text | join(',') }}"/>
 
@@ -31,7 +34,6 @@ 
 

          {{form.priority}}
 

          {{form.milestone}}
 

          {{form.private}}
 

-         {{form.csrf_token}}
 

          {% if repo.issue_keys %}
 

            {% for field in repo.issue_keys %}
 

              {% if field.key_type == 'list' %}
 
@@ -58,8 +60,10 @@ 
 

              {% endif %}
 

            {% endfor %}
 

          {% endif %}
 

+       {% endif %}
 

+       {% if g.authenticated and (g.repo_user or open_access or g.fas_user.username == issue.user.user) %}
 

          </form>
 

-         {% endif %}
 

+       {% endif %}
 

          <div>
 

            {% if issue.private %}
 

              <span title="Private ticket" class="text-danger fa fa-fw fa-lock"></span>
 
@@ -119,7 +123,7 @@ 
 

      <div class="ml-auto">
 

          <div class="btn-group">
 

          <div class="dropdown">
 

-           {% if g.authenticated and (g.repo_user or open_access) %}
 

+           {% if g.authenticated and (g.repo_user or open_access or g.fas_user.username == issue.user.user) %}
 

            <a href="javascript:void(0)" class="font-weight-bold btn btn-sm {{'btn-success' if issue.status=='Open' else 'btn-danger'}} dropdown-toggle"
 

            id="dropdownMenuButton" data-toggle='dropdown' aria-haspopup="true" aria-expanded="false">
 

            {% else %}
 
@@ -331,7 +335,7 @@ 
 

                  {% else %}
 

                    <div class="text-muted">
 

                      None
 

-                     {% if g.authenticated and (g.repo_user or open_access) and issue.status|lower == 'open'
 

+                     {% if g.authenticated and (g.repo_user or g.fas_user.username == issue.user.user or open_access) and issue.status|lower == 'open'
 

                        and (not issue.assignee or issue.assignee.username != g.fas_user.username)
 

                        and not repo.settings.get('issue_tracker_read_only', False) %}
 

                        &mdash; <a href="javascript:void(0)" id="take-btn"
 
@@ -1024,7 +1028,7 @@ 
 

  

  

  <script type="text/javascript">
 

- {% if g.authenticated and (g.repo_user or open_access) %}
 

+ {% if g.authenticated and (g.repo_user or issue.user.user == g.fas_user.username or open_access) %}
 

  function take_issue(){
 

    var _url = "{{ url_for('api_ns.api_assign_issue',
 

              repo=repo.name, namespace=repo.namespace, username=username,

file modified
+69 
@@ -1247,6 +1247,75 @@ 
 

          output = self.app.get('/test/issue/1')
 

          self.assertEqual(output.status_code, 404)
 

  

+     @patch('pagure.lib.git.update_git', MagicMock(return_value=True))
 

+     @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
 

+     def test_view_issue_author(self):
 

+         """ Test the view_issue endpoint when you're the author. """
 

+         tests.create_projects(self.session)
 

+         tests.create_projects_git(
 

+             os.path.join(self.path, 'repos'), bare=True)
 

+ 

+         # Create issues to play with
 

+         repo = pagure.lib.get_authorized_project(self.session, 'test')
 

+         msg = pagure.lib.new_issue(
 

+             session=self.session,
 

+             repo=repo,
 

+             title='Test issue',
 

+             content='We should work on this',
 

+             user='foo',
 

+         )
 

+         self.session.commit()
 

+         self.assertEqual(msg.title, 'Test issue')
 

+ 

+         output = self.app.get('/test/issue/1')
 

+         self.assertEqual(output.status_code, 200)
 

+         output_text = output.get_data(as_text=True)
 

+         # Not authentified = No edit & no Close
 

+         self.assertNotIn(
 

+             '<a class="btn btn-outline-secondary btn-sm border-0" '
 

+             'href="/test/issue/1/edit" title="Edit this issue">\n',
 

+             output_text)
 

+         self.assertNotIn(
 

+             '<form action="/test/issue/1/update" method="post" class="hidden"',
 

+             output_text)
 

+         self.assertNotIn(
 

+             '<input type="hidden" id="statusform_status" name="status" '
 

+             'value=""/>\n', output_text)
 

+         self.assertNotIn(
 

+             '<input type="hidden" id="statusform_close_status" '
 

+             'name="close_status" value=""/>', output_text)
 

+         self.assertIn(
 

+             '<a href="/login/?next=http%3A%2F%2Flocalhost%2Ftest%2Fissue%2F1">'
 

+             'Login</a>\n          to comment on this ticket.',
 

+             output_text)
 

+ 

+         user = tests.FakeUser(username='foo')
 

+         with tests.user_set(self.app.application, user):
 

+             output = self.app.get('/test/issue/1')
 

+             self.assertEqual(output.status_code, 200)
 

+             output_text = output.get_data(as_text=True)
 

+             # Author = Ability to close ticket
 

+             self.assertIn(
 

+                 '<input type="hidden" id="statusform_status" name="status" '
 

+                 'value=""/>', output_text)
 

+             self.assertIn(
 

+                 '<input type="hidden" id="statusform_close_status" '
 

+                 'name="close_status" value=""/>', output_text)
 

+             # Author = edit
 

+             self.assertIn(
 

+                 '<a class="btn btn-outline-secondary btn-sm border-0"'
 

+                  ' href="/test/issue/1/edit" title="Edit this issue">',
 

+                 output_text)
 

+             self.assertFalse(
 

+                 '<a href="/login/">Login</a> to comment on this ticket.'
 

+                 in output_text)
 

+             # author admin = take
 

+             self.assertIn('function take_issue(){', output_text)
 

+             self.assertIn('function drop_issue(){', output_text)
 

+             self.assertIn(
 

+                 '<a href="javascript:void(0)" id="take-btn"\n',
 

+                 output_text)
 

+ 

      @patch('pagure.lib.git.update_git')
 

      @patch('pagure.lib.notify.send_email')
 

      def test_view_issue_user_ticket(self, p_send_email, p_ugt):

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

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

This looks fine to me.
Do we want tests to confirm this doesn't regress again?

Do we want tests to confirm this doesn't regress again?

+1, I'll see what I can do

rebased onto 4cc3b53
6 years ago

1 new commit added

  • Make flake8 happy
6 years ago

Tests are passing, let's get this in :)

Pull-Request has been merged by pingou
6 years ago
Changes Summary 3
+5 -4
file changed
pagure/lib/tasks.py
+10 -6
file changed
pagure/templates/issue.html
+69 -0
file changed
tests/test_pagure_flask_ui_issues.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.