Launchpad itself

Merge lp:~michael.nelson/launchpad/652838-select-diffs-for-syncing into lp:launchpad/db-devel

  1. 652838-select-diffs-for-syncing
  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: 9863
Proposed branch: lp:~michael.nelson/launchpad/652838-select-diffs-for-syncing
Merge into: lp:launchpad/db-devel
Diff against target: 489 lines (+264/-51)
4 files modified
lib/canonical/launchpad/templates/launchpad-form.pt (+4/-3)
lib/lp/registry/browser/distroseries.py (+76/-2)
lib/lp/registry/browser/tests/test_series_views.py (+161/-43)
lib/lp/registry/templates/distroseries-localdifferences.pt (+23/-3)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/652838-select-diffs-for-syncing
Reviewer Review Type Date Requested Status
Curtis Hovey (community) ui Approve
Guilherme Salgado (community) ui* Approve
Jeroen T. Vermeulen (community) code Approve
Review via email: mp+37572@code.launchpad.net

Commit message

Allow multiple differences to be selected for syncing.

Description of the change

Overview
========
This branch fixes bug 652838 by adding (non-js) check boxes and the sync button for syncing differences.

Note: it does not add the actual sync operation itself, but just a stub action. Julian will be organising the actual operation at at later point (obviously before the feature is enabled).

UI demo
=======
http://people.canonical.com/~michaeln/tmp/652838-select-diffs.ogv

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

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

Details
=======
I had 2 issues, and one ongoing:

1) There doesn't seem to be a natural way to have a dynamic name for the action as required by the prototype at:

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

You'll see the solution in the view's initialize() method, but any better options would be gratefully accepted.

2) There doesn't seem to be a natural way to have dynamic vocabularies based on the view. I've looked at various ways that we've done this in the past, and none are ideal.

In translations a vocabulary factory is used (see lp.translations.browser.translationimportqueue.TranslationImportQueueView), but vocabulary factories are designed to be based on the context, not the view (they implement IContextSourceBinder), so translations adds an __init__ kwarg, which means the vocabulary factory can't be instantiated in the interface description as it normally would, which means no LaunchpadFormView with its schema etc.

In soyuz an extra field and widget is added to the form during view initialisation (ie. one that is not on the schema, see lp.soyuz.browser.archive.ArchiveSourceSelectionFormView)

I decided instead to include the field on the schema with an empty vocabulary, and then update the vocabulary during the view's setUpFields() call. But again, any better ideas are welcome.

3) Finally, two tests where I render the view and check tags are failing, it seems, because the traversal request/lp:person is returning None:
http://pastebin.ubuntu.com/506335/

I'm still looking into why this is. So far, tracked it down to the fact that, even within the person_logged_in context manager, the tests view.request.principal is zope.principalregistry.principalregistry.UnauthenticatedPrincipal.

To test
=======

bin/test -vvm test_series_views

I'll upload a quick screencast soon.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

It's just a crying shame that you have to go to these lengths to include dynamic information in your view. I looked for alternatives but didn't find any.

You mentioned that you'd talk to the zope people; I think that's a good idea. If only Zope's @action accepted a callable as an alternative to a string!

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

The UI changes look good to me. I think it'd be nice to have an extra checkbox for selecting all/none versions, though.

review: Approve (ui*)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

> The UI changes look good to me. I think it'd be nice to have an extra
> checkbox for selecting all/none versions, though.

salgado: thanks. And yes, the all/none checkbox is included on the mockup, I hope to add it in a later branch, but it's not a priority right now.

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

The behaviour is nice. Buttons should be in title case (except for subordinating conjunctions and commonly used prepositions):
    Sync selected %s versions into => Sync Selected %s Versions into %s

This may look odd since the interpolated names may not be title case, but the user did that, not us.

review: Approve (ui)
Revision history for this message
Leonard Richardson (leonardr) wrote :

FYI, I just did a branch with dynamically-named buttons:

https://code.edge.launchpad.net/~leonardr/launchpad/rename-grant-permissions/+merge/37590

Basically, in the action code I created my own Actions object and copied in new Action objects. My Actions object was based on a class Actions object containing generic actions.

Revision history for this message
Gary Poster (gary) wrote :

I asked Leonard to comment on your (1), which he did, above. formlib actions are unpleasantly limited.

