launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17605
[Merge] lp:~wgrant/launchpad/sanitise-labels into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/sanitise-labels into lp:launchpad.
Commit message:
Remove the duplicate search title from bug listings, and massively refactor the Person bug views.
Requested reviews:
William Grant (wgrant): code
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/sanitise-labels/+merge/243202
Remove the duplicate search title from bug listings, and massively refactor the Person bug views.
--
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/app/interfaces/launchpad.py'
--- lib/lp/app/interfaces/launchpad.py 2013-05-10 06:28:17 +0000
+++ lib/lp/app/interfaces/launchpad.py 2014-11-29 04:56:30 +0000
@@ -14,6 +14,7 @@
'IHasIcon',
'IHasLogo',
'IHasMugshot',
+ 'IHeadingContext',
'ILaunchpadCelebrities',
'ILaunchpadUsage',
'IPrivacy',
@@ -179,3 +180,13 @@
"""Something created on a certain date."""
datecreated = Attribute("The date on which I was created.")
+
+
+class IHeadingContext(Interface):
+ """Something that appears in a page's header section.
+
+ This is a marker to allow views to avoid duplicating header
+ information in the body. The header is generated from
+ IHeadingBreadcrumbs, so you also need to define one of those for
+ each of these.
+ """
=== modified file 'lib/lp/blueprints/interfaces/sprint.py'
--- lib/lp/blueprints/interfaces/sprint.py 2014-11-17 18:36:16 +0000
+++ lib/lp/blueprints/interfaces/sprint.py 2014-11-29 04:56:30 +0000
@@ -30,6 +30,7 @@
from lp import _
from lp.app.validators.name import name_validator
+from lp.app.interfaces.launchpad import IHeadingContext
from lp.blueprints.interfaces.specificationtarget import IHasSpecifications
from lp.registry.interfaces.role import (
IHasDrivers,
@@ -56,7 +57,7 @@
return getUtility(ISprintSet)[name]
-class ISprint(IHasOwner, IHasDrivers, IHasSpecifications):
+class ISprint(IHasOwner, IHasDrivers, IHasSpecifications, IHeadingContext):
"""A sprint, or conference, or meeting."""
id = Int(title=_('The Sprint ID'))
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2014-11-14 22:20:18 +0000
+++ lib/lp/bugs/browser/bugtask.py 2014-11-29 04:56:30 +0000
@@ -3082,10 +3082,6 @@
"""Should the reporter widget be shown on the advanced search page?"""
return True
- def shouldShowTagsCombinatorWidget(self):
- """Should the tags combinator widget show on the search page?"""
- return True
-
def shouldShowReleaseCriticalPortlet(self):
"""Should the page include a portlet showing release-critical bugs
for different series.
@@ -3241,21 +3237,6 @@
else:
return None
- @property
- def search_macro_title(self):
- """The search macro's title text."""
- return u"Search bugs %s" % self.context_description
-
- @property
- def context_description(self):
- """A phrase describing the context of the bug.
-
- The phrase is intended to be used for headings like
- "Bugs in $context", "Search bugs in $context". This
- property should be overridden for person related views.
- """
- return "in %s" % self.context.displayname
-
class BugNominationsView(BugTaskSearchListingView):
"""View for accepting/declining bug nominations."""
@@ -4023,9 +4004,6 @@
"""Return the heading to search all Bugs."""
return "Search all bug reports"
- def search_macro_title(self):
- return u'Search all bugs'
-
@property
def label(self):
return self.getSearchPageHeading()
=== modified file 'lib/lp/bugs/browser/person.py'
--- lib/lp/bugs/browser/person.py 2014-11-29 00:03:36 +0000
+++ lib/lp/bugs/browser/person.py 2014-11-29 04:56:30 +0000
@@ -189,29 +189,36 @@
return sorted(L, key=itemgetter('package_name'))
-class PersonAssignedBugTaskSearchListingView(RelevantMilestonesMixin,
- BugTaskSearchListingView):
+class FilteredSearchListingViewMixin(RelevantMilestonesMixin,
+ BugTaskSearchListingView):
+ columns_to_show = ["id", "summary", "bugtargetdisplayname",
+ "importance", "status"]
+
+ @property
+ def page_title(self):
+ return self.label
+
+ def searchUnbatched(self, searchtext=None, context=None,
+ extra_params=None):
+ context = context or self.context
+ extra_params = extra_params or {}
+ extra_params.update(self.getExtraParams(context))
+ return super(FilteredSearchListingViewMixin, self).searchUnbatched(
+ searchtext, context, extra_params)
+
+ def getSimpleSearchURL(self):
+ """Return a URL that can be used as an href to the simple search."""
+ return canonical_url(self.context, view_name=self.view_name)
+
+
+class PersonAssignedBugTaskSearchListingView(FilteredSearchListingViewMixin):
"""All bugs assigned to someone."""
- columns_to_show = ["id", "summary", "bugtargetdisplayname",
- "importance", "status"]
- page_title = 'Assigned bugs'
+ label = 'Assigned bugs'
view_name = '+assignedbugs'
- def searchUnbatched(self, searchtext=None, context=None,
- extra_params=None):
- """Return the open bugs assigned to a person."""
- if context is None:
- context = self.context
-
- if extra_params is None:
- extra_params = dict()
- else:
- extra_params = dict(extra_params)
- extra_params['assignee'] = context
-
- sup = super(PersonAssignedBugTaskSearchListingView, self)
- return sup.searchUnbatched(searchtext, context, extra_params)
+ def getExtraParams(self, context):
+ return {'assignee': context}
def shouldShowAssigneeWidget(self):
"""Should the assignee widget be shown on the advanced search page?"""
@@ -221,100 +228,25 @@
"""Should the team assigned bugs portlet be shown?"""
return True
- def shouldShowTagsCombinatorWidget(self):
- """Should the tags combinator widget show on the search page?"""
- return False
-
- @property
- def context_description(self):
- """See `BugTaskSearchListingView`."""
- return "assigned to %s" % self.context.displayname
-
- def getSearchPageHeading(self):
- """The header for the search page."""
- return "Bugs %s" % self.context_description
-
- def getAdvancedSearchButtonLabel(self):
- """The Search button for the advanced search page."""
- return "Search bugs %s" % self.context_description
-
- def getSimpleSearchURL(self):
- """Return a URL that can be used as an href to the simple search."""
- return canonical_url(self.context, view_name="+assignedbugs")
-
- @property
- def label(self):
- return self.getSearchPageHeading()
-
-
-class PersonCommentedBugTaskSearchListingView(RelevantMilestonesMixin,
- BugTaskSearchListingView):
+
+class PersonCommentedBugTaskSearchListingView(FilteredSearchListingViewMixin):
"""All bugs commented on by a Person."""
- columns_to_show = ["id", "summary", "bugtargetdisplayname",
- "importance", "status"]
- page_title = 'Commented bugs'
-
- def searchUnbatched(self, searchtext=None, context=None,
- extra_params=None):
- """Return the open bugs commented on by a person."""
- if context is None:
- context = self.context
-
- if extra_params is None:
- extra_params = dict()
- else:
- extra_params = dict(extra_params)
- extra_params['bug_commenter'] = context
-
- sup = super(PersonCommentedBugTaskSearchListingView, self)
- return sup.searchUnbatched(searchtext, context, extra_params)
-
- @property
- def context_description(self):
- """See `BugTaskSearchListingView`."""
- return "commented on by %s" % self.context.displayname
-
- def getSearchPageHeading(self):
- """The header for the search page."""
- return "Bugs %s" % self.context_description
-
- def getAdvancedSearchButtonLabel(self):
- """The Search button for the advanced search page."""
- return "Search bugs %s" % self.context_description
-
- def getSimpleSearchURL(self):
- """Return a URL that can be used as an href to the simple search."""
- return canonical_url(self.context, view_name="+commentedbugs")
-
- @property
- def label(self):
- return self.getSearchPageHeading()
-
-
-class PersonAffectingBugTaskSearchListingView(
- RelevantMilestonesMixin, BugTaskSearchListingView):
+ label = 'Commented bugs'
+ view_name = '+commentedbugs'
+
+ def getExtraParams(self, context):
+ return {'bug_commenter': context}
+
+
+class PersonAffectingBugTaskSearchListingView(FilteredSearchListingViewMixin):
"""All bugs affecting someone."""
- columns_to_show = ["id", "summary", "bugtargetdisplayname",
- "importance", "status"]
+ label = 'Bugs affecting'
view_name = '+affectingbugs'
- page_title = 'Bugs affecting' # The context is added externally.
-
- def searchUnbatched(self, searchtext=None, context=None,
- extra_params=None):
- """Return the open bugs assigned to a person."""
- if context is None:
- context = self.context
-
- if extra_params is None:
- extra_params = dict()
- else:
- extra_params = dict(extra_params)
- extra_params['affected_user'] = context
-
- sup = super(PersonAffectingBugTaskSearchListingView, self)
- return sup.searchUnbatched(searchtext, context, extra_params)
+
+ def getExtraParams(self, context):
+ return {'affected_user': context}
def shouldShowAssigneeWidget(self):
"""Should the assignee widget be shown on the advanced search page?"""
@@ -324,40 +256,13 @@
"""Should the team assigned bugs portlet be shown?"""
return True
- def shouldShowTagsCombinatorWidget(self):
- """Should the tags combinator widget show on the search page?"""
- return False
-
- @property
- def context_description(self):
- """See `BugTaskSearchListingView`."""
- return "affecting %s" % self.context.displayname
-
- def getSearchPageHeading(self):
- """The header for the search page."""
- return "Bugs %s" % self.context_description
-
- def getAdvancedSearchButtonLabel(self):
- """The Search button for the advanced search page."""
- return "Search bugs %s" % self.context_description
-
- def getSimpleSearchURL(self):
- """Return a URL that can be used as an href to the simple search."""
- return canonical_url(self.context, view_name=self.view_name)
-
- @property
- def label(self):
- return self.getSearchPageHeading()
-
-
-class PersonRelatedBugTaskSearchListingView(RelevantMilestonesMixin,
- BugTaskSearchListingView,
+
+class PersonRelatedBugTaskSearchListingView(FilteredSearchListingViewMixin,
FeedsMixin):
"""All bugs related to someone."""
- columns_to_show = ["id", "summary", "bugtargetdisplayname",
- "importance", "status"]
- page_title = 'Related bugs'
+ label = 'Related bugs'
+ view_name = '+bugs'
def searchUnbatched(self, searchtext=None, context=None,
extra_params=None):
@@ -392,130 +297,36 @@
return context.searchTasks(
assignee_params, subscriber_params, owner_params, commenter_params)
- @property
- def context_description(self):
- """See `BugTaskSearchListingView`."""
- return "related to %s" % self.context.displayname
-
- def getSearchPageHeading(self):
- return "Bugs %s" % self.context_description
-
- def getAdvancedSearchButtonLabel(self):
- return "Search bugs %s" % self.context_description
-
- def getSimpleSearchURL(self):
- return canonical_url(self.context, view_name="+bugs")
-
- @property
- def label(self):
- return self.getSearchPageHeading()
-
-
-class PersonReportedBugTaskSearchListingView(RelevantMilestonesMixin,
- BugTaskSearchListingView):
+
+class PersonReportedBugTaskSearchListingView(FilteredSearchListingViewMixin):
"""All bugs reported by someone."""
- columns_to_show = ["id", "summary", "bugtargetdisplayname",
- "importance", "status"]
- page_title = 'Reported bugs'
-
- def searchUnbatched(self, searchtext=None, context=None,
- extra_params=None):
- """Return the bugs reported by a person."""
- if context is None:
- context = self.context
-
- if extra_params is None:
- extra_params = dict()
- else:
- extra_params = dict(extra_params)
+ label = 'Reported bugs'
+ view_name = '+reportedbugs'
+
+ def getExtraParams(self, context):
# Specify both owner and bug_reporter to try to prevent the same
# bug (but different tasks) being displayed.
- extra_params['owner'] = context
- extra_params['bug_reporter'] = context
-
- sup = super(PersonReportedBugTaskSearchListingView, self)
- return sup.searchUnbatched(searchtext, context, extra_params)
-
- @property
- def context_description(self):
- """See `BugTaskSearchListingView`."""
- return "reported by %s" % self.context.displayname
-
- def getSearchPageHeading(self):
- """The header for the search page."""
- return "Bugs %s" % self.context_description
-
- def getAdvancedSearchButtonLabel(self):
- """The Search button for the advanced search page."""
- return "Search bugs %s" % self.context_description
-
- def getSimpleSearchURL(self):
- """Return a URL that can be used as an href to the simple search."""
- return canonical_url(self.context, view_name="+reportedbugs")
+ return {'owner': context, 'bug_reporter': context}
def shouldShowReporterWidget(self):
"""Should the reporter widget be shown on the advanced search page?"""
return False
- def shouldShowTagsCombinatorWidget(self):
- """Should the tags combinator widget show on the search page?"""
- return False
-
- @property
- def label(self):
- return self.getSearchPageHeading()
-
-
-class PersonSubscribedBugTaskSearchListingView(RelevantMilestonesMixin,
- BugTaskSearchListingView):
+
+class PersonSubscribedBugTaskSearchListingView(FilteredSearchListingViewMixin):
"""All bugs someone is subscribed to."""
- columns_to_show = ["id", "summary", "bugtargetdisplayname",
- "importance", "status"]
- page_title = 'Subscribed bugs'
+ label = 'Subscribed bugs'
view_name = '+subscribedbugs'
- def searchUnbatched(self, searchtext=None, context=None,
- extra_params=None):
- """Return the bugs subscribed to by a person."""
- if context is None:
- context = self.context
-
- if extra_params is None:
- extra_params = dict()
- else:
- extra_params = dict(extra_params)
- extra_params['subscriber'] = context
-
- sup = super(PersonSubscribedBugTaskSearchListingView, self)
- return sup.searchUnbatched(searchtext, context, extra_params)
+ def getExtraParams(self, context):
+ return {'subscriber': context}
def shouldShowTeamPortlet(self):
"""Should the team subscribed bugs portlet be shown?"""
return True
- @property
- def context_description(self):
- """See `BugTaskSearchListingView`."""
- return "%s is subscribed to" % self.context.displayname
-
- def getSearchPageHeading(self):
- """The header for the search page."""
- return "Bugs %s" % self.context_description
-
- def getAdvancedSearchButtonLabel(self):
- """The Search button for the advanced search page."""
- return "Search bugs %s is Cc'd to" % self.context.displayname
-
- def getSimpleSearchURL(self):
- """Return a URL that can be used as an href to the simple search."""
- return canonical_url(self.context, view_name="+subscribedbugs")
-
- @property
- def label(self):
- return self.getSearchPageHeading()
-
class PersonSubscriptionsView(LaunchpadView):
"""All the subscriptions for a person."""
=== modified file 'lib/lp/bugs/browser/tests/test_buglisting.py'
--- lib/lp/bugs/browser/tests/test_buglisting.py 2014-11-14 23:21:57 +0000
+++ lib/lp/bugs/browser/tests/test_buglisting.py 2014-11-29 04:56:30 +0000
@@ -207,20 +207,6 @@
self.assertFalse(
'Y.lp.app.batchnavigator.BatchNavigatorHooks' in view())
- def test_search_macro_title(self):
- # The title text is displayed for the macro `simple-search-form`.
- product = self.factory.makeProduct(
- displayname='Test Product', official_malone=True)
- view = create_initialized_view(product, '+bugs')
- self.assertEqual(
- 'Search bugs in Test Product', view.search_macro_title)
-
- # The title is shown.
- form_title_matches = Tag(
- 'Search form title', 'h3', text=view.search_macro_title)
- view = create_initialized_view(product, '+bugs')
- self.assertThat(view.render(), HTMLContains(form_title_matches))
-
def test_search_macro_div_node_with_css_class(self):
# The <div> enclosing the search form in the macro
# `simple-search-form` has the CSS class "dynamic_bug_listing".
=== modified file 'lib/lp/bugs/browser/tests/test_bugs.py'
--- lib/lp/bugs/browser/tests/test_bugs.py 2014-02-19 04:01:46 +0000
+++ lib/lp/bugs/browser/tests/test_bugs.py 2014-11-29 04:56:30 +0000
@@ -100,7 +100,7 @@
self.application, '+bugs', rootsite='bugs')
content = view.render()
# we should get some valid content out of this
- self.assertIn('Search all bugs', content)
+ self.assertIn('Search all bug reports', content)
def _assert_getBugData(self, related_bug=None):
# The getBugData method works as expected.
=== modified file 'lib/lp/bugs/templates/bugtarget-macros-search.pt'
--- lib/lp/bugs/templates/bugtarget-macros-search.pt 2012-10-02 06:36:44 +0000
+++ lib/lp/bugs/templates/bugtarget-macros-search.pt 2014-11-29 04:56:30 +0000
@@ -83,9 +83,6 @@
tal:attributes="action search_url|string:">
<form method="get" name="search" class="primary search dynamic_bug_listing"
tal:attributes="action search_url|string:">
- <h3 tal:content="view/search_macro_title">
- Search bugs in Ubuntu
- </h3>
<p>
<tal:searchbox replace="structure view/widgets/searchtext" />
<input type="submit" name="search" value="Search" />
=== modified file 'lib/lp/bugs/templates/bugtask-macros-tableview.pt'
--- lib/lp/bugs/templates/bugtask-macros-tableview.pt 2014-02-18 11:40:52 +0000
+++ lib/lp/bugs/templates/bugtask-macros-tableview.pt 2014-11-29 04:56:30 +0000
@@ -307,8 +307,7 @@
>An error message.</div>
</div>
- <div condition="view/shouldShowTagsCombinatorWidget"
- class="value">
+ <div class="value">
<label style="font-weight: normal">
<input id="field.tags_combinator.0"
name="field.tags_combinator"
=== modified file 'lib/lp/registry/browser/tests/test_person.py'
--- lib/lp/registry/browser/tests/test_person.py 2014-07-24 07:56:06 +0000
+++ lib/lp/registry/browser/tests/test_person.py 2014-11-29 04:56:30 +0000
@@ -1411,21 +1411,12 @@
self.assertEqual(expected, view.getMilestoneWidgetValues())
self.assertThat(recorder, HasQueryCount(LessThan(6)))
- def test_context_description(self):
- # view.context_description returns a string that can be used
- # in texts like "Bugs in $context_descirption"
- view = create_initialized_view(self.person, self.view_name)
- self.assertEqual(
- self.expected_context_description % self.person.displayname,
- view.context_description)
-
class TestPersonRelatedBugTaskSearchListingView(
BugTaskViewsTestBase, TestCaseWithFactory):
"""Tests for PersonRelatedBugTaskSearchListingView."""
view_name = '+bugs'
- expected_context_description = 'related to %s'
def setUp(self):
super(TestPersonRelatedBugTaskSearchListingView, self).setUp()
@@ -1442,7 +1433,6 @@
"""Tests for PersonAssignedBugTaskSearchListingView."""
view_name = '+assignedbugs'
- expected_context_description = 'assigned to %s'
def setUp(self):
super(TestPersonAssignedBugTaskSearchListingView, self).setUp()
@@ -1456,7 +1446,6 @@
"""Tests for PersonAssignedBugTaskSearchListingView."""
view_name = '+commentedbugs'
- expected_context_description = 'commented on by %s'
def setUp(self):
super(TestPersonCommentedBugTaskSearchListingView, self).setUp()
@@ -1470,7 +1459,6 @@
"""Tests for PersonAssignedBugTaskSearchListingView."""
view_name = '+reportedbugs'
- expected_context_description = 'reported by %s'
def setUp(self):
super(TestPersonReportedBugTaskSearchListingView, self).setUp()
@@ -1484,7 +1472,6 @@
"""Tests for PersonAssignedBugTaskSearchListingView."""
view_name = '+subscribedbugs'
- expected_context_description = '%s is subscribed to'
def setUp(self):
super(TestPersonSubscribedBugTaskSearchListingView, self).setUp()
@@ -1499,7 +1486,6 @@
"""Tests for PersonAffectingBugTaskSearchListingView."""
view_name = '+affectingbugs'
- expected_context_description = 'affecting %s'
def setUp(self):
super(TestPersonAffectingBugTaskSearchListingView, self).setUp()
=== modified file 'lib/lp/registry/interfaces/distributionsourcepackage.py'
--- lib/lp/registry/interfaces/distributionsourcepackage.py 2013-10-01 00:57:56 +0000
+++ lib/lp/registry/interfaces/distributionsourcepackage.py 2014-11-29 04:56:30 +0000
@@ -22,6 +22,7 @@
from lp import _
from lp.answers.interfaces.questiontarget import IQuestionTarget
+from lp.app.interfaces.launchpad import IHeadingContext
from lp.bugs.interfaces.bugtarget import (
IBugTarget,
IHasOfficialBugTags,
@@ -38,8 +39,8 @@
from lp.soyuz.enums import ArchivePurpose
-class IDistributionSourcePackage(IBugTarget, IHasBranches, IHasMergeProposals,
- IHasOfficialBugTags,
+class IDistributionSourcePackage(IHeadingContext, IBugTarget, IHasBranches,
+ IHasMergeProposals, IHasOfficialBugTags,
IStructuralSubscriptionTarget,
IQuestionTarget, IHasDrivers):
"""Represents a source package in a distribution.
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2014-11-18 00:07:27 +0000
+++ lib/lp/registry/interfaces/person.py 2014-11-29 04:56:30 +0000
@@ -98,6 +98,7 @@
IHasIcon,
IHasLogo,
IHasMugshot,
+ IHeadingContext,
IPrivacy,
)
from lp.app.validators import LaunchpadValidationError
@@ -1844,7 +1845,7 @@
class IPerson(IPersonPublic, IPersonLimitedView, IPersonViewRestricted,
IPersonEditRestricted, IPersonSpecialRestricted, IHasStanding,
- ISetLocation):
+ ISetLocation, IHeadingContext):
"""A Person."""
export_as_webservice_entry(plural_name='people')
=== modified file 'lib/lp/registry/interfaces/pillar.py'
--- lib/lp/registry/interfaces/pillar.py 2013-01-07 02:40:55 +0000
+++ lib/lp/registry/interfaces/pillar.py 2014-11-29 04:56:30 +0000
@@ -32,6 +32,7 @@
)
from lp import _
+from lp.app.interfaces.launchpad import IHeadingContext
from lp.registry.enums import (
BranchSharingPolicy,
BugSharingPolicy,
@@ -50,7 +51,7 @@
]
-class IPillar(Interface):
+class IPillar(IHeadingContext):
"""An object that might be a project, a project group, or a distribution.
This is a polymorphic object served by the pillar set. Check the
References
