pagure

Clone

#1627 Allow dropping of assignments for an issue (Fixes: #1255)
Merged 7 years ago by pingou. Opened 7 years ago by pfrields.
pfrields/pagure issue1255  into  master

file modified
+4 -1 
@@ -825,7 +825,7 @@ 
 

  

      form = pagure.forms.AssignIssueForm(csrf_enabled=False)
 

      if form.validate_on_submit():
 

-         assignee = form.assignee.data
 

+         assignee = form.assignee.data or None
 

          try:
 

              # New comment
 

              message = pagure.lib.add_issue_assignee(
 
@@ -837,6 +837,9 @@ 
 

              )
 

              SESSION.commit()
 

              output['message'] = message
 

+         except pagure.exceptions.PagureException as err:  # pragma: no cover
 

+             raise pagure.exceptions.APIError(
 

+                 400, error_code=APIERROR.ENOCODE, error=str(err))
 

          except SQLAlchemyError as err:  # pragma: no cover
 

              SESSION.rollback()
 

              APP.logger.exception(err)

file modified
+1 -1 
@@ -416,7 +416,7 @@ 
 

      ''' Form to assign an user to an issue. '''
 

      assignee = wtforms.TextField(
 

          'Assignee <span class="error">*</span>',
 

-         [wtforms.validators.Required()]
 

+         [wtforms.validators.Optional()]
 

      )

file modified
+3 -3 
@@ -384,7 +384,7 @@ 
 

      ''' Add an assignee to an issue, in other words, assigned an issue. '''
 

      user_obj = get_user(session, user)
 

  

-     if assignee is None and issue.assignee is not None:
 

+     if not assignee and issue.assignee is not None:
 

          issue.assignee_id = None
 

          issue.last_updated = datetime.datetime.utcnow()
 

          session.add(issue)
 
@@ -413,8 +413,8 @@ 
 

                  {'unassigned': '-'}))
 

  

          return 'Assignee reset'
 

-     elif assignee is None and issue.assignee is None:
 

-         return
 

+     elif not assignee and issue.assignee is None:
 

+         return 'Nothing to change'
 

  

      # Validate the assignee
 

      assignee_obj = get_user(session, assignee)

file modified
+66 -25 
@@ -192,6 +192,12 @@ 
 

                      assignee=issue.assignee.username) }}">
 

                      {{ issue.assignee.username }}
 

                    </a>
 

+ 		{% if authenticated and (issue.assignee.username == g.fas_user.username) %}
 

+ 		  <button class="btn btn-sm pull-xs-right" id="drop-btn"
 

+ 			  title="drop the assignment of this issue">
 

+ 		    Drop
 

+ 		  </button>
 

+ 		{% endif %}
 

                {% else %}
 

                  unassigned
 

                {% endif %}
 
@@ -698,6 +704,60 @@ 
 

  </script>
 

  {% endif %}
 

  <script type="text/javascript">
 

+ 

+ {% if authenticated and g.repo_admin %}
 

+ function take_issue(){
 

+   var _url = "{{ url_for('api_ns.api_assign_issue', repo=repo.name, username=username, issueid=issueid) }}";
 

+   var _data = {assignee: "{{ g.fas_user.username }}"};
 

+   $.post (_url, _data ).done(
 

+     function(data) {
 

+       var _user_url = '\n<a href="{{ url_for("view_issues", repo=repo.name, username=username) }}'
 

+         + '?assignee={{ g.fas_user.username }}">'
 

+         + '{{ g.fas_user.username }}</a>'
 

+         + '<button class="btn btn-sm pull-xs-right" id="drop-btn" title="drop the assignment of this issue">Drop</button>';
 

+       $('#assignee_plain').html(_user_url);
 

+       $('#assignee').val("{{ g.fas_user.username }}");
 

+       setup_btn_take_drop();
 

+     }
 

+   )
 

+   return false;
 

+ }
 

+ {% endif %}
 

+ 

+ {% if authenticated and (
 

+     g.repo_admin
 

+     or issue.user.user == g.fas_user.username
 

+     or issue.assignee.user == g.fas_user.username) %}
 

