launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01455
[Merge] lp:~allenap/launchpad/sub-search-ui-bug-656823 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/sub-search-ui-bug-656823 into lp:launchpad with lp:~allenap/launchpad/wire-up-filter-subs-bug-655567 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#656823 Subscribing to a search lacks a UI
https://bugs.launchpad.net/bugs/656823
This isn't directly relevant to the bug, but is some stuff I wanted to
do before I begin making bigger changes.
- PersonSubscriptionsView only needs to inherit from LaunchpadView.
- I also simplified it a bit, and added a couple of XXXs to mark out
some potentially inefficient bits.
- I prettified the +subscriptions template, added lower navigation
links, fixed it to only display navigation links if there are more
items than fit on the page, removed an illegal id (a static id in a
repeated block), and generally tidied up.
This is targeted to db-devel but I may land these changes in devel.
--
https://code.launchpad.net/~allenap/launchpad/sub-search-ui-bug-656823/+merge/37976
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/sub-search-ui-bug-656823 into lp:launchpad.
=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py 2010-10-07 23:49:13 +0000
+++ lib/lp/registry/browser/person.py 2010-10-08 14:39:45 +0000
@@ -206,10 +206,6 @@
from canonical.launchpad.webapp.login import logoutPerson
from canonical.launchpad.webapp.menu import get_current_view
from canonical.launchpad.webapp.publisher import LaunchpadView
-from lp.app.browser.tales import (
- DateTimeFormatterAPI,
- PersonFormatterAPI,
- )
from canonical.lazr.utils import smartquote
from canonical.widgets import (
LaunchpadDropdownWidget,
@@ -226,6 +222,10 @@
from lp.answers.interfaces.questionenums import QuestionParticipation
from lp.answers.interfaces.questionsperson import IQuestionsPerson
from lp.app.browser.stringformatter import FormattersAPI
+from lp.app.browser.tales import (
+ DateTimeFormatterAPI,
+ PersonFormatterAPI,
+ )
from lp.app.errors import (
NotFoundError,
UnexpectedFormData,
@@ -2344,14 +2344,16 @@
return self.getSearchPageHeading()
-class PersonSubscriptionsView(BugTaskSearchListingView):
+class PersonSubscriptionsView(LaunchpadView):
"""All the subscriptions for a person."""
page_title = 'Subscriptions'
def subscribedBugTasks(self):
- """Return a BatchNavigator for distinct bug tasks to which the
- person is subscribed."""
+ """
+ Return a BatchNavigator for distinct bug tasks to which the person is
+ subscribed.
+ """
bug_tasks = self.context.searchTasks(None, user=self.user,
order_by='-date_last_updated',
status=(BugTaskStatus.NEW,
@@ -2364,38 +2366,37 @@
bug_subscriber=self.context)
sub_bug_tasks = []
- sub_bugs = []
+ sub_bugs = set()
+ # XXX: GavinPanella 2010-10-08 bug=656904: This materializes the
+ # entire result set. It would probably be more efficient implemented
+ # with a pre_iter_hook on a DecoratedResultSet.
for task in bug_tasks:
+ # We order the bugtasks by date_last_updated but we always display
+ # the default task for the bug. This is to avoid ordering issues
+ # in tests and also prevents user confusion (because nothing is
+ # more confusing than your subscription targets changing seemingly
+ # at random).
if task.bug not in sub_bugs:
- # We order the bugtasks by date_last_updated but we
- # always display the default task for the bug. This is
- # to avoid ordering issues in tests and also prevents
- # user confusion (because nothing is more confusing than
- # your subscription targets changing seemingly at
- # random).
+ # XXX: GavinPanella 2010-10-08 bug=656904: default_bugtask
+ # causes a query to be executed. It would be more efficient to
+ # get the default bugtask in bulk, in a pre_iter_hook on a
+ # DecoratedResultSet perhaps.
sub_bug_tasks.append(task.bug.default_bugtask)
- sub_bugs.append(task.bug)
+ sub_bugs.add(task.bug)
+
return BatchNavigator(sub_bug_tasks, self.request)
def canUnsubscribeFromBugTasks(self):
- viewed_person = self.context
- if self.user is None:
- return False
- elif viewed_person == self.user:
- return True
- elif (viewed_person.is_team and
- self.user.inTeam(viewed_person)):
- return True
+ """Can the current user unsubscribe from the bug tasks shown?"""
+ return (self.user is not None and
+ self.user.inTeam(self.context))
- def getSubscriptionsPageHeading(self):
+ @property
+ def label(self):
"""The header for the subscriptions page."""
return "Subscriptions for %s" % self.context.displayname
- @property
- def label(self):
- return self.getSubscriptionsPageHeading()
-
class PersonVouchersView(LaunchpadFormView):
"""Form for displaying and redeeming commercial subscription vouchers."""
=== modified file 'lib/lp/registry/templates/person-subscriptions.pt'
--- lib/lp/registry/templates/person-subscriptions.pt 2010-09-28 20:55:37 +0000
+++ lib/lp/registry/templates/person-subscriptions.pt 2010-10-08 14:39:45 +0000
@@ -1,71 +1,75 @@
<html
- xmlns="http://www.w3.org/1999/xhtml";
- xmlns:tal="http://xml.zope.org/namespaces/tal";
- xmlns:metal="http://xml.zope.org/namespaces/metal";
- xmlns:i18n="http://xml.zope.org/namespaces/i18n";
- xml:lang="en"
- lang="en"
- dir="ltr"
- metal:use-macro="view/macro:page/main_side"
- i18n:domain="launchpad"
->
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ xml:lang="en"
+ lang="en"
+ dir="ltr"
+ metal:use-macro="view/macro:page/main_side"
+ i18n:domain="launchpad">
<body>
<div metal:fill-slot="main"
- tal:define="sub_bugs view/subscribedBugTasks;
- sub_bug_batch sub_bugs/currentBatch">
+ tal:define="display_name context/fmt:displayname;
+ sub_bugs view/subscribedBugTasks;
+ sub_bug_batch sub_bugs/currentBatch">
+
<tal:no_subscribed_bugs condition="not: sub_bug_batch|nothing">
- <p><tal:person content="context/fmt:displayname">Sample
- Person</tal:person> does not have any direct bug subscriptions.</p>
+ <p>
+ <tal:person content="display_name">Sample
+ Person</tal:person> does not have any direct bug subscriptions.
+ </p>
</tal:no_subscribed_bugs>
<tal:subscribed_bugs condition="sub_bug_batch|nothing">
<p>
- Direct bug subscriptions for
- <tal:person content="context/fmt:displayname">Sample Person</tal:person>
+ Direct bug subscriptions for
+ <tal:person content="display_name">Sample Person</tal:person>
</p>
- <div tal:replace="structure sub_bugs/@@+navigation-links-lower"/>
- <table id="bug_subscriptions" class="listing">
- <thead>
- <tr>
- <th colspan="3">Summary</th>
- <th>
- In
- </th>
- <th tal:condition="view/canUnsubscribeFromBugTasks">
- </th>
- </tr>
- </thead>
- <tr tal:repeat="sub_bug_task sub_bug_batch">
- <td class="icon left">
- <span tal:replace="structure sub_bug_task/image:icon" />
- </td>
- <td id="bug_number" style="text-align: right">
- <span tal:replace="sub_bug_task/bug/id" />
- </td>
- <td>
- <a tal:attributes="href sub_bug_task/fmt:url"
- tal:content="sub_bug_task/bug/title" />
- </td>
- <td tal:content="sub_bug_task/bugtargetdisplayname"
- tal:condition="sub_bug_task/bugtargetdisplayname|nothing"
- >mozilla-firefox (Ubuntu)</td>
- <td tal:condition="view/canUnsubscribeFromBugTasks"
- align="center">
- <a
- tal:attributes="
- title string:Unsubscribe ${context/fmt:displayname} from bug ${sub_bug_task/bug/id};
- id string:unsubscribe-subscriber-${context/id};
- href string:${sub_bug_task/fmt:url}/+subscribe;
- "
- >
+ <tal:multipage condition="sub_bugs/has_multiple_pages">
+ <tal:navigation
+ replace="structure sub_bugs/@@+navigation-links-upper"/>
+ </tal:multipage>
+ <table id="bug_subscriptions" class="listing">
+ <thead>
+ <tr>
+ <th colspan="3">Summary</th>
+ <th>
+ In
+ </th>
+ <th tal:condition="view/canUnsubscribeFromBugTasks">
+ </th>
+ </tr>
+ </thead>
+ <tr tal:repeat="sub_bug_task sub_bug_batch">
+ <td class="icon left">
+ <span tal:replace="structure sub_bug_task/image:icon" />
+ </td>
+ <td style="text-align: right">
+ <span tal:replace="sub_bug_task/bug/id" />
+ </td>
+ <td>
+ <a tal:attributes="href sub_bug_task/fmt:url"
+ tal:content="sub_bug_task/bug/title" />
+ </td>
+ <td tal:content="sub_bug_task/bugtargetdisplayname"
+ tal:condition="sub_bug_task/bugtargetdisplayname|nothing" />
+ <td tal:condition="view/canUnsubscribeFromBugTasks" align="center">
+ <a tal:attributes="id string:unsubscribe-subscriber-${context/id};
+ href sub_bug_task/fmt:url/+subscribe;
+ title string:Unsubscribe ${display_name} from bug ${sub_bug_task/bug/id};">
<img src="/@@/edit"
- tal:attributes="id string:unsubscribe-icon-${context/id};
- position string:relative"/>
+ tal:attributes="id string:unsubscribe-icon-${context/id};
+ position string:relative"/>
</a>
</td>
- </tr>
- </table>
+ </tr>
+ </table>
+ <tal:multipage condition="sub_bugs/has_multiple_pages">
+ <tal:navigation
+ replace="structure sub_bugs/@@+navigation-links-lower"/>
+ </tal:multipage>
</tal:subscribed_bugs>
</div>
