From fc17a87fe52e09b2df31055f5344a4b19d727603 Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Feb 01 2017 13:52:52 +0000 Subject: [PATCH 1/3] Delete a custom field value if it was submitted with empty contents Fixes: pagure-importer#107 Signed-off-by: Patrick Uiterwijk --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 6a292fc..6e4b833 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -3517,7 +3517,7 @@ def set_custom_key_value(session, issue, key, value): if current_field: if current_field.key.key_type == 'boolean': value = value or False - if value is None: + if value is None or value == '': session.delete(current_field) updated = True delete = True @@ -3525,16 +3525,15 @@ def set_custom_key_value(session, issue, key, value): current_field.value = value updated = True else: - if not value: - raise pagure.exceptions.PagureException( - 'No value given to this new custom field: %s' % key.name + if value is None or value == '': + delete = True + else: + current_field = model.IssueValues( + issue_uid=issue.uid, + key_id=key.id, + value=value, ) - current_field = model.IssueValues( - issue_uid=issue.uid, - key_id=key.id, - value=value, - ) - updated = True + updated = True if not delete: session.add(current_field) From 31c5eeb3916620756731943dda1542d3e5734aa3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 01 2017 13:52:52 +0000 Subject: [PATCH 2/3] Fix resetting the value of a boolean custom field --- diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index b5ab9dc..d0933d9 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -396,7 +396,8 @@ {%- if field.key_type == 'boolean' %} type="checkbox" {% endif %} class="form-control" name="{{ field.name }}" id="{{ field.name }}" {%- if field.name in knowns_keys %} - {% if field.key_type == 'boolean' %} checked + {% if field.key_type == 'boolean'%} + {% if knowns_keys[field.name].value in ['true', 'on', '1'] %}checked{% endif %} {% else %} value="{{ knowns_keys[field.name].value }}" {% endif %} {%- endif -%} /> diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index f4fba3f..13f79aa 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -283,21 +283,20 @@ def update_issue(repo, issueid, username=None, namespace=None): # Update the custom keys/fields for key in repo.issue_keys: value = flask.request.form.get(key.name) - if value: - if key.key_type == 'link': - links = value.split(',') - for link in links: - link = link.replace(' ', '') - if not urlpattern.match(link): - flask.abort( - 400, - 'Meta-data "link" field ' - '(%s) has invalid url (%s) ' % - (key.name, link)) - messages.add( - pagure.lib.set_custom_key_value( - SESSION, issue, key, value) - ) + if key.key_type == 'link': + links = value.split(',') + for link in links: + link = link.replace(' ', '') + if not urlpattern.match(link): + flask.abort( + 400, + 'Meta-data "link" field ' + '(%s) has invalid url (%s) ' % + (key.name, link)) + messages.add( + pagure.lib.set_custom_key_value( + SESSION, issue, key, value) + ) # Update ticket this one depends on messages.union(set(pagure.lib.update_dependency_issue( From 59b17116c4dbeef688715d1a46605fda4e83a73f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 01 2017 13:52:52 +0000 Subject: [PATCH 3/3] Fix the unit-tests for the change made to custom fields Since we allowed resetting the content of a custom field a test that used to fail is now passing, and this is a good thing. --- diff --git a/tests/test_pagure_flask_api_issue.py b/tests/test_pagure_flask_api_issue.py index 5ff71b6..6b9837c 100644 --- a/tests/test_pagure_flask_api_issue.py +++ b/tests/test_pagure_flask_api_issue.py @@ -1917,19 +1917,23 @@ class PagureFlaskApiIssuetests(tests.Modeltests): self.assertEqual( sorted(key.data), ['ack', 'nack', 'needs review']) - # No value specified while we try to create the field + # Reset the value of the field to None output = self.app.post( '/api/0/test/issue/1/custom/bugzilla', headers=headers) - self.assertEqual(output.status_code, 400) + self.assertEqual(output.status_code, 200) data = json.loads(output.data) self.assertDictEqual( data, { - "error": "No value given to this new custom field: bugzilla", - "error_code": "ENOCODE", + 'message': 'Custom field adjusted' } ) + 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, @@ -1945,6 +1949,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) + 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) @@ -1962,6 +1967,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) + repo = pagure.lib.get_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) self.assertEqual(len(issue.other_fields), 1) self.assertEqual(issue.other_fields[0].key.name, 'bugzilla') @@ -1981,6 +1987,7 @@ class PagureFlaskApiIssuetests(tests.Modeltests): } ) + 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)