+ function drop_issue(){
 

+   var _url = "{{ url_for(
 

+       'api_ns.api_assign_issue', repo=repo.name, username=username, issueid=issueid
 

+       ) }}";
 

+   var _data = {assignee: ""};
 

+   $.post( _url, _data ).done(
 

+     function(data) {
 

+       var _user_url = '\nunassigned'
 

+         + '<button class="btn btn-sm pull-xs-right" id="take-btn" title="assign this issue to you">Take</button>';
 

+       $('#assignee_plain').html(_user_url);
 

+       $('#assignee').val("");
 

+       setup_btn_take_drop();
 

+     }
 

+   )
 

+   return false;
 

+ }
 

+ {% endif %}
 

+ 

+ function setup_btn_take_drop(){
 

+   {% if authenticated and g.repo_admin %}
 

+   $("#take-btn").click(take_issue)
 

+   {% endif %}
 

+   {% if authenticated and (
 

+     g.repo_admin
 

+     or issue.user.user == g.fas_user.username
 

+     or issue.assignee.user == g.fas_user.username) %}
 

+   $("#drop-btn").click(drop_issue);
 

+   {% endif %}
 

+ }
 

+ 

  $( document ).ready(function() {
 

  

  
@@ -869,31 +929,12 @@ 
 

        }
 

      }
 

    );
 

- 

-   {% if authenticated and g.repo_admin %}
 

-     $("#take-btn").click(function(){
 

-       var _url = "{{ url_for(
 

-         'api_ns.api_assign_issue',
 

-         repo=repo.name,
 

-         username=username,
 

-         namespace=repo.namespace,
 

-         issueid=issueid
 

-         ) }}";
 

-       var _data = {assignee: "{{ g.fas_user.username }}"};
 

-       $.post( _url, _data ).done(
 

-         function(data) {
 

-           var _user_url = '\n<a href="{{ url_for("view_issues",
 

-             repo=repo.name,
 

-             username=username,
 

-             namespace=repo.namespace) }}'
 

-             + '?assignee={{ g.fas_user.username }}">'
 

-             + '{{ g.fas_user.username }}</a>';
 

-           $('#assignee_plain').html(_user_url);
 

-           $('#assignee').val("{{ g.fas_user.username }}");
 

-         }
 

-       )
 

-       return false;
 

-     });
 

+   

+   {% if authenticated and (
 

+     g.repo_admin
 

+     or issue.user.user == g.fas_user.username
 

+     or issue.assignee.user == g.fas_user.username) %}
 

+   setup_btn_take_drop();
 

    {% endif %}
 

  

    {% if authenticated %}

file modified
+56 -14 
@@ -1443,34 +1443,76 @@ 
 

          issue = pagure.lib.search_issues(self.session, repo, issueid=1)
 

          self.assertEqual(len(issue.comments), 0)
 

  

+         # No change
 

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

+         issue = pagure.lib.search_issues(self.session, repo, issueid=1)
 

+         self.assertEqual(issue.status, 'Open')
 

+ 

          data = {
 

-             'title': 'test issue',
 

+             'assignee': 'pingou',
 

          }
 

  

-         # Incomplete request
 

+         # Valid request
 

          output = self.app.post(
 

              '/api/0/test/issue/1/assign', data=data, headers=headers)
 

-         self.assertEqual(output.status_code, 400)
 

+         self.assertEqual(output.status_code, 200)
 

          data = json.loads(output.data)
 

          self.assertDictEqual(
 

              data,
 

-             {
 

-               "error": "Invalid or incomplete input submited",
 

-               "error_code": "EINVALIDREQ",
 

-               "errors": {"assignee": ["This field is required."]}
 

-             }
 

+             {'message': 'Issue assigned'}
 

          )
 

  

-         # No change
 

+         # Un-assign
 

+         output = self.app.post(
 

+             '/api/0/test/issue/1/assign', data=data, headers=headers)
 

+         self.assertEqual(output.status_code, 200)
 

+         data = json.loads(output.data)
 

+         self.assertDictEqual(
 

+             data,
 

+             {'message': 'Assignee reset'}
 

+         )
 

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

          issue = pagure.lib.search_issues(self.session, repo, issueid=1)
 

-         self.assertEqual(issue.status, 'Open')
 

+         self.assertEqual(issue.assignee, None)
 

  

-         data = {
 

-             'assignee': 'pingou',
 

-         }
 

+         # Un-assign
 

+         data = {'assignee': None}
 

+         output = self.app.post(
 

+             '/api/0/test/issue/1/assign', data=data, headers=headers)
 

+         self.assertEqual(output.status_code, 200)
 

+         data = json.loads(output.data)
 

+         self.assertDictEqual(
 

+             data,
 

+             {'message': 'Nothing to change'}
 

+         )
 

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

+         issue = pagure.lib.search_issues(self.session, repo, issueid=1)
 

+         self.assertEqual(issue.assignee, None)
 

  

-         # Valid request
 

+         # Re-assign for the rest of the tests
 

+         data = {'assignee': 'pingou'}
 

+         output = self.app.post(
 

+             '/api/0/test/issue/1/assign', data=data, headers=headers)
 

+         self.assertEqual(output.status_code, 200)
 

+         data = json.loads(output.data)
 

+         self.assertDictEqual(
 

+             data,
 

+             {'message': 'Issue assigned'}
 

+         )
 

+ 

+         # Un-assign
 

+         data = {'assignee': ''}
 

+         output = self.app.post(
 

+             '/api/0/test/issue/1/assign', data=data, headers=headers)
 

+         self.assertEqual(output.status_code, 200)
 

+         data = json.loads(output.data)
 

+         self.assertDictEqual(
 

+             data,
 

+             {'message': 'Assignee reset'}
 

+         )
 

+ 

+         # Re-assign for the rest of the tests
 

+         data = {'assignee': 'pingou'}
 

          output = self.app.post(
 

              '/api/0/test/issue/1/assign', data=data, headers=headers)
 

          self.assertEqual(output.status_code, 200)

Signed-off-by: Paul W. Frields stickster@gmail.com

Unit test coming shortly.

Indentation looks odd here, mind pushing it a little to the right?

I know it wasn't there before, but I'd propose: return 'Nothing to change'

Ok, so I see only one test failing (jenkins is drunk), I propose the following change:

diff --git a/ pagure/api/issue.py b/ pagure/api/issue.py
index 1ef6edf..41ed130 100644
--- a/ pagure/api/issue.py      
+++ b/ pagure/api/issue.py      
@@ -825,7 +825,7 @@ def api_assign_issue(repo, issueid, username=None, namespace=None):

     form = pagure.forms.AssignIssueForm(csrf_enabled=False)
     if form.validate_on_submit():
-        assignee = form.assignee.data
+        assignee = form.assignee.data or None
         try:
             # New comment
             message = pagure.lib.add_issue_assignee(
diff --git a/ pagure/lib/__init__.py b/ pagure/lib/__init__.py
index 1a1ea44..a4fb42c 100644
--- a/ pagure/lib/__init__.py   
+++ b/ pagure/lib/__init__.py   
@@ -414,7 +414,7 @@ def add_issue_assignee(session, issue, assignee, user, ticketfolder,

         return 'Assignee reset'
     elif not assignee and issue.assignee is None:
-        return
+        return 'Nothing to change'

     # Validate the assignee
     assignee_obj = get_user(session, assignee)
diff --git a/ tests/test_pagure_flask_api_issue.py b/ tests/test_pagure_flask_api_issue.py
index e044330..71c850c 100644
--- a/ tests/test_pagure_flask_api_issue.py     
+++ b/ tests/test_pagure_flask_api_issue.py     
@@ -1443,34 +1443,76 @@ class PagureFlaskApiIssuetests(tests.Modeltests):
         issue = pagure.lib.search_issues(self.session, repo, issueid=1)
         self.assertEqual(len(issue.comments), 0)

+        # No change
+        repo = pagure.lib.get_project(self.session, 'test')
+        issue = pagure.lib.search_issues(self.session, repo, issueid=1)
+        self.assertEqual(issue.status, 'Open')
+
         data = {
-            'title': 'test issue',
+            'assignee': 'pingou',
         }

-        # Incomplete request
+        # Valid request
         output = self.app.post(
             '/api/0/test/issue/1/assign', data=data, headers=headers)
-        self.assertEqual(output.status_code, 400)
+        self.assertEqual(output.status_code, 200)
         data = json.loads(output.data)
         self.assertDictEqual(
             data,
-            {
-              "error": "Invalid or incomplete input submited",
-              "error_code": "EINVALIDREQ",
-              "errors": {"assignee": ["This field is required."]}
-            }
+            {'message': 'Issue assigned'}
         )

-        # No change
+        # Un-assign
+        output = self.app.post(
+            '/api/0/test/issue/1/assign', data=data, headers=headers)
+        self.assertEqual(output.status_code, 200)
+        data = json.loads(output.data)
+        self.assertDictEqual(
+            data,
+            {'message': 'Assignee reset'}
+        )
         repo = pagure.lib.get_project(self.session, 'test')
         issue = pagure.lib.search_issues(self.session, repo, issueid=1)
-        self.assertEqual(issue.status, 'Open')
+        self.assertEqual(issue.assignee, None)

-        data = {
-            'assignee': 'pingou',
-        }
+        # Un-assign
+        data = {'assignee': None}
+        output = self.app.post(
+            '/api/0/test/issue/1/assign', data=data, headers=headers)
+        self.assertEqual(output.status_code, 200)
+        data = json.loads(output.data)
+        self.assertDictEqual(
+            data,
+            {'message': 'Nothing to change'}
+        )
+        repo = pagure.lib.get_project(self.session, 'test')
+        issue = pagure.lib.search_issues(self.session, repo, issueid=1)
+        self.assertEqual(issue.assignee, None)

-        # Valid request
+        # Re-assign for the rest of the tests
+        data = {'assignee': 'pingou'}
+        output = self.app.post(
+            '/api/0/test/issue/1/assign', data=data, headers=headers)
+        self.assertEqual(output.status_code, 200)
+        data = json.loads(output.data)
+        self.assertDictEqual(
+            data,
+            {'message': 'Issue assigned'}
+        )
+
+        # Un-assign
+        data = {'assignee': ''}
+        output = self.app.post(
+            '/api/0/test/issue/1/assign', data=data, headers=headers)
+        self.assertEqual(output.status_code, 200)
+        data = json.loads(output.data)
+        self.assertDictEqual(
+            data,
+            {'message': 'Assignee reset'}
+        )
+
+        # Re-assign for the rest of the tests
+        data = {'assignee': 'pingou'}
         output = self.app.post(
             '/api/0/test/issue/1/assign', data=data, headers=headers)
         self.assertEqual(output.status_code, 200)

Basically, we :
- Set the assignee to None if the content of the data is ''
- Be explicit when nothing changed
- remove the old check that a POST request without assignee would not work
- check that a POST request without assignee does reset the assignee
- check that a follow-up POST request with assignee as None does not change anything
- re-assign the issue
- un-assign again, but in a different way
- re-assign again so the rest of the tests keep working :)

rebased
7 years ago

rebased
7 years ago

rebased
7 years ago

OK, the test is now included. For what it's worth, I got about 75% close on my own! :grin:

Alright, from my side all I can see needed is a final rebase :)

ping?

Knowing that Paul is busy this week and afk after, I'm going to merge this one manually.

Thanks for working on it Paul! :)

Commit 538c06a fixes this pull-request

Pull-Request has been merged by pingou@pingoured.fr
7 years ago
Metadata
None
No Tags
Flags
jenkins
failure
Build failed
7 years ago
Changes Summary 5
+4 -1
file changed
pagure/api/issue.py
+1 -1
file changed
pagure/forms.py
+3 -3
file changed
pagure/lib/__init__.py
+66 -25
file changed
pagure/templates/issue.html
+56 -14
file changed
tests/test_pagure_flask_api_issue.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.