launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03730
[Merge] lp:~gary/launchpad/bug-772754-1 into lp:launchpad/db-devel
Gary Poster has proposed merging lp:~gary/launchpad/bug-772754-1 into lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #772754 in Launchpad itself: "After better-bug-notification changes, list of bug subscribers is confusing"
https://bugs.launchpad.net/launchpad/+bug/772754
For more details, see:
https://code.launchpad.net/~gary/launchpad/bug-772754-1/+merge/62185
This branch simply rearranges the bug subscription portlet to coincide with the basic plan for fixing bug 772754 (if you want to see the mockup, a somewhat old one that is on the right track is https://launchpadlibrarian.net/71552495/all-in-one.png). Subsequent branches will change the interior of the subscription and subscriber portlet more significantly.
Lint is happy for the Python files. It is not happy for the CSS files, because of properties it doesn't know about and because of some line length issues. I did not touch the CSS changes because I believe that the properties are intended, and I'm paranoid that the line length issues might be there for a reason as well, and I'm being lazy (at least for the line length issues). If you really want me to make the line length issues, they will be easy enough to do.
Thanks
Gary
--
https://code.launchpad.net/~gary/launchpad/bug-772754-1/+merge/62185
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug-772754-1 into lp:launchpad/db-devel.
=== modified file 'database/schema/security.cfg'
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css 2011-05-11 18:05:10 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css 2011-05-24 18:46:06 +0000
@@ -1366,9 +1366,13 @@
background: #fbfbfb;
}
.side h2 {
- font-size: 16px; /* BAD: the markup should be h3? */
+ font-size: 16px;
line-height: 20px;
}
+.side h3 {
+ font-size: 14px;
+ line-height: 18px;
+ }
.side ul {
background: #fbfbfb;
}
=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css 2011-04-11 16:12:18 +0000
+++ lib/canonical/launchpad/icing/style.css 2011-05-24 18:46:06 +0000
@@ -550,6 +550,11 @@
background-image: url(/@@/spinner) !important;
}
+/* This lets us have a help link sprite and a normal link sprite on the same
+ row, which the .vertical .sprite rule in the 3.0 file does not allow. */
+div#mute-link-container a.sprite {
+ width: auto;
+}
/* === Translations === */
=== modified file 'lib/lp/answers/browser/question.py'
=== modified file 'lib/lp/answers/stories/webservice.txt'
=== modified file 'lib/lp/blueprints/model/sprint.py'
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2011-04-28 12:51:56 +0000
+++ lib/lp/bugs/browser/configure.zcml 2011-05-24 18:46:06 +0000
@@ -535,6 +535,12 @@
for="lp.bugs.interfaces.bug.IBug"
class="lp.bugs.browser.bug.BugView"
permission="launchpad.View"
+ name="+portlet-subscription"
+ template="../templates/bug-portlet-subscription.pt"/>
+ <browser:page
+ for="lp.bugs.interfaces.bug.IBug"
+ class="lp.bugs.browser.bug.BugView"
+ permission="launchpad.View"
name="+portlet-subscribers"
template="../templates/bug-portlet-subscribers.pt"/>
<browser:page
=== modified file 'lib/lp/bugs/browser/tests/test_bug_context_menu.py'
--- lib/lp/bugs/browser/tests/test_bug_context_menu.py 2011-05-17 12:00:48 +0000
+++ lib/lp/bugs/browser/tests/test_bug_context_menu.py 2011-05-24 18:46:06 +0000
@@ -79,6 +79,6 @@
request = LaunchpadTestRequest()
request.features = get_relevant_feature_controller()
view = create_initialized_view(
- self.bug, name="+portlet-subscribers", request=request)
+ self.bug, name="+portlet-subscription", request=request)
html = view.render()
self.assertTrue('class="sprite maybe mute-help"' in html)
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py 2011-05-24 17:41:47 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py 2011-05-24 18:46:06 +0000
@@ -97,14 +97,14 @@
request = LaunchpadTestRequest()
request.features = get_relevant_feature_controller()
view = create_initialized_view(
- self.bug, name="+portlet-subscribers", request=request)
+ self.bug, name="+portlet-subscription", request=request)
html = view.render()
self.assertTrue('menu-link-editsubscriptions' in html)
self.assertTrue('/+subscriptions' in html)
def test_edit_subscriptions_link_not_shown_when_feature_disabled(self):
view = create_initialized_view(
- self.bug, name="+portlet-subscribers")
+ self.bug, name="+portlet-subscription")
html = view.render()
self.assertTrue('menu-link-editsubscriptions' not in html)
self.assertTrue('/+subscriptions' not in html)
@@ -115,7 +115,7 @@
with person_logged_in(person):
with FeatureFixture({self.feature_flag_1: None}):
view = create_initialized_view(
- self.bug, name="+portlet-subscribers")
+ self.bug, name="+portlet-subscription")
self.assertFalse(view.user_should_see_mute_link)
html = view.render()
self.assertFalse('mute_subscription' in html)
@@ -136,7 +136,7 @@
self.target.addBugSubscription(person, person)
self.assertFalse(self.bug.isMuted(person))
view = create_initialized_view(
- self.bug, name="+portlet-subscribers")
+ self.bug, name="+portlet-subscription")
self.assertTrue(view.user_should_see_mute_link,
"User should see mute link.")
contents = view.render()
@@ -166,7 +166,7 @@
self.bug.personIsAlsoNotifiedSubscriber(
person), "Person should be a notified subscriber")
view = create_initialized_view(
- self.bug, name="+portlet-subscribers")
+ self.bug, name="+portlet-subscription")
self.assertTrue(view.user_should_see_mute_link,
"User should see mute link.")
contents = view.render()
@@ -193,7 +193,7 @@
self.bug.personIsAlsoNotifiedSubscriber(
person))
view = create_initialized_view(
- self.bug, name="+portlet-subscribers")
+ self.bug, name="+portlet-subscription")
self.assertFalse(view.user_should_see_mute_link)
html = view.render()
self.assertTrue('mute_subscription' in html)
@@ -202,6 +202,7 @@
self.assertTrue(
self._hasCSSClass(html, 'mute-link-container', 'hidden'),
'No "hidden" CSS class in mute-link-container.')
+<<<<<<< TREE
def test_mute_subscription_link_shown_if_muted(self):
# If a person is muted but not otherwise subscribed, they should still
@@ -226,3 +227,29 @@
self.assertFalse(
self._hasCSSClass(
contents, 'mute-link-container', 'hidden'))
+=======
+
+ def test_mute_subscription_link_shown_if_muted(self):
+ # If a person is muted but not otherwise subscribed, they should still
+ # see the (un)mute link.
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ with FeatureFixture({self.feature_flag_1: 'on'}):
+ self.bug.mute(person, person)
+ # The user isn't subscribed already, but is muted.
+ self.assertFalse(self.bug.isSubscribed(person))
+ self.assertFalse(
+ self.bug.personIsAlsoNotifiedSubscriber(
+ person))
+ self.assertTrue(self.bug.isMuted(person))
+ view = create_initialized_view(
+ self.bug, name="+portlet-subscription")
+ self.assertTrue(view.user_should_see_mute_link,
+ "User should see mute link.")
+ contents = view.render()
+ self.assertTrue('mute_subscription' in contents,
+ "'mute_subscription' not in contents.")
+ self.assertFalse(
+ self._hasCSSClass(
+ contents, 'mute-link-container', 'hidden'))
+>>>>>>> MERGE-SOURCE
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-05-23 12:18:15 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-05-24 18:46:06 +0000
@@ -439,8 +439,16 @@
# its form is submitted and the bug was already muted.
with person_logged_in(self.person):
self.bug.mute(self.person, self.person)
+<<<<<<< TREE
self.assertTrue(self.bug.isMuted(self.person))
create_initialized_view(
self.bug.default_bugtask, name="+mute",
form={'field.actions.unmute': 'Unmute bug mail'})
self.assertFalse(self.bug.isMuted(self.person))
+=======
+ self.assertTrue(self.bug.isMuted(self.person))
+ create_initialized_view(
+ self.bug.default_bugtask, name="+mute",
+ form={'field.actions.mute': 'Unmute bug mail'})
+ self.assertTrue(self.bug.isMuted(self.person))
+>>>>>>> MERGE-SOURCE
=== modified file 'lib/lp/bugs/interfaces/bug.py'
=== modified file 'lib/lp/bugs/javascript/subscription.js'
--- lib/lp/bugs/javascript/subscription.js 2011-05-24 04:30:46 +0000
+++ lib/lp/bugs/javascript/subscription.js 2011-05-24 18:46:06 +0000
@@ -34,7 +34,7 @@
namespace._reasons = reasons;
/* These are components for team participation. */
-var _OF_TEAM = 'of the team {team}. That team is ';
+var _OF_TEAM = 'of the team {team}, which is ';
var _OF_TEAMS = 'of the teams {teams}. Those teams are ';
var _BECAUSE_TEAM_IS = _BECAUSE_YOU_ARE + 'a member ' + _OF_TEAM;
var _ADMIN_BECAUSE_TEAM_IS = (
@@ -293,7 +293,7 @@
'innerHTML',
safely_render_description(
{reason: 'Contact {teams} to request the administrators '+
- 'make a change',
+ 'make a change.',
vars: {
teams: add_url_element_to_links(
args.teams, '/+contactuser')}}));
@@ -1212,10 +1212,14 @@
.addClass('subscription-description');
var action_node = subscription.action(subscription.args);
if (Y.Lang.isValue(action_node)) {
- node.appendChild(action_node.setStyle('float', 'right'));
+ var div = Y.Node.create('<div />');
+ div.appendChild(action_node);
+ div.setStyle('float', 'right')
+ .setStyle('paddingLeft', '3em');
+ node.appendChild(div);
}
node.appendChild(
- Y.Node.create('<div></div>')
+ Y.Node.create('<div />')
.addClass('description-text')
.set('innerHTML',
safely_render_description(subscription, extra_data)));
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-05-23 09:13:32 +0000
+++ lib/lp/bugs/model/bug.py 2011-05-24 18:46:06 +0000
@@ -969,6 +969,12 @@
See the comment in getDirectSubscribers for a description of the
recipients argument.
"""
+ if self.private:
+ # We short-circuit for private bugs, since actually
+ # returning something non-empty here causes things to break
+ # in fun and interesting ways (see bug 780248).
+ return []
+
if level is None:
level = BugNotificationLevel.LIFECYCLE
info = self.getSubscriptionInfo(level)
=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers-content.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers-content.pt 2011-05-18 18:34:19 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers-content.pt 2011-05-24 18:46:06 +0000
@@ -8,7 +8,7 @@
direct_subscriptions view/sorted_direct_subscriptions;
">
<div class="section" id="subscribers-direct">
- <h2>Subscribers</h2>
+ <h3>Directly subscribed</h3>
<div id="subscribers-links">
<div
tal:condition="direct_subscriptions"
@@ -64,7 +64,7 @@
id="subscribers-indirect"
class="Section"
>
- <h2>Also notified</h2>
+ <h3>Also notified</h3>
<div
tal:repeat="subscriber also_notified_subscribers"
tal:content="structure subscriber/fmt:link"
=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers.pt 2011-05-10 11:30:46 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers.pt 2011-05-24 18:46:06 +0000
@@ -6,26 +6,9 @@
id="portlet-subscribers"
metal:define-macro="custom"
>
- <div class="section" tal:define="context_menu context/menu:context"
- metal:define-slot="heading">
- <div
- tal:attributes="class view/current_user_subscription_class"
- tal:content="structure context_menu/subscription/render" />
- <div id="sub-unsub-spinner">Subscribing...</div>
+ <h2>Other bug subscribers</h2>
+ <div tal:define="context_menu context/menu:context">
<div tal:content="structure context_menu/addsubscriber/render" />
- <div tal:condition="request/features/malone.advanced-structural-subscriptions.enabled"
- tal:content="structure context_menu/editsubscriptions/render" />
- <tal:show-mute condition="
- request/features/malone.advanced-subscriptions.enabled">
- <div tal:attributes="class view/current_user_mute_class"
- id="mute-link-container">
- <span tal:replace="structure context_menu/mute_subscription/render"/>
- <a target="help" class="sprite maybe mute-help"
- href="/+help/subscription-mute.html"
- > <span class="invisible-link">Mute help</span></a>
- <div style="float: left" id="mute-unmute-spinner">Unmuting...</div>
- </div>
- </tal:show-mute>
</div>
<a id="subscribers-ids-link"
tal:define="bug context/bug|context"
@@ -37,23 +20,4 @@
style="text-align: center; display: none">
<img src="/@@/spinner" />
</div>
- <script type="text/javascript">
- LPS.use('io-base', 'node', 'lp.bugs.bugtask_index.portlets', function(Y) {
- // Must be done inline here to ensure the load event fires.
- // This is a work around for a YUI3 issue with event handling.
- var subscription_link = Y.one('.menu-link-subscription');
- var subscription_link_handler;
- if (subscription_link) {
- subscription_link_handler = subscription_link.on(
- 'click', function(e) { e.preventDefault(); });
- }
-
- Y.on('domready', function() {
- if (Y.lp.bugs.bugtask_index.portlets) {
- Y.lp.bugs.bugtask_index.portlets.load_subscribers_portlet(
- subscription_link, subscription_link_handler);
- }
- });
- });
- </script>
</div>
=== added file 'lib/lp/bugs/templates/bug-portlet-subscription.pt'
--- lib/lp/bugs/templates/bug-portlet-subscription.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscription.pt 2011-05-24 18:46:06 +0000
@@ -0,0 +1,49 @@
+<div
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ class="portlet vertical"
+ id="portlet-subscription"
+ metal:define-macro="custom"
+>
+ <div class="section" tal:define="context_menu context/menu:context"
+ metal:define-slot="heading">
+ <div
+ tal:attributes="class view/current_user_subscription_class"
+ tal:content="structure context_menu/subscription/render" />
+ <div id="sub-unsub-spinner">Subscribing...</div>
+ <div tal:condition="request/features/malone.advanced-structural-subscriptions.enabled"
+ tal:content="structure context_menu/editsubscriptions/render" />
+ <tal:show-mute condition="
+ request/features/malone.advanced-subscriptions.enabled">
+ <div tal:attributes="class view/current_user_mute_class"
+ id="mute-link-container">
+ <span tal:replace="structure context_menu/mute_subscription/render"
+ /> <a target="help" class="sprite maybe mute-help"
+ href="/+help/subscription-mute.html"
+ > <span class="invisible-link">Mute help</span></a>
+ <div style="float: left" id="mute-unmute-spinner">Unmuting...</div>
+ </div>
+ </tal:show-mute>
+ </div>
+ <script type="text/javascript">
+ LPS.use('io-base', 'node',
+ 'lp.bugs.bugtask_index.portlets', function(Y) {
+ // Must be done inline here to ensure the load event fires.
+ // This is a work around for a YUI3 issue with event handling.
+ var subscription_link = Y.one('.menu-link-subscription');
+ var subscription_link_handler;
+ if (subscription_link) {
+ subscription_link_handler = subscription_link.on(
+ 'click', function(e) { e.preventDefault(); });
+ }
+
+ Y.on('domready', function() {
+ if (Y.lp.bugs.bugtask_index.portlets) {
+ Y.lp.bugs.bugtask_index.portlets.load_subscribers_portlet(
+ subscription_link, subscription_link_handler);
+ }
+ });
+ });
+ </script>
+</div>
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2011-05-16 15:29:15 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2011-05-24 18:46:06 +0000
@@ -80,6 +80,7 @@
<div tal:replace="structure context/bug/@@+portlet-privacy" />
<div tal:replace="structure context/bug/@@+portlet-actions" />
<div tal:replace="structure context/bug/@@+portlet-duplicates" />
+ <div tal:replace="structure context/bug/@@+portlet-subscription" />
<div tal:replace="structure context/bug/@@+portlet-subscribers" />
<div tal:replace="structure context/bug/@@+portlet-questions" />
<div tal:replace="structure context/bug/@@+portlet-specs" />
=== modified file 'lib/lp/bugs/tests/test_bug.py'
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2011-05-24 10:08:33 +0000
+++ lib/lp/registry/browser/distroseries.py 2011-05-24 18:46:06 +0000
@@ -853,6 +853,7 @@
return (has_perm and
self.cached_differences.batch.total() > 0)
+<<<<<<< TREE
@cachedproperty
def pending_syncs(self):
"""Pending synchronization jobs for this distroseries.
@@ -898,6 +899,27 @@
return (
not self.isNewerThanParent(dsd) and not self.hasPendingSync(dsd))
+=======
+ @cachedproperty
+ def pending_syncs(self):
+ """Pending synchronization jobs for this distroseries.
+
+ :return: A dict mapping (name, version) package specifications to
+ pending sync jobs.
+ """
+ job_source = getUtility(IPlainPackageCopyJobSource)
+ return job_source.getPendingJobsPerPackage(self.context)
+
+ def hasPendingSync(self, dsd):
+ """Is there a package-copying job pending to resolve `dsd`?"""
+ return self.pending_syncs.get(specify_dsd_package(dsd)) is not None
+
+ def canRequestSync(self, dsd):
+ """Does it make sense to request a sync for this difference?"""
+ # XXX JeroenVermeulen bug=783435: Also compare versions.
+ return not self.hasPendingSync(dsd)
+
+>>>>>>> MERGE-SOURCE
@property
def specified_name_filter(self):
"""If specified, return the name filter from the GET form data."""
=== modified file 'lib/lp/registry/browser/distroseriesdifference.py'
=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py 2011-05-24 14:22:31 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-05-24 18:46:06 +0000
@@ -1345,6 +1345,7 @@
self.assertFalse(view.canPerformSync())
+<<<<<<< TREE
def test_hasPendingSync_returns_False_if_no_pending_sync(self):
dsd = self.factory.makeDistroSeriesDifference()
view = create_initialized_view(
@@ -1423,6 +1424,34 @@
dsd.derived_series, '+localpackagediffs')
self.assertTrue(view.canRequestSync(dsd))
+=======
+ def test_hasPendingSync_returns_False_if_no_pending_sync(self):
+ dsd = self.factory.makeDistroSeriesDifference()
+ view = create_initialized_view(
+ dsd.derived_series, '+localpackagediffs')
+ self.assertFalse(view.hasPendingSync(dsd))
+
+ def test_hasPendingSync_returns_True_if_pending_sync(self):
+ dsd = self.factory.makeDistroSeriesDifference()
+ view = create_initialized_view(
+ dsd.derived_series, '+localpackagediffs')
+ view.pending_syncs = {specify_dsd_package(dsd): object()}
+ self.assertTrue(view.hasPendingSync(dsd))
+
+ def test_canRequestSync_returns_False_if_pending_sync(self):
+ dsd = self.factory.makeDistroSeriesDifference()
+ view = create_initialized_view(
+ dsd.derived_series, '+localpackagediffs')
+ view.pending_syncs = {specify_dsd_package(dsd): object()}
+ self.assertFalse(view.canRequestSync(dsd))
+
+ def test_canRequestSync_returns_True_if_sync_makes_sense(self):
+ dsd = self.factory.makeDistroSeriesDifference()
+ view = create_initialized_view(
+ dsd.derived_series, '+localpackagediffs')
+ self.assertTrue(view.canRequestSync(dsd))
+
+>>>>>>> MERGE-SOURCE
def _syncAndGetView(self, derived_series, person, sync_differences,
difference_type=None, view_name='+localpackagediffs'):
# A helper to get the POST'ed sync view.
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2011-05-24 11:32:34 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2011-05-24 18:46:06 +0000
@@ -77,6 +77,7 @@
tal:attributes="class src_name">
<td>
+<<<<<<< TREE
<tal:checkbox
condition="can_perform_sync"
define="can_request python:view.canRequestSync(difference)">
@@ -89,7 +90,16 @@
tal:condition="not: can_request"
type="checkbox" disabled="disabled" />
</tal:checkbox>
+=======
+ <tal:checkbox tal:condition="python: view.canRequestSync(difference)">
+ <input tal:condition="can_perform_sync"
+ name="field.selected_differences" type="checkbox"
+ tal:attributes="
+ value diff_id;
+ id string:field.selected_differences.${diff_id}"/>
+>>>>>>> MERGE-SOURCE
+<<<<<<< TREE
<a tal:attributes="href difference/fmt:url"
class="js-action toggle-extra"
tal:content="src_name">Foo</a>
@@ -99,6 +109,16 @@
tal:condition="python: view.hasPendingSync(difference)">
synchronizing…
</span>
+=======
+ <a tal:attributes="href difference/fmt:url"
+ class="js-action toggle-extra"
+ tal:content="src_name">Foo</a>
+ </tal:checkbox>
+ <tal:clockface condition="python: view.hasPendingSync(difference)">
+ <span class="sprite build_depwait"></span>
+ <i>(Synchronizing)</i>
+ </tal:clockface>
+>>>>>>> MERGE-SOURCE
</td>
<td tal:condition="python: not(view.has_unique_parent) and view.show_parent_version">
<a tal:attributes="href difference/parent_series/fmt:url"
=== modified file 'lib/lp/soyuz/browser/tests/test_package_copying_mixin.py'
=== modified file 'lib/lp/soyuz/model/archive.py'
=== modified file 'lib/lp/soyuz/model/queue.py'
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-05-24 17:41:47 +0000
+++ lib/lp/testing/factory.py 2011-05-24 18:46:06 +0000
@@ -420,9 +420,15 @@
The string returned will always be a valid name that can be used in
Launchpad URLs.
+<<<<<<< TREE
:param prefix: Used as a prefix for the unique string. If
unspecified, generates a name starting with 'unique' and
mentioning the calling source location.
+=======
+ :param prefix: Used as a prefix for the unique string. If
+ unspecified, generates a name starting with 'unique' and
+ mentioning the calling source location.
+>>>>>>> MERGE-SOURCE
"""
if prefix is None:
frame = sys._getframe(2)
=== modified file 'lib/lp/testing/views.py'
Follow ups