For (2), context-based vocabularies are what I expected you to use, but I see they are not sufficient for some reason. Without looking at the code more carefully, I don't know why. In their original usage, views are expected to be only a presentation of the context/model, so the context should be sufficient. I'm not sure why views are more than a presentation here, but would be interested to hear why. For the short term though, unless there's a nice way to refactor to actually use the context, hacks are the order of the day. :-(

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

On Tue, Oct 5, 2010 at 6:18 PM, Gary Poster <email address hidden> wrote:
> I asked Leonard to comment on your (1), which he did, above.  formlib actions are unpleasantly limited.

Thanks Gary and Leonard.

>
> For (2), context-based vocabularies are what I expected you to use, but I see they are not sufficient for some reason.  Without looking at the code more carefully, I don't know why.  In their original usage, views are expected to be only a presentation of the context/model, so the context should be sufficient.  I'm not sure why views are more than a presentation here, but would be interested to hear why.  For the short term though, unless there's a nice way to refactor to actually use the context, hacks are the order of the day. :-(

In this case, the context is a distro series, the view is displaying a
batched (and soon to be filtered) list of items related to that
distroseries, which can be selected for an operation. Another similar
situation that comes to mind is copying packages for an archive. The
view's context is the archive, but the view lists the related batched
(and filtered) packages.

In both cases we've manually created a field with a vocabulary
matching the items listed on screen so that validation and the action
can only operate on the current batch of items. Does that make sense,
or is there a better way?

> --
> https://code.launchpad.net/~michael.nelson/launchpad/652838-select-diffs-for-syncing/+merge/37572
> You are the owner of lp:~michael.nelson/launchpad/652838-select-diffs-for-syncing.
>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/templates/launchpad-form.pt'
--- lib/canonical/launchpad/templates/launchpad-form.pt 2010-06-22 17:12:23 +0000
+++ lib/canonical/launchpad/templates/launchpad-form.pt 2010-10-05 15:06:16 +0000
@@ -76,9 +76,10 @@
76 </div>76 </div>
77 <div class="actions" id="launchpad-form-actions"77 <div class="actions" id="launchpad-form-actions"
78 metal:define-slot="buttons">78 metal:define-slot="buttons">
79 <input tal:repeat="action view/actions"79 <tal:actions repeat="action view/actions">
80 tal:replace="structure action/render"80 <input tal:replace="structure action/render"
81 />81 tal:condition="action/available"/>
82 </tal:actions>
82 <tal:has-cancel-link condition="view/cancel_url">83 <tal:has-cancel-link condition="view/cancel_url">
83 <tal:or condition="view/has_available_actions">or&nbsp;</tal:or>84 <tal:or condition="view/has_available_actions">or&nbsp;</tal:or>
84 <a tal:attributes="href view/cancel_url">Cancel</a>85 <a tal:attributes="href view/cancel_url">Cancel</a>
8586
=== modified file 'lib/lp/registry/browser/distroseries.py'
--- lib/lp/registry/browser/distroseries.py 2010-09-21 08:42:29 +0000
+++ lib/lp/registry/browser/distroseries.py 2010-10-05 15:06:16 +0000
@@ -21,8 +21,12 @@
21from zope.component import getUtility21from zope.component import getUtility
22from zope.event import notify22from zope.event import notify
23from zope.formlib import form23from zope.formlib import form
24from zope.interface import Interface
24from zope.lifecycleevent import ObjectCreatedEvent25from zope.lifecycleevent import ObjectCreatedEvent
25from zope.schema import Choice26from zope.schema import (
27 Choice,
28 List,
29 )
26from zope.schema.vocabulary import (30from zope.schema.vocabulary import (
27 SimpleTerm,31 SimpleTerm,
28 SimpleVocabulary,32 SimpleVocabulary,
@@ -59,6 +63,7 @@
59 stepthrough,63 stepthrough,
60 stepto,64 stepto,
61 )65 )
66from canonical.widgets import LabeledMultiCheckBoxWidget
62from canonical.widgets.itemswidgets import LaunchpadDropdownWidget67from canonical.widgets.itemswidgets import LaunchpadDropdownWidget
63from lp.app.errors import NotFoundError68from lp.app.errors import NotFoundError
64from lp.blueprints.browser.specificationtarget import (69from lp.blueprints.browser.specificationtarget import (
@@ -536,8 +541,18 @@
536 return navigator541 return navigator
537542
538543
539class DistroSeriesLocalDifferences(LaunchpadView):544class IDifferencesFormSchema(Interface):
545 selected_differences = List(
546 title=_('Selected differences'),
547 value_type=Choice(vocabulary=SimpleVocabulary([])),
548 description=_("Select the differences for syncing."),
549 required=True)
550
551
552class DistroSeriesLocalDifferences(LaunchpadFormView):
540 """Present differences between a derived series and its parent."""553 """Present differences between a derived series and its parent."""
554 schema = IDifferencesFormSchema
555 custom_widget('selected_differences', LabeledMultiCheckBoxWidget)
541556
542 page_title = 'Local package differences'557 page_title = 'Local package differences'
543558
@@ -546,6 +561,13 @@
546 if not getFeatureFlag('soyuz.derived-series-ui.enabled'):561 if not getFeatureFlag('soyuz.derived-series-ui.enabled'):
547 self.request.response.redirect(canonical_url(self.context))562 self.request.response.redirect(canonical_url(self.context))
548 return563 return
564
565 # Update the label for sync action.
566 self.__class__.actions.byname['actions.sync'].label = (
567 "Sync Selected %s Versions into %s" % (
568 self.context.parent_series.displayname,
569 self.context.displayname,
570 ))
549 super(DistroSeriesLocalDifferences, self).initialize()571 super(DistroSeriesLocalDifferences, self).initialize()
550572
551 @property573 @property
@@ -563,3 +585,55 @@
563 utility = getUtility(IDistroSeriesDifferenceSource)585 utility = getUtility(IDistroSeriesDifferenceSource)
564 differences = utility.getForDistroSeries(self.context)586 differences = utility.getForDistroSeries(self.context)
565 return BatchNavigator(differences, self.request)587 return BatchNavigator(differences, self.request)
588
589 def setUpFields(self):
590 """Add the selected differences field.
591
592 As this field depends on other search/filtering field values
593 for its own vocabulary, we set it up after all the others.
594 """
595 super(DistroSeriesLocalDifferences, self).setUpFields()
596 has_edit = check_permission('launchpad.Edit', self.context)
597
598 terms = [
599 SimpleTerm(diff, diff.source_package_name.name,
600 diff.source_package_name.name)
601 for diff in self.cached_differences.batch]
602 diffs_vocabulary = SimpleVocabulary(terms)
603 choice = self.form_fields['selected_differences'].field.value_type
604 choice.vocabulary = diffs_vocabulary
605
606 @action(_("Sync Sources"), name="sync", validator='validate_sync',
607 condition='canPerformSync')
608 def sync_sources(self, action, data):
609 """Mark the diffs as syncing and request the sync.
610
611 Currently this is a stub operation, the details of which will
612 be implemented later.
613 """
614 selected_differences = data['selected_differences']
615 diffs = [
616 diff.source_package_name.name
617 for diff in selected_differences]
618
619 self.request.response.addNotification(
620 "The following sources would have been synced if this "
621 "wasn't just a stub operation: " + ", ".join(diffs))
622
623 self.next_url = self.request.URL
624
625 def validate_sync(self, action, data):
626 """Validate selected differences."""
627 form.getWidgetsData(self.widgets, self.prefix, data)
628
629 if len(data.get('selected_differences', [])) == 0:
630 self.setFieldError(
631 'selected_differences', 'No differences selected.')
632
633 def canPerformSync(self, *args):
634 """Return whether a sync can be performed.
635
636 This method is used as a condition for the above sync action, as
637 well as directly in the template.
638 """
639 return check_permission('launchpad.Edit', self.context)
566640
=== modified file 'lib/lp/registry/browser/tests/test_series_views.py'
--- lib/lp/registry/browser/tests/test_series_views.py 2010-09-21 15:52:01 +0000
+++ lib/lp/registry/browser/tests/test_series_views.py 2010-10-05 15:06:16 +0000
@@ -1,6 +1,8 @@
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 __future__ import with_statement
5
4__metaclass__ = type6__metaclass__ = type
57
6from BeautifulSoup import BeautifulSoup8from BeautifulSoup import BeautifulSoup
@@ -14,7 +16,10 @@
14from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities16from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
15from canonical.launchpad.webapp.batching import BatchNavigator17from canonical.launchpad.webapp.batching import BatchNavigator
16from canonical.launchpad.webapp.publisher import canonical_url18from canonical.launchpad.webapp.publisher import canonical_url
17from canonical.testing import LaunchpadZopelessLayer19from canonical.testing import (
20 LaunchpadZopelessLayer,
21 LaunchpadFunctionalLayer,
22 )
18from lp.registry.enum import (23from lp.registry.enum import (
19 DistroSeriesDifferenceStatus,24 DistroSeriesDifferenceStatus,
20 DistroSeriesDifferenceType,25 DistroSeriesDifferenceType,
@@ -28,7 +33,10 @@
28 getFeatureFlag,33 getFeatureFlag,
29 per_thread,34 per_thread,
30 )35 )
31from lp.testing import TestCaseWithFactory36from lp.testing import (
37 TestCaseWithFactory,
38 person_logged_in,
39 )
32from lp.testing.views import create_initialized_view40from lp.testing.views import create_initialized_view
3341
3442
@@ -61,40 +69,35 @@
61 self.assertEqual(view.needs_linking, None)69 self.assertEqual(view.needs_linking, None)
6270
6371
72def set_derived_series_ui_feature_flag(test_case):
73 # Helper to set the feature flag enabling the derived series ui.
74 ignore = getFeatureStore().add(FeatureFlag(
75 scope=u'default', flag=u'soyuz.derived-series-ui.enabled',
76 value=u'on', priority=1))
77
78 # XXX Michael Nelson 2010-09-21 bug=631884
79 # Currently LaunchpadTestRequest doesn't set per-thread
80 # features.
81 def in_scope(value):
82 return True
83 per_thread.features = FeatureController(in_scope)
84
85 def reset_per_thread_features():
86 per_thread.features = None
87 test_case.addCleanup(reset_per_thread_features)
88
89
64class DistroSeriesLocalPackageDiffsTestCase(TestCaseWithFactory):90class DistroSeriesLocalPackageDiffsTestCase(TestCaseWithFactory):
65 """Test the distroseries +localpackagediffs view."""91 """Test the distroseries +localpackagediffs view."""
6692
67 layer = LaunchpadZopelessLayer93 layer = LaunchpadZopelessLayer
6894
69 def makeDerivedSeries(self, derived_name=None, parent_name=None):
70 # Helper that creates a derived distro series.
71 parent = self.factory.makeDistroSeries(name=parent_name)
72 derived_series = self.factory.makeDistroSeries(
73 name=derived_name, parent_series=parent)
74 return derived_series
75
76 def setDerivedSeriesUIFeatureFlag(self):
77 # Helper to set the feature flag enabling the derived series ui.
78 ignore = getFeatureStore().add(FeatureFlag(
79 scope=u'default', flag=u'soyuz.derived-series-ui.enabled',
80 value=u'on', priority=1))
81
82 # XXX Michael Nelson 2010-09-21 bug=631884
83 # Currently LaunchpadTestRequest doesn't set per-thread
84 # features.
85 def in_scope(value):
86 return True
87 per_thread.features = FeatureController(in_scope)
88
89 def reset_per_thread_features():
90 per_thread.features = None
91 self.addCleanup(reset_per_thread_features)
92
93 def test_view_redirects_without_feature_flag(self):95 def test_view_redirects_without_feature_flag(self):
94 # If the feature flag soyuz.derived-series-ui.enabled is not set the96 # If the feature flag soyuz.derived-series-ui.enabled is not set the
95 # view simply redirects to the derived series.97 # view simply redirects to the derived series.
96 derived_series = self.makeDerivedSeries(98 derived_series = self.factory.makeDistroSeries(
97 parent_name='lucid', derived_name='derilucid')99 name='derilucid', parent_series=self.factory.makeDistroSeries(
100 name='lucid'))
98101
99 self.assertIs(102 self.assertIs(
100 None, getFeatureFlag('soyuz.derived-series-ui.enabled'))103 None, getFeatureFlag('soyuz.derived-series-ui.enabled'))
@@ -108,8 +111,9 @@
108111
109 def test_label(self):112 def test_label(self):
110 # The view label includes the names of both series.113 # The view label includes the names of both series.
111 derived_series = self.makeDerivedSeries(114 derived_series = self.factory.makeDistroSeries(
112 parent_name='lucid', derived_name='derilucid')115 name='derilucid', parent_series=self.factory.makeDistroSeries(
116 name='lucid'))
113117
114 view = create_initialized_view(118 view = create_initialized_view(
115 derived_series, '+localpackagediffs')119 derived_series, '+localpackagediffs')
@@ -122,8 +126,9 @@
122 def test_batch_includes_needing_attention_only(self):126 def test_batch_includes_needing_attention_only(self):
123 # The differences attribute includes differences needing127 # The differences attribute includes differences needing
124 # attention only.128 # attention only.
125 derived_series = self.makeDerivedSeries(129 derived_series = self.factory.makeDistroSeries(
126 parent_name='lucid', derived_name='derilucid')130 name='derilucid', parent_series=self.factory.makeDistroSeries(
131 name='lucid'))
127 current_difference = self.factory.makeDistroSeriesDifference(132 current_difference = self.factory.makeDistroSeriesDifference(
128 derived_series=derived_series)133 derived_series=derived_series)
129 old_difference = self.factory.makeDistroSeriesDifference(134 old_difference = self.factory.makeDistroSeriesDifference(
@@ -138,8 +143,9 @@
138143
139 def test_batch_includes_different_versions_only(self):144 def test_batch_includes_different_versions_only(self):
140 # The view contains differences of type DIFFERENT_VERSIONS only.145 # The view contains differences of type DIFFERENT_VERSIONS only.
141 derived_series = self.makeDerivedSeries(146 derived_series = self.factory.makeDistroSeries(
142 parent_name='lucid', derived_name='derilucid')147 name='derilucid', parent_series=self.factory.makeDistroSeries(
148 name='lucid'))
143 different_versions_diff = self.factory.makeDistroSeriesDifference(149 different_versions_diff = self.factory.makeDistroSeriesDifference(
144 derived_series=derived_series)150 derived_series=derived_series)
145 unique_diff = self.factory.makeDistroSeriesDifference(151 unique_diff = self.factory.makeDistroSeriesDifference(
@@ -155,10 +161,11 @@
155161
156 def test_template_includes_help_link(self):162 def test_template_includes_help_link(self):
157 # The help link for popup help is included.163 # The help link for popup help is included.
158 derived_series = self.makeDerivedSeries(164 derived_series = self.factory.makeDistroSeries(
159 parent_name='lucid', derived_name='derilucid')165 name='derilucid', parent_series=self.factory.makeDistroSeries(
166 name='lucid'))
160167
161 self.setDerivedSeriesUIFeatureFlag()168 set_derived_series_ui_feature_flag(self)
162 view = create_initialized_view(169 view = create_initialized_view(
163 derived_series, '+localpackagediffs')170 derived_series, '+localpackagediffs')
164171
@@ -169,14 +176,15 @@
169176
170 def test_diff_row_includes_last_comment_only(self):177 def test_diff_row_includes_last_comment_only(self):
171 # The most recent comment is rendered for each difference.178 # The most recent comment is rendered for each difference.
172 derived_series = self.makeDerivedSeries(179 derived_series = self.factory.makeDistroSeries(
173 parent_name='lucid', derived_name='derilucid')180 name='derilucid', parent_series=self.factory.makeDistroSeries(
181 name='lucid'))
174 difference = self.factory.makeDistroSeriesDifference(182 difference = self.factory.makeDistroSeriesDifference(
175 derived_series=derived_series)183 derived_series=derived_series)
176 difference.addComment(difference.owner, "Earlier comment")184 difference.addComment(difference.owner, "Earlier comment")
177 difference.addComment(difference.owner, "Latest comment")185 difference.addComment(difference.owner, "Latest comment")
178186
179 self.setDerivedSeriesUIFeatureFlag()187 set_derived_series_ui_feature_flag(self)
180 view = create_initialized_view(188 view = create_initialized_view(
181 derived_series, '+localpackagediffs')189 derived_series, '+localpackagediffs')
182190
@@ -192,12 +200,13 @@
192200
193 def test_diff_row_links_to_extra_details(self):201 def test_diff_row_links_to_extra_details(self):
194 # The source package name links to the difference details.202 # The source package name links to the difference details.
195 derived_series = self.makeDerivedSeries(203 derived_series = self.factory.makeDistroSeries(
196 parent_name='lucid', derived_name='derilucid')204 name='derilucid', parent_series=self.factory.makeDistroSeries(
205 name='lucid'))
197 difference = self.factory.makeDistroSeriesDifference(206 difference = self.factory.makeDistroSeriesDifference(
198 derived_series=derived_series)207 derived_series=derived_series)
199208
200 self.setDerivedSeriesUIFeatureFlag()209 set_derived_series_ui_feature_flag(self)
201 view = create_initialized_view(210 view = create_initialized_view(
202 derived_series, '+localpackagediffs')211 derived_series, '+localpackagediffs')
203 soup = BeautifulSoup(view())212 soup = BeautifulSoup(view())
@@ -210,6 +219,115 @@
210 self.assertEqual(difference.source_package_name.name, links[0].string)219 self.assertEqual(difference.source_package_name.name, links[0].string)
211220
212221
222class DistroSeriesLocalPackageDiffsFunctionalTestCase(TestCaseWithFactory):
223
224 layer = LaunchpadFunctionalLayer
225
226 def test_canPerformSync_non_editor(self):
227 # Non-editors do not see options to sync.
228 derived_series = self.factory.makeDistroSeries(
229 name='derilucid', parent_series=self.factory.makeDistroSeries(
230 name='lucid'))
231 difference = self.factory.makeDistroSeriesDifference(
232 derived_series=derived_series)
233
234 set_derived_series_ui_feature_flag(self)
235 with person_logged_in(self.factory.makePerson()):
236 view = create_initialized_view(
237 derived_series, '+localpackagediffs')
238
239 self.assertFalse(view.canPerformSync())
240
241 def test_canPerformSync_editor(self):
242 # Editors are presented with options to perform syncs.
243 derived_series = self.factory.makeDistroSeries(
244 name='derilucid', parent_series=self.factory.makeDistroSeries(
245 name='lucid'))
246 difference = self.factory.makeDistroSeriesDifference(
247 derived_series=derived_series)
248
249 set_derived_series_ui_feature_flag(self)
250 with person_logged_in(derived_series.owner):
251 view = create_initialized_view(
252 derived_series, '+localpackagediffs')
253 self.assertTrue(view.canPerformSync())
254
255 def test_sync_notification_on_success(self):
256 # Syncing one or more diffs results in a stub notification.
257 derived_series = self.factory.makeDistroSeries(
258 name='derilucid', parent_series=self.factory.makeDistroSeries(
259 name='lucid'))
260 difference = self.factory.makeDistroSeriesDifference(
261 source_package_name_str='my-src-name',
262 derived_series=derived_series)
263
264 set_derived_series_ui_feature_flag(self)
265 with person_logged_in(derived_series.owner):
266 view = create_initialized_view(
267 derived_series, '+localpackagediffs',
268 method='POST', form={
269 'field.selected_differences': [
270 difference.source_package_name.name,
271 ],
272 'field.actions.sync': 'Sync',
273 })
274
275 self.assertEqual(0, len(view.errors))
276 notifications = view.request.response.notifications
277 self.assertEqual(1, len(notifications))
278 self.assertEqual(
279 "The following sources would have been synced if this wasn't "
280 "just a stub operation: my-src-name",
281 notifications[0].message)
282 self.assertEqual(302, view.request.response.getStatus())
283
284 def test_sync_error_nothing_selected(self):
285 # An error is raised when a sync is requested without any selection.
286 derived_series = self.factory.makeDistroSeries(
287 name='derilucid', parent_series=self.factory.makeDistroSeries(
288 name='lucid'))
289 difference = self.factory.makeDistroSeriesDifference(
290 source_package_name_str='my-src-name',
291 derived_series=derived_series)
292
293 set_derived_series_ui_feature_flag(self)
294 with person_logged_in(derived_series.owner):
295 view = create_initialized_view(
296 derived_series, '+localpackagediffs',
297 method='POST', form={
298 'field.selected_differences': [],
299 'field.actions.sync': 'Sync',
300 })
301
302 self.assertEqual(1, len(view.errors))
303 self.assertEqual(
304 'No differences selected.', view.errors[0])
305
306 def test_sync_error_invalid_selection(self):
307 # An error is raised when an invalid difference is selected.
308 derived_series = self.factory.makeDistroSeries(
309 name='derilucid', parent_series=self.factory.makeDistroSeries(
310 name='lucid'))
311 difference = self.factory.makeDistroSeriesDifference(
312 source_package_name_str='my-src-name',
313 derived_series=derived_series)
314
315 set_derived_series_ui_feature_flag(self)
316 with person_logged_in(derived_series.owner):
317 view = create_initialized_view(
318 derived_series, '+localpackagediffs',
319 method='POST', form={
320 'field.selected_differences': ['some-other-name'],
321 'field.actions.sync': 'Sync',
322 })
323
324 self.assertEqual(2, len(view.errors))
325 self.assertEqual(
326 'No differences selected.', view.errors[0])
327 self.assertEqual(
328 'Invalid value', view.errors[1].error_name)
329
330
213class TestMilestoneBatchNavigatorAttribute(TestCaseWithFactory):331class TestMilestoneBatchNavigatorAttribute(TestCaseWithFactory):
214 """Test the series.milestone_batch_navigator attribute."""332 """Test the series.milestone_batch_navigator attribute."""
215333
216334
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2010-09-28 14:42:41 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2010-10-05 15:06:16 +0000
@@ -27,7 +27,9 @@
27 more about syncing from the parent series</a>).27 more about syncing from the parent series</a>).
28 </p>28 </p>
2929
30 <div tal:condition="differences/batch">30 <div metal:use-macro="context/@@launchpad_form/form">
31
32 <div tal:condition="differences/batch" metal:fill-slot="widgets">
31 <tal:navigation_top33 <tal:navigation_top
32 replace="structure differences/@@+navigation-links-upper" />34 replace="structure differences/@@+navigation-links-upper" />
33 <table class="listing">35 <table class="listing">
@@ -44,10 +46,18 @@
44 <tal:difference repeat="difference differences/batch">46 <tal:difference repeat="difference differences/batch">
45 <tr tal:define="parent_source_pub difference/parent_source_pub;47 <tr tal:define="parent_source_pub difference/parent_source_pub;
46 source_pub difference/source_pub;48 source_pub difference/source_pub;
49 src_name difference/source_package_name/name;
47 signer source_pub/sourcepackagerelease/uploader/fmt:link|nothing;"50 signer source_pub/sourcepackagerelease/uploader/fmt:link|nothing;"
48 tal:attributes="class parent_source_pub/source_package_name">51 tal:attributes="class parent_source_pub/source_package_name">
49 <td><a tal:attributes="href difference/fmt:url" class="toggle-extra"52 <td>
50 tal:content="parent_source_pub/source_package_name">Foo</a>53 <input tal:condition="view/canPerformSync"
54 name="field.selected_differences" type="checkbox"
55 tal:attributes="
56 value src_name;
57 id string:field.selected_differences.${src_name}"/>
58
59 <a tal:attributes="href difference/fmt:url" class="toggle-extra"
60 tal:content="parent_source_pub/source_package_name">Foo</a>
51 </td>61 </td>
52 <td><a tal:attributes="href parent_source_pub/sourcepackagerelease/fmt:url">62 <td><a tal:attributes="href parent_source_pub/sourcepackagerelease/fmt:url">
53 <tal:replace63 <tal:replace
@@ -80,7 +90,17 @@
80 </tal:difference>90 </tal:difference>
81 </tbody>91 </tbody>
82 </table>92 </table>
93 <tal:selectable_differences_end
94 define="widget nocall:view/widgets/selected_differences;
95 field_name widget/context/__name__;
96 error python:view.getFieldError(field_name);">
97 <input tal:attributes="name string:${widget/name}-empty-marker"
98 type="hidden" value="1" />
99 <div class="error message" tal:condition="error"
100 tal:content="structure error">Error message</div>
101 </tal:selectable_differences_end>
83 </div>102 </div>
103 </div>
84<script type="text/javascript">104<script type="text/javascript">
85LPS.use('lp.registry.distroseriesdifferences_details', function(Y) {105LPS.use('lp.registry.distroseriesdifferences_details', function(Y) {
86 diff_module = Y.lp.registry.distroseriesdifferences_details106 diff_module = Y.lp.registry.distroseriesdifferences_details

Subscribers

People subscribed via source and target branches

to status/vote changes: