Launchpad itself

Merge lp:~michael.nelson/launchpad/644328-blacklist-ui into lp:launchpad/db-devel

  1. 644328-blacklist-ui
  2. Merge into db-devel
Proposed by Michael Nelson
Status: Merged
Approved by: Michael Nelson
Approved revision: no longer in the source branch.
Merged at revision: 9843
Proposed branch: lp:~michael.nelson/launchpad/644328-blacklist-ui
Merge into: lp:launchpad/db-devel
Diff against target: 774 lines (+414/-67)
15 files modified
lib/canonical/launchpad/icing/style-3-0.css.in (+1/-1)
lib/canonical/launchpad/webapp/interfaces.py (+4/-0)
lib/canonical/launchpad/webapp/servers.py (+23/-19)
lib/canonical/launchpad/webapp/tests/test_servers.py (+14/-0)
lib/lp/registry/browser/distroseriesdifference.py (+50/-4)
lib/lp/registry/browser/tests/test_distroseriesdifference_views.py (+79/-3)
lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py (+25/-0)
lib/lp/registry/browser/tests/test_series_views.py (+0/-9)
lib/lp/registry/interfaces/distroseriesdifference.py (+11/-0)
lib/lp/registry/javascript/distroseriesdifferences_details.js (+122/-28)
lib/lp/registry/model/distroseriesdifference.py (+5/-0)
lib/lp/registry/templates/distroseries-localdifferences.pt (+4/-2)
lib/lp/registry/templates/distroseriesdifference-listing-extra.pt (+12/-0)
lib/lp/registry/tests/test_distroseriesdifference.py (+33/-0)
lib/lp/registry/windmill/tests/test_distroseriesdifference_expander.py (+31/-1)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/644328-blacklist-ui
Reviewer Review Type Date Requested Status
Henning Eggers (community) code Approve
Curtis Hovey (community) ui Approve
Guilherme Salgado (community) ui* Approve
Review via email: mp+36442@code.launchpad.net

Commit message

Adds UI for blacklisting distroseriesdifferences.

Description of the change

Overview
========

This branch implements the UI for blacklisting source package differences for derived distro series.

See the LEP here:
https://dev.launchpad.net/LEP/DerivativeDistributions

and the specific mockup here:
https://dev.launchpad.net/LEP/DerivativeDistributions?action=AttachFile&do=get&target=derived-series-diffs_uiround2.png

You can view a 1min demo of the UI here:

http://people.canonical.com/~michaeln/tmp/644328-blacklist-ui.ogv

updated after salgado's ui review to:
http://people.canonical.com/~michaeln/tmp/644328-review-salgado.ogv

(Note: the issue I mention about the default radio not being selected in chromium was resolved by wrapping the inputs in an empty form element).

To test
=======
bin/test -vv -m test_servers -m test_distroseriesdifference_views -m test_distroseriesdifference_webservice -m test_distroseriesdifference_expander

To demo locally:
================
Run http://pastebin.ubuntu.com/494656/ in bin/iharness and then open
https://launchpad.dev/ubuntu/hoary/+localpackagediffs

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

I forgot to mention, I've ventured slightly from that mockup after some earlier discussions with Henning... that's why 'Add to blacklist' on the mockup has been replaced by two separate links.

Revision history for this message
Guilherme Salgado (salgado) wrote :

<salgado> noodles775, I like the graying out because it makes it easy to see everything that was changed at a glance and maybe undo, but the text (as well as the links) is a bit too verbose (IMHO) and don't allow switching between all different states, so I was thinking of using 3 radio buttons there. "Blacklisted: (*) No, ( ) This version, ( ) All versions"
<salgado> but that's more like a nice-to-have thing, not that important
<noodles775> salgado: that actually sounds a bit like what I'd proposed on the mockup (a single 'Add to blacklist' that would then give you those options), but henninge convinced me on a previous MP that it was a bit too much... hangon, I'll find the discussion.
<salgado> well, except that the radio buttons allow you to see all the possible states without having to click anything
<salgado> just like the links do
<salgado> but sure, maybe he'll be able to convince me as well. :)
<noodles775> salgado: ok - if you search for blacklist at https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739 you'll see a bit of background.
<noodles775> salgado: but I think your radio button option you mention could work too... Also, if you've got suggestions for making the text less verbose while still being easy to understand, let me know :)
<noodles775> s/you/that you/
<salgado> what I like about the radio button is that you see all the options without having to click anything and there's no repetition
<salgado> the downside is that you need vertical real estate, but you seem to have plenty of that
<salgado> as for the text of the link, the only thing I can think of is something like "Blacklist: _this version_ or _all versions_"
<salgado> but I don't quite like that as it seems to suggest you need to blacklist something
<noodles775> salgado: Yeah. RE the radio buttons, I was going to say that it would require two clicks (ie select/submit), but it doesn't of course... we could do the api update when an option is selected quite easily. And that would mean no need for 'Undo'. I like it more and more :)
<salgado> indeed, I hadn't thought about the undo

Revision history for this message
Curtis Hovey (sinzui) wrote :

I radio button the suggestion since it provides context. If I blacklist all version, I can still see an option to black list one version (and know I have made a mistake). This assumes there are three buttons and each fires an ajax event on change.

I think we have a "Undo" problem in Lp. It needs a standard presentation, an icon and rules about where is goes (like "or _Cancel_") so that users know what to expect. As Henning remarked in the other review, the text of the action will vary in size and that could make the list of several open rows hard to scan.

Revision history for this message
Michael Nelson (michael.nelson) wrote :

Hi Guilherme,

Here's an updated screencast based on your UI review:
http://people.canonical.com/~michaeln/tmp/644328-review-salgado.ogv

Let me know if it's what you were thinking.

Revision history for this message
Guilherme Salgado (salgado) wrote :

That works really nice, thanks for doing it, Michael. It'd be important to try and make the default radio button be initially selected on chromium as well, but if it turns out to be too tricky, maybe you can just file a bug for now.

Revision history for this message
Guilherme Salgado (salgado) :
review: Approve (ui*)
Revision history for this message
Curtis Hovey (sinzui) wrote :

This looks great. Thanks Michael and Salgado for doing such a good job on this.

review: Approve (ui)
Revision history for this message
Henning Eggers (henninge) wrote :
Download full text (20.5 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi Michael,
thank you for this cool feature. I don't see any problems with your branch
just a few suggestions some of which we already discussed in IRC

 review approve code

Cheers,
Henning

> === modified file 'lib/canonical/launchpad/icing/style-3-0.css.in'
> === modified file 'lib/canonical/launchpad/webapp/interfaces.py'
> === modified file 'lib/canonical/launchpad/webapp/servers.py'
> --- lib/canonical/launchpad/webapp/servers.py 2010-09-22 15:17:39 +0000
> +++ lib/canonical/launchpad/webapp/servers.py 2010-09-27 15:03:26 +0000
> @@ -526,7 +526,27 @@
> return decoded_qs
>
>
> -class BasicLaunchpadRequest:
> +class LaunchpadBrowserRequestMixin:
> + """A mixin for classes that share some method implementations."""

We already talked about how senseless this comment is ... Can you please
improve it?

> +
> + def getRootURL(self, rootsite):
> + """See IBasicLaunchpadRequest."""
> + if rootsite is not None:
> + assert rootsite in allvhosts.configs, (
> + "rootsite is %s. Must be in %r." % (
> + rootsite, sorted(allvhosts.configs.keys())))
> + root_url = allvhosts.configs[rootsite].rooturl
> + else:
> + root_url = self.getApplicationURL() + '/'
> + return root_url
> +
> + @property
> + def is_ajax(self):
> + """See `IBasicLaunchpadRequest`."""
> + return 'XMLHttpRequest' == self.getHeader('HTTP_X_REQUESTED_WITH')

I was just wondering: How much of a standard is the "X-Requested-With" header?
Is there a fall-back if browser does not send it?

> +
> +
> +class BasicLaunchpadRequest(LaunchpadBrowserRequestMixin):
> """Mixin request class to provide stepstogo."""
>
> implements(IBasicLaunchpadRequest)
> @@ -581,24 +601,8 @@
> return get_query_string_params(self)
>
>
> -class LaunchpadBrowserRequestMixin:
> - """A mixin for classes that share some method implementations."""
> -
> - def getRootURL(self, rootsite):
> - """See IBasicLaunchpadRequest."""
> - if rootsite is not None:
> - assert rootsite in allvhosts.configs, (
> - "rootsite is %s. Must be in %r." % (
> - rootsite, sorted(allvhosts.configs.keys())))
> - root_url = allvhosts.configs[rootsite].rooturl
> - else:
> - root_url = self.getApplicationURL() + '/'
> - return root_url
> -
> -
> class LaunchpadBrowserRequest(BasicLaunchpadRequest, BrowserRequest,
> - NotificationRequest, ErrorReportRequest,
> - LaunchpadBrowserRequestMixin):
> + NotificationRequest, ErrorReportRequest):
> """Integration of launchpad mixin request classes to make an uber
> launchpad request class.
> """
> @@ -1370,7 +1374,7 @@
>
>
> class PublicXMLRPCRequest(BasicLaunchpadRequest, XMLRPCRequest,
> - ErrorReportRequest, LaunchpadBrowserRequestMixin):
> + ErrorReportRequest):
> """Request type for doing public XML-RPC in Launchpad."""
>
> def _createResponse(self):
>
...

review: Approve (code)
Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (11.4 KiB)

On Mon, Sep 27, 2010 at 7:49 PM, Henning Eggers
<email address hidden> wrote:
> Review: Approve code
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi Michael,
> thank you for this cool feature. I don't see any problems with your branch
> just a few suggestions some of which we already discussed in IRC

Cheers Henninge, comments below.

>> === modified file 'lib/canonical/launchpad/webapp/servers.py'
>> --- lib/canonical/launchpad/webapp/servers.py 2010-09-22 15:17:39 +0000
>> +++ lib/canonical/launchpad/webapp/servers.py 2010-09-27 15:03:26 +0000
>> @@ -526,7 +526,27 @@
>>      return decoded_qs
>>
>>
>> -class BasicLaunchpadRequest:
>> +class LaunchpadBrowserRequestMixin:
>> +    """A mixin for classes that share some method implementations."""
>
> We already talked about how senseless this comment is ... Can you please
> improve it?

Yep - I didn't write it (just moved it), but have updated it to:

"""Provides methods used for both API and web browser requests."""

>> +    @property
>> +    def is_ajax(self):
>> +        """See `IBasicLaunchpadRequest`."""
>> +        return 'XMLHttpRequest' == self.getHeader('HTTP_X_REQUESTED_WITH')
>
> I was just wondering: How much of a standard is the "X-Requested-With" header?
> Is there a fall-back if browser does not send it?

It is sent by most ajax libraries when sending xhr requests. The above
implementation matches Django's implementation.

>> === modified file 'lib/canonical/launchpad/webapp/tests/test_servers.py'
>> --- lib/canonical/launchpad/webapp/tests/test_servers.py      2010-08-20 20:31:18 +0000
>> +++ lib/canonical/launchpad/webapp/tests/test_servers.py      2010-09-27 15:03:26 +0000
>> @@ -358,6 +358,16 @@
>>              retried_request.response.getHeader('Vary'),
>>              'Cookie, Authorization')
>>
>> +    def test_is_ajax(self):
>
> Please split this test up:
>       def test_is_not_ajax(self):

Done (test_is_ajax_false).

>>      implements(IConversation)
>> +    schema = IDistroSeriesDifferenceForm
>> +    custom_widget('blacklist_options', RadioWidget)
>> +
>> +    @property
>> +    def initial_values(self):
>> +        """Ensure the correct radio button is checked for blacklisting."""
>> +        if self.context.status in (
>> +            DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
>> +            DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
>> +            ):
>
> My feel is that multi-line if statements are to be avoided at all costs. ;)
> They just read badly. You can construct the tuple first and use that in the if
> statement.

OK, that's much better.
>> +    def test_show_blacklist_options(self):
>> +        # Blacklist options are shown if requested by an editor via
>> +        # ajax.
>> +        ds_diff = self.factory.makeDistroSeriesDifference()
>> +
>> +        # Without JS, even editors don't see blacklist options.
>> +        with person_logged_in(ds_diff.owner):
>> +            view = create_initialized_view(
>> +                ds_diff, '+listing-distroseries-extra')
>> +        self.assertFalse(view.show_blacklist_options)
>
> Yes, as you mentioned, please split this test up as well.

Done - it's now 3 tests.

>> +    def test_bl...

changes_henninge.diff
=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py 2010-09-27 14:00:44 +0000
+++ lib/canonical/launchpad/webapp/servers.py 2010-09-28 06:46:35 +0000
@@ -527,7 +527,7 @@
527527
528528
529class LaunchpadBrowserRequestMixin:529class LaunchpadBrowserRequestMixin:
530 """A mixin for classes that share some method implementations."""530 """Provides methods used for both API and web browser requests."""
531531
532 def getRootURL(self, rootsite):532 def getRootURL(self, rootsite):
533 """See IBasicLaunchpadRequest."""533 """See IBasicLaunchpadRequest."""
534534
=== modified file 'lib/canonical/launchpad/webapp/tests/test_servers.py'
--- lib/canonical/launchpad/webapp/tests/test_servers.py 2010-09-24 15:38:27 +0000
+++ lib/canonical/launchpad/webapp/tests/test_servers.py 2010-09-27 15:33:27 +0000
@@ -358,14 +358,18 @@
358 retried_request.response.getHeader('Vary'),358 retried_request.response.getHeader('Vary'),
359 'Cookie, Authorization')359 'Cookie, Authorization')
360360
361 def test_is_ajax(self):361 def test_is_ajax_false(self):
362 """True if the HTTP_X_REQUESTED_WITH is set appropriately."""362 """Normal requests do not define HTTP_X_REQUESTED_WITH."""
363 request = LaunchpadBrowserRequest(StringIO.StringIO(''), {})363 request = LaunchpadBrowserRequest(StringIO.StringIO(''), {})
364
364 self.assertFalse(request.is_ajax)365 self.assertFalse(request.is_ajax)
365366
367 def test_is_ajax_true(self):
368 """Requests with HTTP_X_REQUESTED_WITH set are ajax requests."""
366 request = LaunchpadBrowserRequest(StringIO.StringIO(''), {369 request = LaunchpadBrowserRequest(StringIO.StringIO(''), {
367 'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest',370 'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest',
368 })371 })
372
369 self.assertTrue(request.is_ajax)373 self.assertTrue(request.is_ajax)
370374
371375
372376
=== modified file 'lib/lp/registry/browser/distroseriesdifference.py'
--- lib/lp/registry/browser/distroseriesdifference.py 2010-09-27 13:46:18 +0000
+++ lib/lp/registry/browser/distroseriesdifference.py 2010-09-28 06:51:57 +0000
@@ -53,10 +53,11 @@
53 @property53 @property
54 def initial_values(self):54 def initial_values(self):
55 """Ensure the correct radio button is checked for blacklisting."""55 """Ensure the correct radio button is checked for blacklisting."""
56 if self.context.status in (56 blacklisted_statuses = (
57 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,57 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
58 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,58 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
59 ):59 )
60 if self.context.status in blacklisted_statuses:
60 return dict(blacklist_options=self.context.status)61 return dict(blacklist_options=self.context.status)
6162
62 return dict(blacklist_options='NONE')63 return dict(blacklist_options='NONE')
6364
=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_views.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-27 14:31:19 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-28 06:56:24 +0000
@@ -8,7 +8,6 @@
8__metaclass__ = type8__metaclass__ = type
99
10from BeautifulSoup import BeautifulSoup10from BeautifulSoup import BeautifulSoup
11from zope.app.form.browser.itemswidgets import RadioWidget
12from zope.component import getUtility11from zope.component import getUtility
1312
14from canonical.launchpad.webapp.servers import LaunchpadTestRequest13from canonical.launchpad.webapp.servers import LaunchpadTestRequest
@@ -127,25 +126,32 @@
127 self.assertIs(None, ds_diff.source_pub)126 self.assertIs(None, ds_diff.source_pub)
128 self.assertIs(None, view.binary_summaries)127 self.assertIs(None, view.binary_summaries)
129128
130 def test_show_blacklist_options(self):129 def test_show_blacklist_options_non_ajax(self):
130 # Blacklist options are not shown for non-ajax requests.
131 ds_diff = self.factory.makeDistroSeriesDifference()
132
133 # Without JS, even editors don't see blacklist options.
134 with person_logged_in(ds_diff.owner):
135 view = create_initialized_view(
136 ds_diff, '+listing-distroseries-extra')
137 self.assertFalse(view.show_blacklist_options)
138
139 def test_show_blacklist_options_editor(self):
131 # Blacklist options are shown if requested by an editor via140 # Blacklist options are shown if requested by an editor via
132 # ajax.141 # ajax.
133 ds_diff = self.factory.makeDistroSeriesDifference()142 ds_diff = self.factory.makeDistroSeriesDifference()
134143
135 # Without JS, even editors don't see blacklist options.
136 with person_logged_in(ds_diff.owner):
137 view = create_initialized_view(
138 ds_diff, '+listing-distroseries-extra')
139 self.assertFalse(view.show_blacklist_options)
140
141 # With a JS request, editors do see blacklist options.
142 request = LaunchpadTestRequest(HTTP_X_REQUESTED_WITH='XMLHttpRequest')144 request = LaunchpadTestRequest(HTTP_X_REQUESTED_WITH='XMLHttpRequest')
143 with person_logged_in(ds_diff.owner):145 with person_logged_in(ds_diff.owner):
144 view = create_initialized_view(146 view = create_initialized_view(
145 ds_diff, '+listing-distroseries-extra', request=request)147 ds_diff, '+listing-distroseries-extra', request=request)
146 self.assertTrue(view.show_blacklist_options)148 self.assertTrue(view.show_blacklist_options)
147149
150 def test_show_blacklist_options_non_editor(self):
148 # Even with a JS request, non-editors do not see the options.151 # Even with a JS request, non-editors do not see the options.
152 ds_diff = self.factory.makeDistroSeriesDifference()
153
154 request = LaunchpadTestRequest(HTTP_X_REQUESTED_WITH='XMLHttpRequest')
149 view = create_initialized_view(155 view = create_initialized_view(
150 ds_diff, '+listing-distroseries-extra', request=request)156 ds_diff, '+listing-distroseries-extra', request=request)
151 self.assertFalse(view.show_blacklist_options)157 self.assertFalse(view.show_blacklist_options)
@@ -256,7 +262,7 @@
256 self.assertEqual(262 self.assertEqual(
257 1, len(soup.findAll('div', {'class': 'blacklist-options'})))263 1, len(soup.findAll('div', {'class': 'blacklist-options'})))
258264
259 def test_blacklist_options_initial_values(self):265 def test_blacklist_options_initial_values_NONE(self):
260 ds_diff = self.factory.makeDistroSeriesDifference()266 ds_diff = self.factory.makeDistroSeriesDifference()
261 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')267 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
262268
@@ -264,6 +270,7 @@
264 # as the default value for the field.270 # as the default value for the field.
265 self.assertEqual('NONE', view.initial_values.get('blacklist_options'))271 self.assertEqual('NONE', view.initial_values.get('blacklist_options'))
266272
273 def test_blacklist_options_initial_values_CURRENT(self):
267 from lp.registry.enum import DistroSeriesDifferenceStatus274 from lp.registry.enum import DistroSeriesDifferenceStatus
268 ds_diff = self.factory.makeDistroSeriesDifference(275 ds_diff = self.factory.makeDistroSeriesDifference(
269 status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)276 status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
@@ -273,6 +280,8 @@
273 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,280 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
274 view.initial_values.get('blacklist_options'))281 view.initial_values.get('blacklist_options'))
275282
283 def test_blacklist_options_initial_values_ALWAYS(self):
284 from lp.registry.enum import DistroSeriesDifferenceStatus
276 ds_diff = self.factory.makeDistroSeriesDifference(285 ds_diff = self.factory.makeDistroSeriesDifference(
277 status=DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS)286 status=DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS)
278 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')287 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
279288
=== modified file 'lib/lp/registry/javascript/distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/distroseriesdifferences_details.js 2010-09-27 10:05:52 +0000
+++ lib/lp/registry/javascript/distroseriesdifferences_details.js 2010-09-28 06:59:24 +0000
@@ -42,7 +42,7 @@
42 var config = {42 var config = {
43 on: {43 on: {
44 success: function(updated_entry, args) {44 success: function(updated_entry, args) {
45 // Let the use know this item is now blacklisted.45 // Let the user know this item is now blacklisted.
46 blacklist_options_container.one('img').remove();46 blacklist_options_container.one('img').remove();
47 blacklist_options_container.all(47 blacklist_options_container.all(
48 'input').set('disabled', false);48 'input').set('disabled', false);
@@ -167,7 +167,6 @@
167 } else {167 } else {
168 details_row = next_row168 details_row = next_row
169 }169 }
170
171 details_row.toggleClass('unseen');170 details_row.toggleClass('unseen');
172 };171 };
173172

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css.in'
--- lib/canonical/launchpad/icing/style-3-0.css.in 2010-09-21 16:02:23 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css.in 2010-09-28 07:13:54 +0000
@@ -2419,7 +2419,7 @@
2419 background-repeat: no-repeat;2419 background-repeat: no-repeat;
2420 background-position:right center;2420 background-position:right center;
2421 }2421 }
2422table#packages_list tr.superseded {2422table#packages_list tr.superseded, tr.blacklisted {
2423 background-color: #eee;2423 background-color: #eee;
2424 }2424 }
2425ul.latest-ppa-updates li:nth-child(odd) {2425ul.latest-ppa-updates li:nth-child(odd) {
24262426
=== modified file 'lib/canonical/launchpad/webapp/interfaces.py'
--- lib/canonical/launchpad/webapp/interfaces.py 2010-09-12 11:43:36 +0000
+++ lib/canonical/launchpad/webapp/interfaces.py 2010-09-28 07:13:54 +0000
@@ -335,6 +335,10 @@
335 query_string_params = Attribute(335 query_string_params = Attribute(
336 'A dictionary of the query string parameters.')336 'A dictionary of the query string parameters.')
337337
338 is_ajax = Bool(
339 title=_('Is ajax'), required=False, readonly=True,
340 description=_("Indicates whether the request is an XMLHttpRequest."))
341
338 def getRootURL(rootsite):342 def getRootURL(rootsite):
339 """Return this request's root URL.343 """Return this request's root URL.
340344
341345
=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py 2010-09-22 15:17:39 +0000
+++ lib/canonical/launchpad/webapp/servers.py 2010-09-28 07:13:54 +0000
@@ -526,7 +526,27 @@
526 return decoded_qs526 return decoded_qs
527527
528528
529class BasicLaunchpadRequest:529class LaunchpadBrowserRequestMixin:
530 """Provides methods used for both API and web browser requests."""
531
532 def getRootURL(self, rootsite):
533 """See IBasicLaunchpadRequest."""
534 if rootsite is not None:
535 assert rootsite in allvhosts.configs, (
536 "rootsite is %s. Must be in %r." % (
537 rootsite, sorted(allvhosts.configs.keys())))
538 root_url = allvhosts.configs[rootsite].rooturl
539 else:
540 root_url = self.getApplicationURL() + '/'
541 return root_url
542
543 @property
544 def is_ajax(self):
545 """See `IBasicLaunchpadRequest`."""
546 return 'XMLHttpRequest' == self.getHeader('HTTP_X_REQUESTED_WITH')
547
548
549class BasicLaunchpadRequest(LaunchpadBrowserRequestMixin):
530 """Mixin request class to provide stepstogo."""550 """Mixin request class to provide stepstogo."""
531551
532 implements(IBasicLaunchpadRequest)552 implements(IBasicLaunchpadRequest)
@@ -581,24 +601,8 @@
581 return get_query_string_params(self)601 return get_query_string_params(self)
582602
583603
584class LaunchpadBrowserRequestMixin:
585 """A mixin for classes that share some method implementations."""
586
587 def getRootURL(self, rootsite):
588 """See IBasicLaunchpadRequest."""
589 if rootsite is not None:
590 assert rootsite in allvhosts.configs, (
591 "rootsite is %s. Must be in %r." % (
592 rootsite, sorted(allvhosts.configs.keys())))
593 root_url = allvhosts.configs[rootsite].rooturl
594 else:
595 root_url = self.getApplicationURL() + '/'
596 return root_url
597
598
599class LaunchpadBrowserRequest(BasicLaunchpadRequest, BrowserRequest,604class LaunchpadBrowserRequest(BasicLaunchpadRequest, BrowserRequest,
600 NotificationRequest, ErrorReportRequest,605 NotificationRequest, ErrorReportRequest):
601 LaunchpadBrowserRequestMixin):
602 """Integration of launchpad mixin request classes to make an uber606 """Integration of launchpad mixin request classes to make an uber
603 launchpad request class.607 launchpad request class.
604 """608 """
@@ -1370,7 +1374,7 @@
13701374
13711375
1372class PublicXMLRPCRequest(BasicLaunchpadRequest, XMLRPCRequest,1376class PublicXMLRPCRequest(BasicLaunchpadRequest, XMLRPCRequest,
1373 ErrorReportRequest, LaunchpadBrowserRequestMixin):1377 ErrorReportRequest):
1374 """Request type for doing public XML-RPC in Launchpad."""1378 """Request type for doing public XML-RPC in Launchpad."""
13751379
1376 def _createResponse(self):1380 def _createResponse(self):
13771381
=== modified file 'lib/canonical/launchpad/webapp/tests/test_servers.py'
--- lib/canonical/launchpad/webapp/tests/test_servers.py 2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/webapp/tests/test_servers.py 2010-09-28 07:13:54 +0000
@@ -358,6 +358,20 @@
358 retried_request.response.getHeader('Vary'),358 retried_request.response.getHeader('Vary'),
359 'Cookie, Authorization')359 'Cookie, Authorization')
360360
361 def test_is_ajax_false(self):
362 """Normal requests do not define HTTP_X_REQUESTED_WITH."""
363 request = LaunchpadBrowserRequest(StringIO.StringIO(''), {})
364
365 self.assertFalse(request.is_ajax)
366
367 def test_is_ajax_true(self):
368 """Requests with HTTP_X_REQUESTED_WITH set are ajax requests."""
369 request = LaunchpadBrowserRequest(StringIO.StringIO(''), {
370 'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest',
371 })
372
373 self.assertTrue(request.is_ajax)
374
361375
362class IThingSet(Interface):376class IThingSet(Interface):
363 """Marker interface for a set of things."""377 """Marker interface for a set of things."""
364378
=== modified file 'lib/lp/registry/browser/distroseriesdifference.py'
--- lib/lp/registry/browser/distroseriesdifference.py 2010-09-17 09:48:32 +0000
+++ lib/lp/registry/browser/distroseriesdifference.py 2010-09-28 07:13:54 +0000
@@ -8,18 +8,59 @@
8 'DistroSeriesDifferenceView',8 'DistroSeriesDifferenceView',
9 ]9 ]
1010
11from zope.interface import implements11from zope.app.form.browser.itemswidgets import RadioWidget
12from zope.interface import (
13 implements,
14 Interface,
15 )
16from zope.schema import Choice
17from zope.schema.vocabulary import (
18 SimpleTerm,
19 SimpleVocabulary,
20 )
1221
13from canonical.launchpad.webapp.publisher import LaunchpadView22from canonical.launchpad.webapp import LaunchpadFormView
23from canonical.launchpad.webapp.authorization import check_permission
24from canonical.launchpad.webapp.launchpadform import custom_widget
25from lp.registry.enum import DistroSeriesDifferenceStatus
14from lp.services.comments.interfaces.conversation import (26from lp.services.comments.interfaces.conversation import (
15 IComment,27 IComment,
16 IConversation,28 IConversation,
17 )29 )
1830
1931
20class DistroSeriesDifferenceView(LaunchpadView):32class IDistroSeriesDifferenceForm(Interface):
33 """An interface used in the browser only for displaying form elements."""
34 blacklist_options = Choice(vocabulary=SimpleVocabulary((
35 SimpleTerm('NONE', 'NONE', 'No'),
36 SimpleTerm(
37 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
38 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS.name,
39 'All versions'),
40 SimpleTerm(
41 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
42 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT.name,
43 'This version'),
44 )))
45
46
47class DistroSeriesDifferenceView(LaunchpadFormView):
2148
22 implements(IConversation)49 implements(IConversation)
50 schema = IDistroSeriesDifferenceForm
51 custom_widget('blacklist_options', RadioWidget)
52
53 @property
54 def initial_values(self):
55 """Ensure the correct radio button is checked for blacklisting."""
56 blacklisted_statuses = (
57 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
58 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
59 )
60 if self.context.status in blacklisted_statuses:
61 return dict(blacklist_options=self.context.status)
62
63 return dict(blacklist_options='NONE')
2364
24 @property65 @property
25 def binary_summaries(self):66 def binary_summaries(self):
@@ -40,11 +81,16 @@
40 @property81 @property
41 def comments(self):82 def comments(self):
42 """See `IConversation`."""83 """See `IConversation`."""
43 # Could use a generator here?
44 return [84 return [
45 DistroSeriesDifferenceDisplayComment(comment) for85 DistroSeriesDifferenceDisplayComment(comment) for
46 comment in self.context.getComments()]86 comment in self.context.getComments()]
4787
88 @property
89 def show_blacklist_options(self):
90 """Only show the options if an editor requests via JS."""
91 return self.request.is_ajax and check_permission(
92 'launchpad.Edit', self.context)
93
4894
49class DistroSeriesDifferenceDisplayComment:95class DistroSeriesDifferenceDisplayComment:
50 """Used simply to provide `IComment` for rendering."""96 """Used simply to provide `IComment` for rendering."""
5197
=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_views.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-17 09:48:32 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_views.py 2010-09-28 07:13:54 +0000
@@ -10,6 +10,7 @@
10from BeautifulSoup import BeautifulSoup10from BeautifulSoup import BeautifulSoup
11from zope.component import getUtility11from zope.component import getUtility
1212
13from canonical.launchpad.webapp.servers import LaunchpadTestRequest
13from canonical.launchpad.webapp.testing import verifyObject14from canonical.launchpad.webapp.testing import verifyObject
14from canonical.testing import (15from canonical.testing import (
15 DatabaseFunctionalLayer,16 DatabaseFunctionalLayer,
@@ -18,14 +19,17 @@
18from lp.registry.browser.distroseriesdifference import (19from lp.registry.browser.distroseriesdifference import (
19 DistroSeriesDifferenceDisplayComment,20 DistroSeriesDifferenceDisplayComment,
20 )21 )
21from lp.registry.enum import DistroSeriesDifferenceType22from lp.registry.enum import (
22from lp.registry.interfaces.distroseriesdifference import IDistroSeriesDifferenceSource23 DistroSeriesDifferenceStatus,
24 DistroSeriesDifferenceType,
25 )
26from lp.registry.interfaces.distroseriesdifference import (
27 IDistroSeriesDifferenceSource)
23from lp.services.comments.interfaces.conversation import (28from lp.services.comments.interfaces.conversation import (
24 IComment,29 IComment,
25 IConversation,30 IConversation,
26 )31 )
27from lp.soyuz.enums import PackagePublishingStatus32from lp.soyuz.enums import PackagePublishingStatus
28from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
29from lp.testing import (33from lp.testing import (
30 celebrity_logged_in,34 celebrity_logged_in,
31 person_logged_in,35 person_logged_in,
@@ -57,6 +61,8 @@
5761
58 def addSummaryToDifference(self, distro_series_difference):62 def addSummaryToDifference(self, distro_series_difference):
59 """Helper that adds binaries with summary info to the source pubs."""63 """Helper that adds binaries with summary info to the source pubs."""
64 # Avoid circular import.
65 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
60 distro_series = distro_series_difference.derived_series66 distro_series = distro_series_difference.derived_series
61 source_package_name_str = (67 source_package_name_str = (
62 distro_series_difference.source_package_name.name)68 distro_series_difference.source_package_name.name)
@@ -124,6 +130,36 @@
124 self.assertIs(None, ds_diff.source_pub)130 self.assertIs(None, ds_diff.source_pub)
125 self.assertIs(None, view.binary_summaries)131 self.assertIs(None, view.binary_summaries)
126132
133 def test_show_blacklist_options_non_ajax(self):
134 # Blacklist options are not shown for non-ajax requests.
135 ds_diff = self.factory.makeDistroSeriesDifference()
136
137 # Without JS, even editors don't see blacklist options.
138 with person_logged_in(ds_diff.owner):
139 view = create_initialized_view(
140 ds_diff, '+listing-distroseries-extra')
141 self.assertFalse(view.show_blacklist_options)
142
143 def test_show_blacklist_options_editor(self):
144 # Blacklist options are shown if requested by an editor via
145 # ajax.
146 ds_diff = self.factory.makeDistroSeriesDifference()
147
148 request = LaunchpadTestRequest(HTTP_X_REQUESTED_WITH='XMLHttpRequest')
149 with person_logged_in(ds_diff.owner):
150 view = create_initialized_view(
151 ds_diff, '+listing-distroseries-extra', request=request)
152 self.assertTrue(view.show_blacklist_options)
153
154 def test_show_blacklist_options_non_editor(self):
155 # Even with a JS request, non-editors do not see the options.
156 ds_diff = self.factory.makeDistroSeriesDifference()
157
158 request = LaunchpadTestRequest(HTTP_X_REQUESTED_WITH='XMLHttpRequest')
159 view = create_initialized_view(
160 ds_diff, '+listing-distroseries-extra', request=request)
161 self.assertFalse(view.show_blacklist_options)
162
127163
128class DistroSeriesDifferenceTemplateTestCase(TestCaseWithFactory):164class DistroSeriesDifferenceTemplateTestCase(TestCaseWithFactory):
129165
@@ -215,3 +251,43 @@
215 1, len(soup.findAll('pre', text="I'm working on this.")))251 1, len(soup.findAll('pre', text="I'm working on this.")))
216 self.assertEqual(252 self.assertEqual(
217 1, len(soup.findAll('pre', text="Here's another comment.")))253 1, len(soup.findAll('pre', text="Here's another comment.")))
254
255 def test_blacklist_options(self):
256 # blacklist options are presented to editors.
257 ds_diff = self.factory.makeDistroSeriesDifference()
258
259 with person_logged_in(ds_diff.owner):
260 request = LaunchpadTestRequest(
261 HTTP_X_REQUESTED_WITH='XMLHttpRequest')
262 view = create_initialized_view(
263 ds_diff, '+listing-distroseries-extra', request=request)
264 soup = BeautifulSoup(view())
265
266 self.assertEqual(
267 1, len(soup.findAll('div', {'class': 'blacklist-options'})))
268
269 def test_blacklist_options_initial_values_NONE(self):
270 ds_diff = self.factory.makeDistroSeriesDifference()
271 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
272
273 # If the difference is not currently blacklisted, 'NONE' is set
274 # as the default value for the field.
275 self.assertEqual('NONE', view.initial_values.get('blacklist_options'))
276
277 def test_blacklist_options_initial_values_CURRENT(self):
278 ds_diff = self.factory.makeDistroSeriesDifference(
279 status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
280 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
281
282 self.assertEqual(
283 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
284 view.initial_values.get('blacklist_options'))
285
286 def test_blacklist_options_initial_values_ALWAYS(self):
287 ds_diff = self.factory.makeDistroSeriesDifference(
288 status=DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS)
289 view = create_initialized_view(ds_diff, '+listing-distroseries-extra')
290
291 self.assertEqual(
292 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
293 view.initial_values.get('blacklist_options'))
218294
=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-09-21 11:17:25 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_webservice.py 2010-09-28 07:13:54 +0000
@@ -59,3 +59,28 @@
59 self.assertEqual(59 self.assertEqual(
60 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,60 DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT,
61 ds_diff.status)61 ds_diff.status)
62
63 def test_unblacklist_not_public(self):
64 # The unblacklist method is not publically available.
65 ds_diff = self.factory.makeDistroSeriesDifference(
66 status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
67 ws_diff = ws_object(self.factory.makeLaunchpadService(), ds_diff)
68
69 self.assertRaises(Unauthorized, ws_diff.unblacklist)
70
71 def test_unblacklist(self):
72 # The unblacklist method can be called by people with edit access.
73 ds_diff = self.factory.makeDistroSeriesDifference(
74 status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
75 ws_diff = ws_object(self.factory.makeLaunchpadService(
76 ds_diff.owner), ds_diff)
77
78 result = ws_diff.unblacklist()
79 transaction.commit()
80
81 utility = getUtility(IDistroSeriesDifferenceSource)
82 ds_diff = utility.getByDistroSeriesAndName(
83 ds_diff.derived_series, ds_diff.source_package_name.name)
84 self.assertEqual(
85 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
86 ds_diff.status)
6287
=== modified file 'lib/lp/registry/browser/tests/test_series_views.py'
--- lib/lp/registry/browser/tests/test_series_views.py 2010-09-21 08:42:29 +0000
+++ lib/lp/registry/browser/tests/test_series_views.py 2010-09-28 07:13:54 +0000
@@ -73,15 +73,6 @@
73 name=derived_name, parent_series=parent)73 name=derived_name, parent_series=parent)
74 return derived_series74 return derived_series
7575
76 def makeControllerInScopes(self, scopes):
77 """Make a controller that will report it's in the given scopes."""
78 call_log = []
79
80 def scope_cb(scope):
81 call_log.append(scope)
82 return scope in scopes
83 return FeatureController(scope_cb), call_log
84
85 def setDerivedSeriesUIFeatureFlag(self):76 def setDerivedSeriesUIFeatureFlag(self):
86 # Helper to set the feature flag enabling the derived series ui.77 # Helper to set the feature flag enabling the derived series ui.
87 ignore = getFeatureStore().add(FeatureFlag(78 ignore = getFeatureStore().add(FeatureFlag(
8879
=== modified file 'lib/lp/registry/interfaces/distroseriesdifference.py'
--- lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-20 15:00:11 +0000
+++ lib/lp/registry/interfaces/distroseriesdifference.py 2010-09-28 07:13:54 +0000
@@ -17,10 +17,12 @@
17 export_as_webservice_entry,17 export_as_webservice_entry,
18 export_write_operation,18 export_write_operation,
19 exported,19 exported,
20 operation_parameters,
20 )21 )
21from lazr.restful.fields import Reference22from lazr.restful.fields import Reference
22from zope.interface import Interface23from zope.interface import Interface
23from zope.schema import (24from zope.schema import (
25 Bool,
24 Choice,26 Choice,
25 Int,27 Int,
26 TextLine,28 TextLine,
@@ -135,6 +137,8 @@
135 def addComment(owner, comment):137 def addComment(owner, comment):
136 """Add a comment on this difference."""138 """Add a comment on this difference."""
137139
140 @operation_parameters(
141 all=Bool(title=_("All"), required=False))
138 @export_write_operation()142 @export_write_operation()
139 def blacklist(all=False):143 def blacklist(all=False):
140 """Blacklist this version or all versions of this source package.144 """Blacklist this version or all versions of this source package.
@@ -143,6 +147,13 @@
143 be blacklisted or just the current (default).147 be blacklisted or just the current (default).
144 """148 """
145149
150 @export_write_operation()
151 def unblacklist():
152 """Removes this difference from the blacklist.
153
154 The status will be updated based on the versions.
155 """
156
146157
147class IDistroSeriesDifference(IDistroSeriesDifferencePublic,158class IDistroSeriesDifference(IDistroSeriesDifferencePublic,
148 IDistroSeriesDifferenceEdit):159 IDistroSeriesDifferenceEdit):
149160
=== modified file 'lib/lp/registry/javascript/distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/distroseriesdifferences_details.js 2010-09-15 15:05:11 +0000
+++ lib/lp/registry/javascript/distroseriesdifferences_details.js 2010-09-28 07:13:54 +0000
@@ -11,40 +11,130 @@
1111
12var namespace = Y.namespace('lp.registry.distroseriesdifferences_details');12var namespace = Y.namespace('lp.registry.distroseriesdifferences_details');
1313
14/**
15 * Create one Launchpad client that will be used with multiple requests.
16 */
17var lp_client = new LP.client.Launchpad();
18
14/*19/*
15 * Setup the expandable rows for each difference.20 * Setup the expandable rows for each difference.
16 *21 *
17 * @method setup_expandable_rows22 * @method setup_expandable_rows
18 */23 */
19namespace.setup_expandable_rows = function() {24namespace.setup_expandable_rows = function() {
20 var start_update = function(uri, container) {25
26 var blacklist_handler = function(e, api_uri, source_name) {
27 // We only want to select the new radio if the update is
28 // successful.
29 e.preventDefault();
30 var blacklist_options_container = this.ancestor('div');
31
32 // Disable all the inputs
33 blacklist_options_container.all('input').set('disabled', 'disabled');
34 e.target.prepend('<img src="/@@/spinner" />');
35
36 var method_name = (e.target.get('value') == 'NONE') ?
37 'unblacklist' : 'blacklist';
38 var blacklist_all = (e.target.get('value') == 'BLACKLISTED_ALWAYS');
39
40 var diff_rows = Y.all('tr.' + source_name);
41
42 var config = {
43 on: {
44 success: function(updated_entry, args) {
45 // Let the user know this item is now blacklisted.
46 blacklist_options_container.one('img').remove();
47 blacklist_options_container.all(
48 'input').set('disabled', false);
49 e.target.set('checked', true);
50 Y.each(diff_rows, function(diff_row) {
51 var fade_to_gray = new Y.Anim({
52 node: diff_row,
53 from: { backgroundColor: '#FFFFFF'},
54 to: { backgroundColor: '#EEEEEE'}
55 });
56 if (method_name == 'unblacklist') {
57 fade_to_gray.set('reverse', true);
58 }
59 fade_to_gray.run();
60 });
61 },
62 failure: function(id, response) {
63 blacklist_options_container.one('img').remove();
64 blacklist_options_container.all(
65 'input').set('disabled', false);
66 }
67 },
68 parameters: {
69 all: blacklist_all
70 }
71 };
72
73 lp_client.named_post(api_uri, method_name, config);
74
75 };
76
77 /**
78 * Link the click event for these blacklist options to the correct
79 * api uri.
80 *
81 * @param blacklist_options {Node} The node containing the blacklist
82 * options.
83 * @param source_name {string} The name of the source to update.
84 */
85 var setup_blacklist_options = function(blacklist_options,
86 source_name) {
87 var api_uri = [
88 LP.client.cache.context.self_link,
89 '+difference',
90 source_name
91 ].join('/')
92 Y.on('click', blacklist_handler, blacklist_options.all('input'),
93 blacklist_options, api_uri, source_name);
94 };
95
96 /**
97 * Get the extra information for this diff to display.
98 *
99 * @param uri {string} The uri for the extra diff info.
100 * @param container {Node} A node which must contain a div with the
101 * class 'diff-extra-container' into which the results
102 * are inserted.
103 */
104 var get_extra_diff_info = function(uri, container, source_name) {
21105
22 var in_progress_message = Y.lp.soyuz.base.makeInProgressNode(106 var in_progress_message = Y.lp.soyuz.base.makeInProgressNode(
23 'Fetching difference details ...')107 'Fetching difference details ...')
24 container.insert(in_progress_message, 'replace');108 container.one('div.diff-extra-container').insert(
109 in_progress_message, 'replace');
110
111 var success_cb = function(transaction_id, response, args) {
112 args.container.one('div.diff-extra-container').insert(
113 response.responseText, 'replace');
114 setup_blacklist_options(args.container.one(
115 'div.blacklist-options'), source_name);
116 };
117
118 var failure_cb = function(transaction_id, response, args){
119 var retry_handler = function(e) {
120 e.preventDefault();
121 get_extra_diff_info(args.uri, args.container);
122 };
123 var failure_message = Y.lp.soyuz.base.makeFailureNode(
124 'Failed to fetch difference details.',
125 retry_handler);
126 args.container.insert(failure_message, 'replace');
127
128 var anim = Y.lazr.anim.red_flash({
129 node: args.container
130 });
131 anim.run();
132 };
25133
26 var config = {134 var config = {
27 on: {135 on: {
28 'success': function(transaction_id, response, args) {136 'success': success_cb,
29 args.container.set(137 'failure': failure_cb,
30 'innerHTML', response.responseText);
31 // Change to insert(,'replace)
32 },
33 'failure': function(transaction_id, response, args){
34 var retry_handler = function(e) {
35 e.preventDefault();
36 start_update(args.uri, args.container);
37 };
38 var failure_message = Y.lp.soyuz.base.makeFailureNode(
39 'Failed to fetch difference details.',
40 retry_handler);
41 args.container.insert(failure_message, 'replace');
42
43 var anim = Y.lazr.anim.red_flash({
44 node: args.container
45 });
46 anim.run();
47 }
48 },138 },
49 arguments: {139 arguments: {
50 'container': container,140 'container': container,
@@ -64,18 +154,22 @@
64 // Only insert if there isn't already a container row there.154 // Only insert if there isn't already a container row there.
65 next_row = row.next();155 next_row = row.next();
66 if (next_row == null || !next_row.hasClass('diff-extra')) {156 if (next_row == null || !next_row.hasClass('diff-extra')) {
67 details_row = Y.Node.create(157 var source_name = row.one('a.toggle-extra').get('text');
68 '<table><tr class="diff-extra unseen"><td colspan="5"></td></tr></table>').one('tr');158 var details_row = Y.Node.create([
159 '<table><tr class="diff-extra unseen ' + source_name + '">',
160 ' <td colspan="5">',
161 ' <div class="diff-extra-container"></div>',
162 ' </td></tr></table>'
163 ].join('')).one('tr');
69 row.insert(details_row, 'after');164 row.insert(details_row, 'after');
70 var uri = toggle.get('href');165 var uri = toggle.get('href');
71 start_update(uri, details_row.one('td'));166 get_extra_diff_info(uri, details_row.one('td'), source_name);
72 } else {167 } else {
73 details_row = next_row168 details_row = next_row
74 }169 }
75
76 details_row.toggleClass('unseen');170 details_row.toggleClass('unseen');
171 };
77172
78 };
79 Y.all('table.listing a.toggle-extra').each(function(toggle){173 Y.all('table.listing a.toggle-extra').each(function(toggle){
80 toggle.addClass('treeCollapsed').addClass('sprite');174 toggle.addClass('treeCollapsed').addClass('sprite');
81 toggle.on("click", expander_handler);175 toggle.on("click", expander_handler);
@@ -83,4 +177,4 @@
83177
84};178};
85179
86}, "0.1", {"requires": ["io-base", "lp.soyuz.base"]});180}, "0.1", {"requires": ["io-base", "lp.soyuz.base", "lazr.anim"]});
87181
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2010-09-21 11:17:25 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2010-09-28 07:13:54 +0000
@@ -272,3 +272,8 @@
272 self.status = DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS272 self.status = DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS
273 else:273 else:
274 self.status = DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT274 self.status = DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT
275
276 def unblacklist(self):
277 """See `IDistroSeriesDifference`."""
278 self.status = DistroSeriesDifferenceStatus.NEEDS_ATTENTION
279 self.update()
275280
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2010-09-16 09:32:35 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2010-09-28 07:13:54 +0000
@@ -44,7 +44,8 @@
44 <tal:difference repeat="difference differences/batch">44 <tal:difference repeat="difference differences/batch">
45 <tr tal:define="parent_source_pub difference/parent_source_pub;45 <tr tal:define="parent_source_pub difference/parent_source_pub;
46 source_pub difference/source_pub;46 source_pub difference/source_pub;
47 signer source_pub/sourcepackagerelease/uploader/fmt:link|nothing;">47 signer source_pub/sourcepackagerelease/uploader/fmt:link|nothing;"
48 tal:attributes="class parent_source_pub/source_package_name">
48 <td><a tal:attributes="href difference/fmt:url" class="toggle-extra"49 <td><a tal:attributes="href difference/fmt:url" class="toggle-extra"
49 tal:content="parent_source_pub/source_package_name">Foo</a>50 tal:content="parent_source_pub/source_package_name">Foo</a>
50 </td>51 </td>
@@ -52,7 +53,8 @@
52 <tal:replace53 <tal:replace
53 replace="parent_source_pub/sourcepackagerelease/version"/></a>54 replace="parent_source_pub/sourcepackagerelease/version"/></a>
54 </td>55 </td>
55 <td><a tal:attributes="href source_pub/sourcepackagerelease/fmt:url">56 <td><a tal:attributes="href source_pub/sourcepackagerelease/fmt:url"
57 class="derived-version">
56 <tal:replace58 <tal:replace
57 replace="source_pub/sourcepackagerelease/version"/></a>59 replace="source_pub/sourcepackagerelease/version"/></a>
58 </td>60 </td>
5961
=== modified file 'lib/lp/registry/templates/distroseriesdifference-listing-extra.pt'
--- lib/lp/registry/templates/distroseriesdifference-listing-extra.pt 2010-09-17 09:48:32 +0000
+++ lib/lp/registry/templates/distroseriesdifference-listing-extra.pt 2010-09-28 07:13:54 +0000
@@ -3,6 +3,18 @@
3 xmlns:metal="http://xml.zope.org/namespaces/metal"3 xmlns:metal="http://xml.zope.org/namespaces/metal"
4 xmlns:i18n="http://xml.zope.org/namespaces/i18n"4 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
5 i18n:domain="launchpad">5 i18n:domain="launchpad">
6
7 <tal:show_options condition="view/show_blacklist_options">
8 <div class="blacklist-options" style="float:right">
9 <dl>
10 <dt>Blacklisted:</dt>
11 <dd>
12 <form><tal:replace replace="structure view/widgets/blacklist_options" /></form>
13 </dd>
14 </dl>
15 </div>
16 </tal:show_options>
17
6 <dl>18 <dl>
7 <dt>Binary descriptions:</dt>19 <dt>Binary descriptions:</dt>
8 <dd><ul>20 <dd><ul>
921
=== modified file 'lib/lp/registry/tests/test_distroseriesdifference.py'
--- lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-20 15:00:11 +0000
+++ lib/lp/registry/tests/test_distroseriesdifference.py 2010-09-28 07:13:54 +0000
@@ -369,6 +369,39 @@
369 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,369 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
370 ds_diff.status)370 ds_diff.status)
371371
372 def test_unblacklist(self):
373 # Unblacklisting will return to NEEDS_ATTENTION by default.
374 ds_diff = self.factory.makeDistroSeriesDifference(
375 status=DistroSeriesDifferenceStatus.BLACKLISTED_CURRENT)
376
377 with person_logged_in(ds_diff.owner):
378 ds_diff.unblacklist()
379
380 self.assertEqual(
381 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
382 ds_diff.status)
383
384 def test_unblacklist_resolved(self):
385 # Status is resolved when unblacklisting a now-resolved difference.
386 ds_diff = self.factory.makeDistroSeriesDifference(
387 versions={
388 'derived': '0.9',
389 'parent': '1.0',
390 },
391 status=DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS)
392 new_derived_pub = self.factory.makeSourcePackagePublishingHistory(
393 sourcepackagename=ds_diff.source_package_name,
394 distroseries=ds_diff.derived_series,
395 status=PackagePublishingStatus.PENDING,
396 version='1.0')
397
398 with person_logged_in(ds_diff.owner):
399 ds_diff.unblacklist()
400
401 self.assertEqual(
402 DistroSeriesDifferenceStatus.RESOLVED,
403 ds_diff.status)
404
372 def test_source_package_name_unique_for_derived_series(self):405 def test_source_package_name_unique_for_derived_series(self):
373 # We cannot create two differences for the same derived series406 # We cannot create two differences for the same derived series
374 # for the same package.407 # for the same package.
375408
=== modified file 'lib/lp/registry/windmill/tests/test_distroseriesdifference_expander.py'
--- lib/lp/registry/windmill/tests/test_distroseriesdifference_expander.py 2010-09-16 14:02:30 +0000
+++ lib/lp/registry/windmill/tests/test_distroseriesdifference_expander.py 2010-09-28 07:13:54 +0000
@@ -1,10 +1,15 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4from zope.component import getUtility
5
4import transaction6import transaction
57
6from canonical.launchpad.webapp.publisher import canonical_url8from canonical.launchpad.webapp.publisher import canonical_url
7from canonical.launchpad.windmill.testing import constants9from canonical.launchpad.windmill.testing import constants
10from canonical.launchpad.windmill.testing import lpuser
11from lp.registry.enum import DistroSeriesDifferenceStatus
12from lp.registry.interfaces.distroseriesdifference import IDistroSeriesDifferenceSource
8from lp.registry.windmill.testing import RegistryWindmillLayer13from lp.registry.windmill.testing import RegistryWindmillLayer
9from lp.services.features.model import FeatureFlag, getFeatureStore14from lp.services.features.model import FeatureFlag, getFeatureStore
10from lp.testing import WindmillTestCase15from lp.testing import WindmillTestCase
@@ -32,11 +37,36 @@
32 self.package_diffs_url = (37 self.package_diffs_url = (
33 canonical_url(self.diff.derived_series) + '/+localpackagediffs')38 canonical_url(self.diff.derived_series) + '/+localpackagediffs')
3439
35 def test_diff_extra_details_available(self):40 def test_diff_extra_details_blacklisting(self):
36 """A successful request for the extra info updates the display."""41 """A successful request for the extra info updates the display."""
42 #login_person(self.diff.owner, 'test', self.client)
43 lpuser.FOO_BAR.ensure_login(self.client)
37 self.client.open(url=self.package_diffs_url)44 self.client.open(url=self.package_diffs_url)
38 self.client.waits.forPageLoad(timeout=constants.PAGE_LOAD)45 self.client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
39 self.client.click(link=u'foo')46 self.client.click(link=u'foo')
40 self.client.waits.forElement(47 self.client.waits.forElement(
41 classname=u'diff-extra', timeout=constants.FOR_ELEMENT)48 classname=u'diff-extra', timeout=constants.FOR_ELEMENT)
4249
50 self.client.click(id=u'field.blacklist_options.1')
51 self.client.waits.forElementProperty(
52 option=u'enabled', id=u'field.blacklist_options.1')
53
54 # Reload the diff and ensure it's been updated.
55 transaction.commit()
56 diff_source = getUtility(IDistroSeriesDifferenceSource)
57 diff_reloaded = diff_source.getByDistroSeriesAndName(
58 self.diff.derived_series, 'foo')
59 self.assertEqual(
60 DistroSeriesDifferenceStatus.BLACKLISTED_ALWAYS,
61 diff_reloaded.status)
62
63 # Now set it back so that it's not blacklisted.
64 self.client.click(id=u'field.blacklist_options.0')
65 self.client.waits.forElementProperty(
66 option=u'enabled', id=u'field.blacklist_options.0')
67 transaction.commit()
68 diff_reloaded = diff_source.getByDistroSeriesAndName(
69 self.diff.derived_series, 'foo')
70 self.assertEqual(
71 DistroSeriesDifferenceStatus.NEEDS_ATTENTION,
72 diff_reloaded.status)

Subscribers

People subscribed via source and target branches

to status/vote changes: