This is an automated email from the ASF dual-hosted git repository. brondsem pushed a commit to branch db/8375 in repository https://gitbox.apache.org/repos/asf/allura.git
commit 727d750b938bcffd63d8a677a4325673af9d17a9 Author: Dave Brondsema <d...@brondsema.net> AuthorDate: Wed Aug 26 16:48:56 2020 -0400 [#8375] tracker fixes --- Allura/allura/tests/decorators.py | 11 +++++++++++ ForgeTracker/forgetracker/import_support.py | 4 ++-- ForgeTracker/forgetracker/model/ticket.py | 15 +++++++++------ ForgeTracker/forgetracker/tests/functional/test_root.py | 16 +++++++++------- .../forgetracker/tests/unit/test_ticket_model.py | 16 ++++++++-------- ForgeTracker/forgetracker/tracker_main.py | 4 ++-- 6 files changed, 41 insertions(+), 25 deletions(-) diff --git a/Allura/allura/tests/decorators.py b/Allura/allura/tests/decorators.py index 6a72710..28f6bb6 100644 --- a/Allura/allura/tests/decorators.py +++ b/Allura/allura/tests/decorators.py @@ -21,7 +21,9 @@ import sys import re from functools import wraps import contextlib +from six.moves.urllib.parse import parse_qs +from nose.tools import assert_equal from ming.orm.ormsession import ThreadLocalORMSession from tg import tmpl_context as c from mock import patch @@ -220,3 +222,12 @@ def assert_logmsg_and_no_warnings_or_errors(logs, msg): if r.levelno > logging.INFO: raise AssertionError('unexpected log {} {}'.format(r.levelname, r.getMessage())) assert found_msg, 'Did not find {} in logs: {}'.format(msg, '\n'.join([r.getMessage() for r in logs.records])) + + +def assert_equivalent_urls(url1, url2): + base1, _, qs1 = url1.partition('?') + base2, _, qs2 = url2.partition('?') + assert_equal( + (base1, parse_qs(qs1)), + (base2, parse_qs(qs2)), + ) diff --git a/ForgeTracker/forgetracker/import_support.py b/ForgeTracker/forgetracker/import_support.py index 3a482c6..d643134 100644 --- a/ForgeTracker/forgetracker/import_support.py +++ b/ForgeTracker/forgetracker/import_support.py @@ -73,11 +73,11 @@ class ResettableStream(object): def read(self, size=-1): self._read_header() - data = '' + data = b'' if self.buf_pos < self.stream_pos: data = self.buf.read(size) self.buf_pos += len(data) - if len(data) == size: + if len(data) == size or size == -1: return data size -= len(data) diff --git a/ForgeTracker/forgetracker/model/ticket.py b/ForgeTracker/forgetracker/model/ticket.py index 36b252f..0efbdca 100644 --- a/ForgeTracker/forgetracker/model/ticket.py +++ b/ForgeTracker/forgetracker/model/ticket.py @@ -390,8 +390,7 @@ class Globals(MappedClass): for ticket in tickets: message = '' if labels: - values['labels'] = self.append_new_labels( - ticket.labels, labels.split(',')) + values['labels'] = self.append_new_labels(ticket.labels, labels.split(',')) for k, v in sorted(six.iteritems(values)): if k == 'deleted': if v: @@ -533,9 +532,13 @@ class Globals(MappedClass): return filtered def append_new_labels(self, old_labels, new_labels): - old_labels = set(old_labels) - new_labels = set(l.strip() for l in new_labels) - return list(old_labels | new_labels) + # append without duplicating any. preserve order + labels = old_labels[:] # make copy to ensure no edits to possible underlying model field + for l in new_labels: + l = l.strip() + if l not in old_labels: + labels.append(l) + return labels class TicketHistory(Snapshot): @@ -850,7 +853,7 @@ class Ticket(VersionedArtifact, ActivityObject, VotableArtifact): role_developer = ProjectRole.by_name('Developer') role_creator = ProjectRole.by_user(self.reported_by, upsert=True) if self.reported_by else None def _allow_all(role, perms): - return [ACE.allow(role._id, perm) for perm in perms] + return [ACE.allow(role._id, perm) for perm in sorted(perms)] # sorted just for consistency (for tests) # maintain existing access for developers and the ticket creator, # but revoke all access for everyone else acl = _allow_all(role_developer, security.all_allowed(self, role_developer)) diff --git a/ForgeTracker/forgetracker/tests/functional/test_root.py b/ForgeTracker/forgetracker/tests/functional/test_root.py index eb341cc..bb6ff6f 100644 --- a/ForgeTracker/forgetracker/tests/functional/test_root.py +++ b/ForgeTracker/forgetracker/tests/functional/test_root.py @@ -45,6 +45,7 @@ from tg import tmpl_context as c from tg import app_globals as g from tg import config +from allura.tests.decorators import assert_equivalent_urls from allura.tests.test_globals import squish_spaces from alluratest.controller import TestController, setup_basic_test from allura import model as M @@ -1623,7 +1624,7 @@ class TestFunctionalController(TrackerTestController): edit_link = response.html.find('a', {'title': 'Bulk Edit'}) expected_link = "/p/test/bugs/edit/?q=%21status%3Awont-fix+%26%26+%21status%3Aclosed"\ "&sort=snippet_s+asc&limit=25&filter=&page=0" - assert_equal(expected_link, edit_link['href']) + assert_equivalent_urls(expected_link, edit_link['href']) response = self.app.get(edit_link['href']) ticket_rows = response.html.find('tbody', {'class': 'ticket-list'}) assert_in('test first ticket', ticket_rows.text) @@ -1646,7 +1647,7 @@ class TestFunctionalController(TrackerTestController): assert_in('test third ticket', ticket_rows.text) edit_link = response.html.find('a', {'title': 'Bulk Edit'}) expected_link = "/p/test/bugs/edit/?q=_milestone%3A1.0&sort=ticket_num_i+asc&limit=25&filter=&page=0" - assert_equal(expected_link, edit_link['href']) + assert_equivalent_urls(expected_link, edit_link['href']) response = self.app.get(edit_link['href']) ticket_rows = response.html.find('tbody', {'class': 'ticket-list'}) assert_in('test first ticket', ticket_rows.text) @@ -1667,7 +1668,7 @@ class TestFunctionalController(TrackerTestController): assert_false('test third ticket' in ticket_rows.text) edit_link = response.html.find('a', {'title': 'Bulk Edit'}) expected_link = "/p/test/bugs/edit/?q=status%3Aopen&limit=25&filter=%7B%7D&page=0" - assert_equal(expected_link, edit_link['href']) + assert_equivalent_urls(expected_link, edit_link['href']) response = self.app.get(edit_link['href']) ticket_rows = response.html.find('tbody', {'class': 'ticket-list'}) assert_in('test first ticket', ticket_rows.text) @@ -1983,19 +1984,19 @@ class TestFunctionalController(TrackerTestController): # invalid vote r = self.app.post('/bugs/1/vote', dict(vote='invalid')) expected_resp = json.dumps(dict(status='error', votes_up=0, votes_down=0, votes_percent=0)) - assert r.response.content == expected_resp + assert_equal(r.response.text, expected_resp) # vote up r = self.app.post('/bugs/1/vote', dict(vote='u')) expected_resp = json.dumps(dict(status='ok', votes_up=1, votes_down=0, votes_percent=100)) - assert r.response.content == expected_resp + assert_equal(r.response.text, expected_resp) # vote down by another user r = self.app.post('/bugs/1/vote', dict(vote='d'), extra_environ=dict(username=str('test-user-0'))) expected_resp = json.dumps(dict(status='ok', votes_up=1, votes_down=1, votes_percent=50)) - assert r.response.content == expected_resp + assert_equal(r.response.text, expected_resp) # make sure that on the page we see the same result r = self.app.get('/bugs/1/') @@ -2949,7 +2950,8 @@ class TestHelpTextOptions(TrackerTestController): assert len(r.html.findAll(attrs=dict(id='new-ticket-help-msg'))) == 0 -class test_show_default_fields(TrackerTestController): +class TestShowDefaultFields(TrackerTestController): + def test_show_default_fields(self): r = self.app.get('/admin/bugs/fields') assert '<td>Ticket Number</td> <td><input type="checkbox" name="ticket_num" checked ></td>' in r diff --git a/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py b/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py index 149cc8a..f1ca974 100644 --- a/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py +++ b/ForgeTracker/forgetracker/tests/unit/test_ticket_model.py @@ -139,17 +139,17 @@ class TestTicketModel(TrackerTestWithModel): t.private = True assert_equal(t.acl, [ - ACE.allow(role_developer, 'save_searches'), - ACE.allow(role_developer, 'read'), ACE.allow(role_developer, 'create'), - ACE.allow(role_developer, 'update'), - ACE.allow(role_developer, 'unmoderated_post'), - ACE.allow(role_developer, 'post'), - ACE.allow(role_developer, 'moderate'), ACE.allow(role_developer, 'delete'), - ACE.allow(role_creator, 'read'), - ACE.allow(role_creator, 'post'), + ACE.allow(role_developer, 'moderate'), + ACE.allow(role_developer, 'post'), + ACE.allow(role_developer, 'read'), + ACE.allow(role_developer, 'save_searches'), + ACE.allow(role_developer, 'unmoderated_post'), + ACE.allow(role_developer, 'update'), ACE.allow(role_creator, 'create'), + ACE.allow(role_creator, 'post'), + ACE.allow(role_creator, 'read'), ACE.allow(role_creator, 'unmoderated_post'), DENY_ALL]) assert has_access(t, 'read', user=admin)() diff --git a/ForgeTracker/forgetracker/tracker_main.py b/ForgeTracker/forgetracker/tracker_main.py index 7588ecc..d3228e6 100644 --- a/ForgeTracker/forgetracker/tracker_main.py +++ b/ForgeTracker/forgetracker/tracker_main.py @@ -864,7 +864,7 @@ class RootController(BaseController, FeedController): else: feed = FG.Rss201rev2Feed(**d) for t in result['tickets']: - url = h.absurl(t.url().encode('utf-8')) + url = h.absurl(t.url()) feed_kwargs = dict(title=t.summary, link=url, pubdate=t.mod_date, @@ -1920,7 +1920,7 @@ class MilestoneController(BaseController): else: raise exc.HTTPNotFound() for m in fld.milestones: - if m.name == unquote(milestone).decode('utf-8'): + if m.name == six.ensure_text(unquote(milestone)): break else: raise exc.HTTPNotFound()