launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17509
[Merge] lp:~wgrant/launchpad/misc-fixes into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/misc-fixes into lp:launchpad.
Commit message:
Drop duplicate CVE link, fix bugtask sort order with derived distros, avoid redirecting to bugs in inactive products, fix +expiringmembership grammar, drop duplicate MP email link, fix +bugs title, flip sort arrows, don't show irrelevant license info.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #458188 in Launchpad itself: "No help for CVE reports link on bug listing page"
https://bugs.launchpad.net/launchpad/+bug/458188
Bug #1064310 in Launchpad itself: "Distribution and series Bugs pages link to "CVE reports" twice"
https://bugs.launchpad.net/launchpad/+bug/1064310
Bug #1255941 in Launchpad itself: "Poor grammar in "Renew membership" page"
https://bugs.launchpad.net/launchpad/+bug/1255941
Bug #1278883 in Launchpad itself: "Bug.default_bugtask should avoid inactive products where possible"
https://bugs.launchpad.net/launchpad/+bug/1278883
Bug #1354865 in Launchpad itself: "Ordering of bugs by column is reversed"
https://bugs.launchpad.net/launchpad/+bug/1354865
Bug #1370527 in Launchpad itself: "URL repeated in merge proposal status change e-mail notification"
https://bugs.launchpad.net/launchpad/+bug/1370527
Bug #1377171 in Launchpad itself: "Box showing in the Licenses section of project page"
https://bugs.launchpad.net/launchpad/+bug/1377171
Bug #1392362 in Launchpad itself: "odd rendering with combination of project tasks and derived distribution"
https://bugs.launchpad.net/launchpad/+bug/1392362
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/misc-fixes/+merge/241850
Just a wide selection of trivial UI bugs.
--
https://code.launchpad.net/~wgrant/launchpad/misc-fixes/+merge/241850
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/misc-fixes into lp:launchpad.
=== renamed file 'lib/canonical/launchpad/images/order-descending.png' => 'lib/canonical/launchpad/images/order-ascending.png'
=== renamed file 'lib/canonical/launchpad/images/order-ascending.png' => 'lib/canonical/launchpad/images/order-descending.png'
=== modified file 'lib/lp/app/javascript/sorttable/sorttable.js'
--- lib/lp/app/javascript/sorttable/sorttable.js 2012-04-23 14:01:43 +0000
+++ lib/lp/app/javascript/sorttable/sorttable.js 2014-11-14 22:28:47 +0000
@@ -135,7 +135,7 @@
headrow.removeChild(document.getElementById('sorttable_sortfwdind'));
sortrevind = document.createElement('span');
sortrevind.id = "sorttable_sortrevind";
- sortrevind.innerHTML = stIsIE ? ' <font face="webdings">5</font>' : ' ▴';
+ sortrevind.innerHTML = stIsIE ? ' <font face="webdings">5</font>' : ' ▾';
headrow.appendChild(sortrevind);
return;
}
@@ -169,7 +169,7 @@
headrow.className += ' sorttable_sorted';
sortfwdind = document.createElement('span');
sortfwdind.id = "sorttable_sortfwdind";
- sortfwdind.innerHTML = stIsIE ? ' <font face="webdings">6</font>' : ' ▾';
+ sortfwdind.innerHTML = stIsIE ? ' <font face="webdings">6</font>' : ' ▴';
headrow.appendChild(sortfwdind);
// build an array to sort. This is a Schwartzian transform thing,
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2014-10-22 18:38:16 +0000
+++ lib/lp/bugs/browser/bugtask.py 2014-11-14 22:28:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""IBugTask-related browser views."""
@@ -26,7 +26,6 @@
'BugTaskPrivacyAdapter',
'BugTaskRemoveQuestionView',
'BugTaskSearchListingView',
- 'BugTaskSetNavigation',
'BugTasksNominationsView',
'BugTasksTableView',
'BugTaskTableRowView',
@@ -263,7 +262,6 @@
from lp.services.webapp import (
canonical_url,
enabled_with_permission,
- GetitemNavigation,
LaunchpadView,
Link,
Navigation,
@@ -613,11 +611,6 @@
redirection('references', '..')
-class BugTaskSetNavigation(GetitemNavigation):
- """Navigation for the `IbugTaskSet`."""
- usedfor = IBugTaskSet
-
-
class BugTaskContextMenu(BugContextMenu):
"""Context menu of actions that can be performed upon an `IBugTask`."""
usedfor = IBugTask
@@ -2412,29 +2405,17 @@
@property
def links(self):
bug_target = self.context.context
- if IDistribution.providedBy(bug_target):
- return (
- 'cve',
- )
- elif IDistroSeries.providedBy(bug_target):
- return (
- 'cve',
+ if IDistroSeries.providedBy(bug_target):
+ return (
'nominations',
)
- elif IProduct.providedBy(bug_target):
- return (
- 'cve',
- )
- elif IProductSeries.providedBy(bug_target):
+ if IProductSeries.providedBy(bug_target):
return (
'nominations',
)
else:
return ()
- def cve(self):
- return Link('+cve', 'CVE reports', icon='cve')
-
@enabled_with_permission('launchpad.Edit')
def bugsupervisor(self):
return Link('+bugsupervisor', 'Change bug supervisor', icon='edit')
@@ -2590,7 +2571,7 @@
@property
def page_title(self):
- return "Bugs : %s" % self.context.displayname
+ return "Bugs for %s" % self.context.displayname
label = page_title
@@ -3348,9 +3329,34 @@
getUtility(IBugTaskSet).searchBugIds(search_params))
-def _by_targetname(bugtask):
- """Normalize the bugtask.targetname, for sorting."""
- return re.sub(r"\W", "", bugtask.bugtargetdisplayname)
+def bugtask_sort_key(bugtask):
+ """Return a sort key for displaying a set of tasks for a single bug.
+
+ Designed to make sense when bugtargetdisplayname is shown.
+ """
+ if IDistribution.providedBy(bugtask.target):
+ return (
+ None, bugtask.target.displayname, None, None, None)
+ elif IDistroSeries.providedBy(bugtask.target):
+ return (
+ None, bugtask.target.distribution.displayname,
+ bugtask.target.name, None, None)
+ elif IDistributionSourcePackage.providedBy(bugtask.target):
+ return (
+ bugtask.target.sourcepackagename.name,
+ bugtask.target.distribution.displayname, None, None, None)
+ elif ISourcePackage.providedBy(bugtask.target):
+ return (
+ bugtask.target.sourcepackagename.name,
+ bugtask.target.distribution.displayname,
+ bugtask.target.distroseries.name, None, None)
+ elif IProduct.providedBy(bugtask.target):
+ return (None, None, None, bugtask.target.displayname, None)
+ elif IProductSeries.providedBy(bugtask.target):
+ return (
+ None, None, None, bugtask.target.product.displayname,
+ bugtask.target.name)
+ raise AssertionError("No sort key for %r" % bugtask.target)
class BugTasksNominationsView(LaunchpadView):
@@ -3626,19 +3632,7 @@
included in the returned results.
"""
bug = self.context
- bugtasks = self.bugtasks
-
- upstream_tasks = [
- bugtask for bugtask in bugtasks
- if bugtask.product or bugtask.productseries]
-
- distro_tasks = [
- bugtask for bugtask in bugtasks
- if bugtask.distribution or bugtask.distroseries]
-
- upstream_tasks.sort(key=_by_targetname)
- distro_tasks.sort(key=_by_targetname)
- all_bugtasks = upstream_tasks + distro_tasks
+ all_bugtasks = list(sorted(self.bugtasks, key=bugtask_sort_key))
# Cache whether the bug was converted to a question, since
# bug.getQuestionCreatedFromBug issues a db query each time it
@@ -3684,7 +3678,7 @@
name='+bugtasks-and-nominations-table-row'))
conjoined_master = bugtask.getConjoinedMaster(
- bugtasks, bugtasks_by_package)
+ all_bugtasks, bugtasks_by_package)
view = self._getTableRowView(
bugtask, is_converted_to_question,
conjoined_master is not None)
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2014-02-19 00:35:25 +0000
+++ lib/lp/bugs/browser/configure.zcml 2014-11-14 22:28:47 +0000
@@ -468,9 +468,7 @@
rootsite="bugs"/>
<browser:navigation
module="lp.bugs.browser.bugtask"
- classes="
- BugTaskNavigation
- BugTaskSetNavigation"/>
+ classes="BugTaskNavigation"/>
<browser:page
for="lp.bugs.interfaces.bug.IBug"
class="lp.bugs.browser.bug.BugSubscriptionPortletView"
@@ -545,16 +543,6 @@
class="lp.bugs.browser.bug.BugMarkAsAffectingUserView"
permission="launchpad.AnyPerson"
template="../templates/bug-mark-as-affecting-user.pt"/>
- <browser:defaultView
- for="lp.bugs.interfaces.bugtask.IBugTaskSet"
- name="+index"/>
- <browser:pages
- for="lp.bugs.interfaces.bugtask.IBugTaskSet"
- permission="zope.Public">
- <browser:page
- name="+index"
- template="../templates/bugtasks-index.pt"/>
- </browser:pages>
<browser:page
name="+addcomment"
for="lp.bugs.interfaces.bugtask.IBugTask"
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2013-10-24 06:20:06 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2014-11-14 22:28:47 +0000
@@ -835,6 +835,44 @@
foo_bugtasks_and_nominations_view.getBugTaskAndNominationViews())
self.assertEqual([], task_and_nomination_views)
+ def test_bugtask_sorting(self):
+ # Product tasks come first, sorted by product then series.
+ # Distro tasks follow, sorted by package, distribution, then
+ # series.
+ foo = self.factory.makeProduct(displayname='Foo')
+ self.factory.makeProductSeries(product=foo, name='2.0')
+ self.factory.makeProductSeries(product=foo, name='1.0')
+ bar = self.factory.makeProduct(displayname='Bar')
+ self.factory.makeProductSeries(product=bar, name='0.0')
+
+ barix = self.factory.makeDistribution(displayname='Barix')
+ self.factory.makeDistroSeries(distribution=barix, name='beta')
+ self.factory.makeDistroSeries(distribution=barix, name='alpha')
+ fooix = self.factory.makeDistribution(displayname='Fooix')
+ self.factory.makeDistroSeries(distribution=fooix, name='beta')
+
+ foo_spn = self.factory.makeSourcePackageName('foo')
+ bar_spn = self.factory.makeSourcePackageName('bar')
+
+ expected_targets = [
+ bar, bar.getSeries('0.0'),
+ foo, foo.getSeries('1.0'), foo.getSeries('2.0'),
+ barix.getSourcePackage(bar_spn),
+ barix.getSeries('beta').getSourcePackage(bar_spn),
+ fooix.getSourcePackage(bar_spn),
+ fooix.getSeries('beta').getSourcePackage(bar_spn),
+ barix.getSourcePackage(foo_spn),
+ barix.getSeries('alpha').getSourcePackage(foo_spn),
+ ]
+
+ bug = self.factory.makeBug(target=expected_targets[0])
+ for target in expected_targets[1:]:
+ self.factory.makeBugTask(bug=bug, target=target)
+ view = create_initialized_view(bug, "+bugtasks-and-nominations-table")
+ subviews = view.getBugTaskAndNominationViews()
+ self.assertEqual(
+ expected_targets, [v.context.target for v in subviews])
+
def test_bugtarget_parent_shown_for_orphaned_series_tasks(self):
# Test that a row is shown for the parent of a series task, even
# if the parent doesn't actually have a task.
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2014-04-29 00:44:32 +0000
+++ lib/lp/bugs/model/bug.py 2014-11-14 22:28:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Launchpad bug-related database table classes."""
@@ -44,6 +44,7 @@
)
from storm.expr import (
And,
+ Coalesce,
Desc,
In,
Join,
@@ -650,8 +651,14 @@
@property
def default_bugtask(self):
"""See `IBug`."""
- return Store.of(self).find(
- BugTask, bug=self).order_by(BugTask.id).first()
+ from lp.registry.model.product import Product
+ return Store.of(self).using(
+ BugTask,
+ LeftJoin(Product, Product.id == BugTask.productID)
+ ).find(
+ BugTask, bug=self
+ ).order_by(
+ Desc(Coalesce(Product.active, True)), BugTask.id).first()
@property
def is_complete(self):
=== removed file 'lib/lp/bugs/templates/bugtasks-index.pt'
--- lib/lp/bugs/templates/bugtasks-index.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/bugs/templates/bugtasks-index.pt 1970-01-01 00:00:00 +0000
@@ -1,7 +0,0 @@
-<html
- xmlns:tal="http://xml.zope.org/namespaces/tal";
- tal:replace="python:request.response.redirect('..')">
- <body>
- <a href=".">Here</a>
- </body>
-</html>
=== modified file 'lib/lp/bugs/tests/test_bug.py'
--- lib/lp/bugs/tests/test_bug.py 2013-10-22 04:09:23 +0000
+++ lib/lp/bugs/tests/test_bug.py 2014-11-14 22:28:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2014 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for lp.bugs.model.Bug."""
@@ -24,6 +24,7 @@
UserCannotEditBugTaskMilestone,
)
from lp.testing import (
+ admin_logged_in,
person_logged_in,
StormStatementRecorder,
TestCaseWithFactory,
@@ -31,6 +32,30 @@
from lp.testing.layers import DatabaseFunctionalLayer
+class TestBug(TestCaseWithFactory):
+ layer = DatabaseFunctionalLayer
+
+ def test_default_bugtask(self):
+ product = self.factory.makeProduct()
+ bug = self.factory.makeBug(target=product)
+ first_task = bug.default_bugtask
+ other_task = self.factory.makeBugTask(
+ bug=bug, target=self.factory.makeProduct())
+ self.assertEqual(first_task, bug.default_bugtask)
+ # default_bugtask avoids an inactive product if possible.
+ with admin_logged_in():
+ first_task.target.active = False
+ self.assertEqual(other_task, bug.default_bugtask)
+ # But it'll use the first inactive one if it has to.
+ with admin_logged_in():
+ other_task.target.active = False
+ self.assertEqual(first_task, bug.default_bugtask)
+ # An active distro task wins over an inactive product.
+ distro_task = self.factory.makeBugTask(
+ bug=bug, target=self.factory.makeDistribution())
+ self.assertEqual(distro_task, bug.default_bugtask)
+
+
class TestBugSubscriptionMethods(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
=== modified file 'lib/lp/code/emailtemplates/branch-merge-proposal-created.txt'
--- lib/lp/code/emailtemplates/branch-merge-proposal-created.txt 2012-09-18 21:49:22 +0000
+++ lib/lp/code/emailtemplates/branch-merge-proposal-created.txt 2014-11-14 22:28:47 +0000
@@ -4,5 +4,4 @@
For more details, see:
%(proposal_url)s%(gap)s%(comment)s
--
-%(diff_cutoff_warning)s%(proposal_url)s
-%(reason)s%(edit_subscription)s
+%(diff_cutoff_warning)s%(reason)s%(edit_subscription)s
=== modified file 'lib/lp/code/emailtemplates/branch-merge-proposal-updated.txt'
--- lib/lp/code/emailtemplates/branch-merge-proposal-updated.txt 2011-12-18 22:31:46 +0000
+++ lib/lp/code/emailtemplates/branch-merge-proposal-updated.txt 2014-11-14 22:28:47 +0000
@@ -5,5 +5,4 @@
For more details, see:
%(proposal_url)s
--
-%(proposal_url)s
%(reason)s%(edit_subscription)s
=== modified file 'lib/lp/code/emailtemplates/review-requested.txt'
--- lib/lp/code/emailtemplates/review-requested.txt 2011-12-18 22:31:46 +0000
+++ lib/lp/code/emailtemplates/review-requested.txt 2014-11-14 22:28:47 +0000
@@ -6,5 +6,4 @@
%(comment)s
--
-%(diff_cutoff_warning)s%(proposal_url)s
-%(reason)s%(edit_subscription)s
+%(diff_cutoff_warning)s%(reason)s%(edit_subscription)s
=== modified file 'lib/lp/code/mail/tests/test_branchmergeproposal.py'
--- lib/lp/code/mail/tests/test_branchmergeproposal.py 2013-06-20 05:50:00 +0000
+++ lib/lp/code/mail/tests/test_branchmergeproposal.py 2014-11-14 22:28:47 +0000
@@ -114,7 +114,6 @@
For more details, see:
%(bmp)s
--\x20
- %(bmp)s
%(reason)s
""") % {
'source': bmp.source_branch.bzr_identity,
@@ -516,7 +515,6 @@
For more details, see:
%(bmp)s
--\x20
- %(bmp)s
You are the owner of lp://dev/~bob/super-product/fix-foo-for-bar.
""") % {
'source': bmp.source_branch.bzr_identity,
@@ -644,7 +642,6 @@
This branch is awesome.
--\x20
- %(bmp)s
You are requested to review the proposed merge of %(source)s"""
""" into %(target)s.
""" % {
=== modified file 'lib/lp/registry/stories/product/xx-product-index.txt'
--- lib/lp/registry/stories/product/xx-product-index.txt 2014-09-08 08:07:06 +0000
+++ lib/lp/registry/stories/product/xx-product-index.txt 2014-11-14 22:28:47 +0000
@@ -122,6 +122,7 @@
>>> firefox = Product.selectOneBy(name="firefox")
>>> ignored = login_person(firefox.owner)
>>> firefox.licenses = [License.OTHER_PROPRIETARY]
+ >>> firefox.license_info = u"Internal project."
>>> flush_database_updates()
>>> transaction.commit()
>>> logout()
@@ -145,6 +146,12 @@
>>> user_browser.open('http://launchpad.dev/firefox')
>>> user_browser.contents
'<...This project’s licence is proprietary...
+ >>> print extract_text(
+ ... find_tag_by_id(user_browser.contents, 'licences'))
+ Licences:
+ Other/Proprietary (Internal project.)
+ Commercial subscription expires ...
+
A non-owner does not see that a commercial subscription is due.
@@ -169,6 +176,11 @@
>>> user_browser.open('http://launchpad.dev/firefox')
>>> print find_tag_by_id(owner_browser.contents, 'license-status')
None
+ >>> print extract_text(
+ ... find_tag_by_id(user_browser.contents, 'licences'))
+ Licences:
+ GNU GPL v2
+ Commercial subscription expires ...
Commercial Subscription Expiration
=== modified file 'lib/lp/registry/stories/teammembership/xx-member-renewed-membership.txt'
--- lib/lp/registry/stories/teammembership/xx-member-renewed-membership.txt 2012-08-02 14:18:36 +0000
+++ lib/lp/registry/stories/teammembership/xx-member-renewed-membership.txt 2014-11-14 22:28:47 +0000
@@ -107,7 +107,7 @@
>>> print extract_text(find_tag_by_id(browser.contents, 'maincontent'))
Renew membership of Karl Tilbury in Mirror Administrators
...
- This membership is going to expire in ... from now; if you want to
+ This membership is going to expire ... from now. If you want to
remain a member of Mirror Administrators, you must renew it.
or Cancel
@@ -162,7 +162,7 @@
>>> print extract_text(find_tag_by_id(browser.contents, 'maincontent'))
Renew membership of Landscape Developers in Mirror Administrators
...
- This membership is going to expire in ... from now; if you want this team
+ This membership is going to expire ... from now. If you want this team
to remain a member of Mirror Administrators, you must renew it.
or Cancel
=== modified file 'lib/lp/registry/templates/product-index.pt'
--- lib/lp/registry/templates/product-index.pt 2013-01-25 06:08:10 +0000
+++ lib/lp/registry/templates/product-index.pt 2014-11-14 22:28:47 +0000
@@ -138,7 +138,7 @@
</tal:licenses>
<tal:none condition="not:context/licenses">None specified</tal:none>
<div class="scrolled-box"
- condition="view/show_license_info">
+ tal:condition="view/show_license_info">
(<tal:license_info replace="context/license_info" />)
</div>
</dd>
=== modified file 'lib/lp/registry/templates/teammembership-self-renewal.pt'
--- lib/lp/registry/templates/teammembership-self-renewal.pt 2011-12-08 22:41:00 +0000
+++ lib/lp/registry/templates/teammembership-self-renewal.pt 2014-11-14 22:28:47 +0000
@@ -13,16 +13,16 @@
<div tal:condition="context/canBeRenewedByMember">
<div metal:use-macro="context/@@launchpad_form/form">
<p metal:fill-slot="extra_info">
- This membership is going to expire in
+ This membership is going to expire
<span tal:replace="view/time_before_expiration/fmt:approximateduration"
- /> from now;
+ /> from now.
<tal:is-team condition="context/person/is_team">
- if you want this team to remain a member of
+ If you want this team to remain a member of
<span tal:replace="structure context/team/fmt:link" />,
you must renew it.
</tal:is-team>
<tal:not-team condition="not: context/person/is_team">
- if you want to remain a member of
+ If you want to remain a member of
<span tal:replace="structure context/team/fmt:link" />,
you must renew it.
</tal:not-team>
Follow ups
