From 357aa4327dbf94b5d8a9ae2302645dee51eacacb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 07 2017 15:01:55 +0000 Subject: [PATCH 1/8] Allow dropping all priority information of an issue --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 50c9fa1..8e31e08 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1405,7 +1405,7 @@ def new_tag(session, tag_name, tag_description, tag_color, project_id): def edit_issue(session, issue, ticketfolder, user, repo=None, title=None, content=None, status=None, close_status=-1, - priority=None, milestone=-1, private=False): + priority=-1, milestone=-1, private=False): ''' Edit the specified issue. ''' user_obj = get_user(session, user) @@ -1443,7 +1443,7 @@ def edit_issue(session, issue, ticketfolder, user, repo=None, if old_status: msg += ' (was: %s)' % old_status messages.append(msg) - if priority: + if priority != -1: try: priority = int(priority) except: From 980198928367804b468cd8e1651b8d22bef27dfa Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 07 2017 15:01:55 +0000 Subject: [PATCH 2/8] Couple of fixes around milestones and priorities This commit does a couple of fixes: a) forward issue.milestone to the backend when calling edit_issue() so that we do not remove the milestone to add it back shortly after (triggering two fedmsg notifications one of which is invalid) b) Allow the priority to be None, so all priority information of a ticket can be removed and fix making the current priority the selected one in the form displayed in the UI --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index b436ff4..138d097 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -219,6 +219,7 @@ def update_issue(repo, issueid, username=None, namespace=None): issue=issue, status=new_status, close_status=close_status, + milestone=issue.milestone, private=issue.private, user=flask.g.fas_user.username, ticketfolder=APP.config['TICKETS_FOLDER'] @@ -964,7 +965,9 @@ def view_issue(repo, issueid, username=None, namespace=None): close_status=repo.close_status, ) form.status.data = issue.status - form.priority.data = issue.priority + form.priority.data = '%s' % issue.priority + # issue.priority is an int that we need to convert to string as the form + # relies on string form.milestone.data = issue.milestone form.private.data = issue.private form.close_status.data = '' From 16b1892133fd0932a9bdb2e358fb2a1ca738887d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 07 2017 15:01:56 +0000 Subject: [PATCH 3/8] Fix the unit-tests related to the change in the custom fields --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 28b31e1..ba93efd 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -1960,13 +1960,31 @@ class PagureFlaskApiIssuetests(tests.Modeltests): for key in repo.issue_keys: # Check that the bugzilla field correctly had its data removed if key.name == "bugzilla": - self.assertTrue(key.data is None) + self.assertIsNone(key.data) # Check that the reviewstatus list field still has its list if key.name == "reviewstatus": self.assertEqual( sorted(key.data), ['ack', 'nack', 'needs review']) + # Check that not setting the value on a non-existing custom field + # changes nothing + output = self.app.post( + '/api/0/test/issue/1/custom/bugzilla', headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.data) + self.assertDictEqual( + data, + { + 'message': 'No changes' + } + ) + + repo = pagure.lib.get_project(self.session, 'test') + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(issue.other_fields, []) + self.assertEqual(len(issue.other_fields), 0) + # Invalid value output = self.app.post( '/api/0/test/issue/1/custom/bugzilla', headers=headers, From 605a318e61bee1b3a3a7a7e822045a4c3e19f151 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 07 2017 15:01:56 +0000 Subject: [PATCH 4/8] Give a default value to exclude_groups in user_watch_list This makes the unit-tests pass for this method without changing them --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 8e31e08..7178e5d 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3328,7 +3328,7 @@ def is_watching(session, user, reponame, repouser=None, namespace=None): return False -def user_watch_list(session, user, exclude_groups): +def user_watch_list(session, user, exclude_groups=None): ''' Returns list of all the projects which the user is watching ''' user_obj = search_user(session, username=user) From c00e94d838089c92cc29b65a28e43148f98cb780 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 07 2017 15:01:56 +0000 Subject: [PATCH 5/8] Adjust the unit-tests for the change we made around branch name --- diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 005f3b6..b52c9a8 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -3362,7 +3362,7 @@ index 0000000..fb7093d 'data-toggle="tooltip">feature/foo', output.data) self.assertIn( - '
feature/foo', output.data) self.assertNotIn( - ' Date: Feb 07 2017 15:01:56 +0000 Subject: [PATCH 6/8] Only update the custom field if there is a value to give it --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 7178e5d..96810f2 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3568,12 +3568,14 @@ def set_custom_key_value(session, issue, key, value): current_field.value = value updated = True else: - current_field = model.IssueValues( - issue_uid=issue.uid, - key_id=key.id, - value=value, - ) - if value: + if value is None or value == '': + delete = True + else: + current_field = model.IssueValues( + issue_uid=issue.uid, + key_id=key.id, + value=value, + ) updated = True if not delete: From 6b03d359d1a7a0f36a8dd74f85d9e5766e8c21ed Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 07 2017 15:01:56 +0000 Subject: [PATCH 7/8] Make our unit-tests compatible with flask-wtf >= 0.14.0 --- diff --git a/tests/test_pagure_flask_form.py b/tests/test_pagure_flask_form.py index 46b492d..916c3f8 100644 --- a/tests/test_pagure_flask_form.py +++ b/tests/test_pagure_flask_form.py @@ -62,7 +62,6 @@ class PagureFlaskFormTests(tests.Modeltests): with pagure.APP.test_request_context(method='POST'): form = pagure.forms.ConfirmationForm() data = form.csrf_token.current_token - _, hmac_csrf = data.split('##', 1) # CSRF token expired if hasattr(flask_wtf, '__version__') and \ @@ -72,7 +71,21 @@ class PagureFlaskFormTests(tests.Modeltests): expires = ( datetime.datetime.now() - datetime.timedelta(minutes=1) ).strftime('%Y%m%d%H%M%S') - form.csrf_token.data = '%s##%s' % (expires, hmac_csrf) + + # Change the CSRF format + if hasattr(flask_wtf, '__version__') and \ + tuple([int(e) for e in flask_wtf.__version__.split('.')] + ) >= (0,14,0): + import itsdangerous + timestamp = itsdangerous.base64_encode( + itsdangerous.int_to_bytes(int(expires))) + print '*', data + part1, _, part2 = data.split('.', 2) + form.csrf_token.data = '.'.join([part1, timestamp, part2]) + else: + _, hmac_csrf = data.split('##', 1) + form.csrf_token.data = '%s##%s' % (expires, hmac_csrf) + self.assertFalse(form.validate_on_submit()) def test_csrf_form_w_unexpiring_input(self): @@ -81,9 +94,15 @@ class PagureFlaskFormTests(tests.Modeltests): with pagure.APP.test_request_context(method='POST'): form = pagure.forms.ConfirmationForm() data = form.csrf_token.current_token - _, hmac_csrf = data.split('##', 1) - # CSRF can no longer expire, they have no expiration info - form.csrf_token.data = '##%s' % hmac_csrf + + if hasattr(flask_wtf, '__version__') and \ + tuple([int(e) for e in flask_wtf.__version__.split('.')] + ) >= (0,14,0): + form.csrf_token.data = data + else: + _, hmac_csrf = data.split('##', 1) + # CSRF can no longer expire, they have no expiration info + form.csrf_token.data = '##%s' % hmac_csrf self.assertTrue(form.validate_on_submit()) From 6d5c21e9d2246bb043b880edb2b1b3e4636ac9f4 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 07 2017 15:01:56 +0000 Subject: [PATCH 8/8] Drop the path variable that isn't used anywhere --- diff --git a/pagure/templates/file.html b/pagure/templates/file.html index b8f8166..1268461 100644 --- a/pagure/templates/file.html +++ b/pagure/templates/file.html @@ -74,13 +74,8 @@ {% for file in filename.split('/') %} - {% if loop.first %} - {% set path = file %} - {% else %} - {% set path = path + '/' + file %} - {% endif %} - {% if loop.index != loop.length %}