Make reviewer theme queue orderable (w/ default order by due date) (#22583)
* Make reviewer theme queue orderable (w/ default order by due date)
This commit is contained in:
Mathieu Pillard
GitHub
Родитель
cedcc4917e
Коммит
813b72ba2f
|
|
@ -147,7 +147,7 @@ def initial(form):
|
|||
return data
|
||||
|
||||
|
||||
def check_links(expected, elements, selected=None, verify=True):
|
||||
def check_links(expected, elements, selected=None):
|
||||
"""Useful for comparing an `expected` list of links against PyQuery
|
||||
`elements`. Expected format of links is a list of tuples, like so:
|
||||
|
||||
|
|
@ -159,9 +159,6 @@ def check_links(expected, elements, selected=None, verify=True):
|
|||
|
||||
If you'd like to check if a particular item in the list is selected,
|
||||
pass as `selected` the title of the link.
|
||||
|
||||
Links are verified by default.
|
||||
|
||||
"""
|
||||
for idx, item in enumerate(expected):
|
||||
# List item could be `(text, link)`.
|
||||
|
|
@ -171,19 +168,19 @@ def check_links(expected, elements, selected=None, verify=True):
|
|||
elif isinstance(item, str):
|
||||
text, link = None, item
|
||||
|
||||
e = elements.eq(idx)
|
||||
elm = elements.eq(idx)
|
||||
if text is not None:
|
||||
assert e.text() == text, f'At index {idx}, expected {text}, got {e.text()}'
|
||||
assert (
|
||||
elm.text() == text
|
||||
), f'At index {idx}, expected {text}, got {elm.text()}'
|
||||
if link is not None:
|
||||
# If we passed an <li>, try to find an <a>.
|
||||
if not e.filter('a'):
|
||||
e = e.find('a')
|
||||
assert_url_equal(e.attr('href'), link)
|
||||
if verify and link != '#':
|
||||
assert Client().head(link, follow=True).status_code == 200
|
||||
if not elm.filter('a'):
|
||||
elm = elm.find('a')
|
||||
assert_url_equal(elm.attr('href'), link)
|
||||
if text is not None and selected is not None:
|
||||
e = e.filter('.selected, .sel') or e.parents('.selected, .sel')
|
||||
assert bool(e.length) == (text == selected)
|
||||
elm = elm.filter('.selected, .sel') or elm.parents('.selected, .sel')
|
||||
assert bool(elm.length) == (text == selected)
|
||||
|
||||
|
||||
def assert_url_equal(url, expected, compare_host=False):
|
||||
|
|
|
|||
|
|
@ -177,7 +177,7 @@ class TestCommon(TestCase):
|
|||
('Developer Hub', reverse('devhub.index')),
|
||||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
]
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'), verify=False)
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'))
|
||||
|
||||
def test_tools_developer(self):
|
||||
# Make them a developer.
|
||||
|
|
@ -199,7 +199,7 @@ class TestCommon(TestCase):
|
|||
('Developer Hub', reverse('devhub.index')),
|
||||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
]
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'), verify=False)
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'))
|
||||
|
||||
def test_tools_reviewer(self):
|
||||
user = UserProfile.objects.get(email='reviewer@mozilla.com')
|
||||
|
|
@ -217,7 +217,7 @@ class TestCommon(TestCase):
|
|||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
('Reviewer Tools', reverse('reviewers.dashboard')),
|
||||
]
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'), verify=False)
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'))
|
||||
|
||||
def test_tools_developer_and_reviewer(self):
|
||||
# Make them a developer.
|
||||
|
|
@ -239,7 +239,7 @@ class TestCommon(TestCase):
|
|||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
('Reviewer Tools', reverse('reviewers.dashboard')),
|
||||
]
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'), verify=False)
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'))
|
||||
|
||||
def test_tools_admin(self):
|
||||
user = UserProfile.objects.get(email='admin@mozilla.com')
|
||||
|
|
@ -261,7 +261,7 @@ class TestCommon(TestCase):
|
|||
('Reviewer Tools', reverse('reviewers.dashboard')),
|
||||
('Admin Tools', reverse('admin:index')),
|
||||
]
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'), verify=False)
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'))
|
||||
|
||||
def test_tools_developer_and_admin(self):
|
||||
# Make them a developer.
|
||||
|
|
@ -287,7 +287,7 @@ class TestCommon(TestCase):
|
|||
('Reviewer Tools', reverse('reviewers.dashboard')),
|
||||
('Admin Tools', reverse('admin:index')),
|
||||
]
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'), verify=False)
|
||||
check_links(expected, pq(response.content)('#aux-nav .tools a'))
|
||||
|
||||
|
||||
class TestOtherStuff(TestCase):
|
||||
|
|
|
|||
|
|
@ -1176,7 +1176,7 @@ class QueueTest(ReviewerTest):
|
|||
assert len(rows) == len(self.expected_addons)
|
||||
links = doc('#addon-queue tr.addon-row td a:not(.app-icon)')
|
||||
assert len(links) == len(self.expected_addons)
|
||||
check_links(expected, links, verify=False)
|
||||
check_links(expected, links)
|
||||
return doc
|
||||
|
||||
|
||||
|
|
@ -1467,9 +1467,7 @@ class TestExtensionQueue(QueueTest):
|
|||
),
|
||||
]
|
||||
doc = pq(response.content)
|
||||
check_links(
|
||||
expected, doc('#addon-queue tr.addon-row td a:not(.app-icon)'), verify=False
|
||||
)
|
||||
check_links(expected, doc('#addon-queue tr.addon-row td a:not(.app-icon)'))
|
||||
|
||||
def test_queue_layout(self):
|
||||
self.expected_addons = self.get_expected_addons_by_names(
|
||||
|
|
@ -1747,6 +1745,34 @@ class TestThemeQueue(QueueTest):
|
|||
# - 1 for my add-ons in user menu
|
||||
self._test_results()
|
||||
|
||||
def test_queue_ordering_by_due_date(self):
|
||||
# Bump pending one to the top by making the due date of its version
|
||||
# very old, like they have been waiting for a while.
|
||||
version = self.addons['Pending One'].versions.all()[0]
|
||||
version.update(due_date=self.days_ago(365))
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == 200
|
||||
expected = [
|
||||
(
|
||||
'Pending One 0.1',
|
||||
reverse('reviewers.review', args=[self.addons['Pending One'].pk]),
|
||||
),
|
||||
(
|
||||
'Nominated One 0.1',
|
||||
reverse('reviewers.review', args=[self.addons['Nominated One'].pk]),
|
||||
),
|
||||
(
|
||||
'Nominated Two 0.1',
|
||||
reverse('reviewers.review', args=[self.addons['Nominated Two'].pk]),
|
||||
),
|
||||
(
|
||||
'Pending Two 0.1',
|
||||
reverse('reviewers.review', args=[self.addons['Pending Two'].pk]),
|
||||
),
|
||||
]
|
||||
doc = pq(response.content)
|
||||
check_links(expected, doc('#addon-queue tr.addon-row td a:not(.app-icon)'))
|
||||
|
||||
def test_results_two_versions(self):
|
||||
version1 = self.addons['Nominated One'].versions.all()[0]
|
||||
version2 = self.addons['Nominated Two'].versions.all()[0]
|
||||
|
|
@ -1784,9 +1810,7 @@ class TestThemeQueue(QueueTest):
|
|||
),
|
||||
]
|
||||
doc = pq(response.content)
|
||||
check_links(
|
||||
expected, doc('#addon-queue tr.addon-row td a:not(.app-icon)'), verify=False
|
||||
)
|
||||
check_links(expected, doc('#addon-queue tr.addon-row td a:not(.app-icon)'))
|
||||
|
||||
def test_queue_layout(self):
|
||||
self._test_queue_layout(
|
||||
|
|
@ -2641,7 +2665,7 @@ class TestReview(ReviewBase):
|
|||
('Open in VSC', None),
|
||||
('Browse contents', None),
|
||||
]
|
||||
check_links(expected, items.find('a'), verify=False)
|
||||
check_links(expected, items.find('a'))
|
||||
|
||||
def test_item_history(self, channel=amo.CHANNEL_LISTED):
|
||||
self.addons['something'] = addon_factory(
|
||||
|
|
@ -3080,7 +3104,7 @@ class TestReview(ReviewBase):
|
|||
expected = [
|
||||
('View Product Page', self.addon.get_url_path()),
|
||||
]
|
||||
check_links(expected, doc('#actions-addon a'), verify=False)
|
||||
check_links(expected, doc('#actions-addon a'))
|
||||
|
||||
def test_action_links_as_admin(self):
|
||||
self.login_as_admin()
|
||||
|
|
@ -3093,7 +3117,7 @@ class TestReview(ReviewBase):
|
|||
('Admin Page', reverse('admin:addons_addon_change', args=[self.addon.id])),
|
||||
('Statistics', reverse('stats.overview', args=[self.addon.id])),
|
||||
]
|
||||
check_links(expected, doc('#actions-addon a'), verify=False)
|
||||
check_links(expected, doc('#actions-addon a'))
|
||||
|
||||
def test_unlisted_addon_action_links_as_admin(self):
|
||||
"""No "View Product Page" link for unlisted addons, "edit"/"manage" links
|
||||
|
|
@ -3112,7 +3136,7 @@ class TestReview(ReviewBase):
|
|||
('Admin Page', reverse('admin:addons_addon_change', args=[self.addon.id])),
|
||||
('Statistics', reverse('stats.overview', args=[self.addon.id])),
|
||||
]
|
||||
check_links(expected, doc('#actions-addon a'), verify=False)
|
||||
check_links(expected, doc('#actions-addon a'))
|
||||
|
||||
def test_mixed_channels_action_links_as_admin(self):
|
||||
self.make_addon_unlisted(self.addon)
|
||||
|
|
@ -3136,7 +3160,7 @@ class TestReview(ReviewBase):
|
|||
('Admin Page', reverse('admin:addons_addon_change', args=[self.addon.id])),
|
||||
('Statistics', reverse('stats.overview', args=[self.addon.id])),
|
||||
]
|
||||
check_links(expected, doc('#actions-addon a'), verify=False)
|
||||
check_links(expected, doc('#actions-addon a'))
|
||||
|
||||
def test_mixed_channels_action_links_as_admin_on_unlisted_review(self):
|
||||
self.make_addon_unlisted(self.addon)
|
||||
|
|
@ -3158,7 +3182,7 @@ class TestReview(ReviewBase):
|
|||
('Admin Page', reverse('admin:addons_addon_change', args=[self.addon.id])),
|
||||
('Statistics', reverse('stats.overview', args=[self.addon.id])),
|
||||
]
|
||||
check_links(expected, doc('#actions-addon a'), verify=False)
|
||||
check_links(expected, doc('#actions-addon a'))
|
||||
|
||||
def test_mixed_channels_action_links_as_admin_deleted_addon(self):
|
||||
self.make_addon_unlisted(self.addon)
|
||||
|
|
@ -3182,7 +3206,7 @@ class TestReview(ReviewBase):
|
|||
('Admin Page', reverse('admin:addons_addon_change', args=[self.addon.id])),
|
||||
('Statistics', reverse('stats.overview', args=[self.addon.id])),
|
||||
]
|
||||
check_links(expected, doc('#actions-addon a'), verify=False)
|
||||
check_links(expected, doc('#actions-addon a'))
|
||||
|
||||
def test_mixed_channels_action_links_as_admin_unlisted_deleted_addon(self):
|
||||
self.make_addon_unlisted(self.addon)
|
||||
|
|
@ -3203,7 +3227,7 @@ class TestReview(ReviewBase):
|
|||
('Admin Page', reverse('admin:addons_addon_change', args=[self.addon.id])),
|
||||
('Statistics', reverse('stats.overview', args=[self.addon.id])),
|
||||
]
|
||||
check_links(expected, doc('#actions-addon a'), verify=False)
|
||||
check_links(expected, doc('#actions-addon a'))
|
||||
|
||||
def test_mixed_channels_action_links_as_regular_reviewer(self):
|
||||
self.make_addon_unlisted(self.addon)
|
||||
|
|
@ -3220,7 +3244,7 @@ class TestReview(ReviewBase):
|
|||
expected = [
|
||||
('View Product Page', self.addon.get_url_path()),
|
||||
]
|
||||
check_links(expected, doc('#actions-addon a'), verify=False)
|
||||
check_links(expected, doc('#actions-addon a'))
|
||||
|
||||
def test_admin_links_as_non_admin(self):
|
||||
self.login_as_reviewer()
|
||||
|
|
@ -4022,7 +4046,7 @@ class TestReview(ReviewBase):
|
|||
),
|
||||
]
|
||||
|
||||
check_links(expected, links, verify=False)
|
||||
check_links(expected, links)
|
||||
|
||||
def test_compare_link_auto_approved_ignored(self):
|
||||
first_file = self.addon.current_version.file
|
||||
|
|
@ -4057,7 +4081,7 @@ class TestReview(ReviewBase):
|
|||
version_id=new_version.pk,
|
||||
),
|
||||
]
|
||||
check_links(expected, links, verify=False)
|
||||
check_links(expected, links)
|
||||
|
||||
def test_compare_link_auto_approved_but_confirmed_not_ignored(self):
|
||||
first_file = self.addon.current_version.file
|
||||
|
|
@ -4098,7 +4122,7 @@ class TestReview(ReviewBase):
|
|||
version_id=new_version.pk,
|
||||
),
|
||||
]
|
||||
check_links(expected, links, verify=False)
|
||||
check_links(expected, links)
|
||||
|
||||
def test_compare_link_not_auto_approved_but_confirmed(self):
|
||||
first_file = self.addon.current_version.file
|
||||
|
|
@ -4131,7 +4155,7 @@ class TestReview(ReviewBase):
|
|||
version_id=new_version.pk,
|
||||
),
|
||||
]
|
||||
check_links(expected, links, verify=False)
|
||||
check_links(expected, links)
|
||||
|
||||
def test_download_sources_link(self):
|
||||
version = self.addon.current_version
|
||||
|
|
@ -5375,7 +5399,7 @@ class TestReview(ReviewBase):
|
|||
(f'{old_two}', reverse('reviewers.review', args=[old_two.id])),
|
||||
]
|
||||
doc = pq(response.content)
|
||||
check_links(expected, doc('.addon-addons-sharing-guid a'), verify=False)
|
||||
check_links(expected, doc('.addon-addons-sharing-guid a'))
|
||||
|
||||
assert b'Original Add-on ID' in response.content
|
||||
assert doc('.addon-guid td').text() == self.addon.guid
|
||||
|
|
@ -5399,7 +5423,7 @@ class TestReview(ReviewBase):
|
|||
),
|
||||
]
|
||||
doc = pq(response.content)
|
||||
check_links(expected, doc('.addon-addons-sharing-guid a'), verify=False)
|
||||
check_links(expected, doc('.addon-addons-sharing-guid a'))
|
||||
|
||||
# It shouldn't happen nowadays, but make sure an empty guid isn't
|
||||
# considered.
|
||||
|
|
@ -8636,7 +8660,7 @@ class TestMadQueue(QueueTest):
|
|||
doc = pq(response.content)
|
||||
links = doc('#addon-queue tr.addon-row td a:not(.app-icon)')
|
||||
assert len(links) == len(expected)
|
||||
check_links(expected, links, verify=False)
|
||||
check_links(expected, links)
|
||||
|
||||
def test_only_viewable_with_specific_permission(self):
|
||||
# Content reviewer does not have access.
|
||||
|
|
|
|||
|
|
@ -146,7 +146,7 @@ class PendingManualApprovalQueueTable(AddonQueueTable):
|
|||
url = r'^extension$'
|
||||
permission = amo.permissions.ADDONS_REVIEW
|
||||
|
||||
class Meta:
|
||||
class Meta(AddonQueueTable.Meta):
|
||||
fields = ('addon_name', 'addon_type', 'due_date', 'flags', 'score')
|
||||
exclude = (
|
||||
'last_human_review',
|
||||
|
|
@ -154,6 +154,7 @@ class PendingManualApprovalQueueTable(AddonQueueTable):
|
|||
'metadata_weight',
|
||||
'weight',
|
||||
)
|
||||
orderable = True
|
||||
|
||||
@classmethod
|
||||
def get_queryset(self, request, *, upcoming_due_date_focus=False, **kw):
|
||||
|
|
@ -200,7 +201,7 @@ class ThemesQueueTable(PendingManualApprovalQueueTable):
|
|||
verbose_name='Target Date', accessor='first_version_due_date'
|
||||
)
|
||||
|
||||
class Meta(AddonQueueTable.Meta):
|
||||
class Meta(PendingManualApprovalQueueTable.Meta):
|
||||
exclude = (
|
||||
'score',
|
||||
'addon_type',
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче