Merge lp:~henninge/launchpad/bug-422466-status-picker into lp:launchpad/db-devel
- bug-422466-status-picker
- Merge into db-devel
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Henning Eggers | ||||
Approved revision: | not available | ||||
Merged at revision: | not available | ||||
Proposed branch: | lp:~henninge/launchpad/bug-422466-status-picker | ||||
Merge into: | lp:launchpad/db-devel | ||||
Diff against target: |
1718 lines (+830/-222) 26 files modified
lib/canonical/launchpad/icing/style-3-0.css (+15/-0) lib/canonical/launchpad/javascript/translations/translations.js (+67/-9) lib/canonical/launchpad/security.py (+4/-36) lib/lp/archiveuploader/nascentupload.py (+10/-5) lib/lp/archiveuploader/permission.py (+87/-1) lib/lp/archiveuploader/tests/test_permission.py (+31/-7) lib/lp/archiveuploader/uploadpolicy.py (+0/-23) lib/lp/registry/browser/person.py (+4/-2) lib/lp/registry/browser/tests/poll-views.txt (+134/-0) lib/lp/registry/configure.zcml (+4/-6) lib/lp/registry/doc/distribution-mirror.txt (+21/-2) lib/lp/registry/interfaces/distribution.py (+10/-0) lib/lp/registry/interfaces/sourcepackage.py (+14/-0) lib/lp/registry/model/sourcepackage.py (+18/-1) lib/lp/registry/templates/people-requestmerge.pt (+3/-4) lib/lp/registry/templates/team-polls.pt (+2/-2) lib/lp/registry/templates/team-portlet-polls.pt (+4/-4) lib/lp/registry/tests/test_sourcepackage.py (+57/-0) lib/lp/soyuz/interfaces/component.py (+2/-2) lib/lp/testing/factory.py (+131/-13) lib/lp/translations/browser/hastranslationimports.py (+43/-42) lib/lp/translations/model/translationimportqueue.py (+0/-3) lib/lp/translations/stories/importqueue/xx-translation-import-queue.txt (+15/-13) lib/lp/translations/templates/translation-import-queue-macros.pt (+58/-37) lib/lp/translations/tests/test_translationimportqueue.py (+3/-9) lib/lp/translations/windmill/tests/test_import_queue.py (+93/-1) |
||||
To merge this branch: | bzr merge lp:~henninge/launchpad/bug-422466-status-picker | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Abel Deuring (community) | code | Approve | |
Curtis Hovey (community) | ui | Approve | |
Michael Nelson (community) | ui | Approve | |
Review via email: mp+15035@code.launchpad.net |
Commit message
Added a nice status picker to the import queue page. And colours!
Description of the change
Henning Eggers (henninge) wrote : | # |
Curtis Hovey (sinzui) wrote : | # |
Hi Henning.
I really the addition of the picker, and your considerations of how to deal with unavailable status. I have a few concerns that this addition has exacerbated. The edit/info Translation Queue Entry links looks like they is separated from the content. With the addition of inline editing, the layout looks like a mistake.
I think the icon to edit the translation queue entry must be closer to the parts it modifies. Consider creating the link twice and placing it after the "pot" and "template" parts because that tells me I can change those aspects of the entry.
While pondering a suggestion to fix this, I discovered that the info button is attached the expander, and it expands content that it is not pointing to. This is wrong. Every OS I have seen that uses an expander places it to the left of the content so that the arrows point to where your eyes look (like a checkbox). I toggled several times to learn what was being expanded. The expander should be to the left of the entry. I believe this is a separate bug that should not prevent you from landing this. If a bug is not reported, please do.
Michael Nelson (michael.nelson) wrote : | # |
> Hi Henning.
Great work Henning! This will be an excellent precedent for other call-sites that need dynamically disabled options like this.
Sorry if it's confusing below, I started writing before you pushed your changes. I'm setting this to approve, but only with Curtis' feedback/approval also - there are two points below that I'm keen to hear his thoughts on.
>
> I really the addition of the picker, and your considerations of how to deal
> with unavailable status. I have a few concerns that this addition has
> exacerbated. The edit/info Translation Queue Entry links looks like they is
> separated from the content. With the addition of inline editing, the layout
> looks like a mistake.
I agree with Curtis here (and you - as you mentioned you foresaw this too).
>
> I think the icon to edit the translation queue entry must be closer to the
> parts it modifies. Consider creating the link twice and placing it after the
> "pot" and "template" parts because that tells me I can change those aspects of
> the entry.
That's a good solution that will mean the UI doesn't need to change at all when we eventually in-line those items too. I do think it's worth having the link in both places as suggested, rather than just after the template (as in your recent revs to fix this), although, you then mentioned that it is really only the translation domain that is edited. I'll leave that up to you - you know the usage of the page and can evaluate which option is better in the mean-time.
If you do decide to only have it in the one place, I wonder whether it would be best just to the right of "po/evolution-
As an aside, is there a reason why we're not using columns with headers for the main info in this table? Like:
| Uploaded file | Package/Branch (?) | Uploaded by | Status |
or similar? (I'd still leave the template section where it is, especially if it's optional, as the form implies, otherwise the columns would be too wide and too much data).
>
> While pondering a suggestion to fix this, I discovered that the info button is
> attached the expander, and it expands content that it is not pointing to. This
> is wrong. Every OS I have seen that uses an expander places it to the left of
> the content so that the arrows point to where your eyes look (like a
> checkbox). I toggled several times to learn what was being expanded. The
> expander should be to the left of the entry. I believe this is a separate bug
> that should not prevent you from landing this. If a bug is not reported,
> please do.
+ 1. I actually had to check on edge to see what the info button did. When I clicked locally I saw:
{{{
new_button.attach is not a function
[Break on this error] new_button.
}}}
(OK, you mentioned on IRC that you get this too).
If you do create a bug for the expander, I think it would be much more obvious if you had the following under the "Will be imported into..." (currently when expanded it is ab...
Curtis Hovey (sinzui) wrote : | # |
Thanks for moving the edit icon into the content. I favour the icon by the pot and the template because that clearly indicates that they can be changed. As Michael noted, this change will prepare the users for when this information is editable inline. And as Michael noted, using a single edit icon implies I can only change a subset of the entry, not the whole entry that I am seeing. This suggestion is consistent with the idiom to use the edit icon after all inline content so that user can see that the value can be modified.
Henning Eggers (henninge) wrote : | # |
== Tests ==
bin/test --layer=
bin/test -vvt translationimpo
== Launchpad Lint ==
Sorry, make lint does not support bzr-pipline yet so I cannot easily produce this. :(
Abel Deuring (adeuring) wrote : | # |
nice work!
Preview Diff
1 | === modified file 'lib/canonical/launchpad/icing/style-3-0.css' |
2 | --- lib/canonical/launchpad/icing/style-3-0.css 2009-11-19 05:18:56 +0000 |
3 | +++ lib/canonical/launchpad/icing/style-3-0.css 2009-11-21 09:15:26 +0000 |
4 | @@ -794,6 +794,21 @@ |
5 | background: inherit; |
6 | } |
7 | |
8 | +/* ==== Translations styles ==== */ |
9 | + |
10 | +.translationimportstatusAPPROVED, |
11 | +.translationimportstatusAPPROVED a {color: #050;} |
12 | +.translationimportstatusIMPORTED, |
13 | +.translationimportstatusIMPORTED a {color: green;} |
14 | +.translationimportstatusDELETED, |
15 | +.translationimportstatusDELETED a {color: black;} |
16 | +.translationimportstatusFAILED, |
17 | +.translationimportstatusFAILED a {color: red;} |
18 | +.translationimportstatusNEEDS_REVIEW, |
19 | +.translationimportstatusNEEDS_REVIEW a {color: #f60;} |
20 | +.translationimportstatusBLOCKED, |
21 | +.translationimportstatusBLOCKED a {color: blue;} |
22 | + |
23 | /* ==== Translations hand-made forms ==== */ |
24 | |
25 | form.translations div.fields { |
26 | |
27 | === modified file 'lib/canonical/launchpad/javascript/translations/translations.js' |
28 | --- lib/canonical/launchpad/javascript/translations/translations.js 2009-11-04 19:59:16 +0000 |
29 | +++ lib/canonical/launchpad/javascript/translations/translations.js 2009-11-21 09:15:26 +0000 |
30 | @@ -2,7 +2,7 @@ |
31 | * GNU Affero General Public License version 3 (see the file LICENSE). |
32 | * |
33 | * @module TranslationImportQueue |
34 | - * @requires oop, event, node |
35 | + * @requires oop, event, node, widget, plugin, overlay,lazr.choiceedit |
36 | */ |
37 | |
38 | YUI.add('translations', function(Y) { |
39 | @@ -51,7 +51,7 @@ |
40 | '</div>'; |
41 | new_button = button_field.create(text); |
42 | button_field.replaceChild(new_button, button); |
43 | - new_button.attach('click', (shown ? hide_output : show_output)); |
44 | + new_button.on('click', (shown ? hide_output : show_output)); |
45 | return button_field.get('parentNode'); |
46 | }; |
47 | |
48 | @@ -108,20 +108,77 @@ |
49 | lp.get(entry_uri, output_loader(div)); |
50 | }; |
51 | |
52 | +/** |
53 | + * Create a choice widget for a given status picker. The function's signature |
54 | + * is meant for it to be called from Node.each for each node that contains |
55 | + * a settable status. |
56 | + * The base configuration for the widget is taken from the choice_confs array |
57 | + * which is defined in a code fragment that is included in the page via TAL. |
58 | + */ |
59 | +var init_status_choice = function(content_box, index, list) { |
60 | + content_box.setStyle('display', '') |
61 | + var conf = choice_confs[index]; |
62 | + conf.title = 'Change status to'; |
63 | + conf.contentBox = content_box; |
64 | + var status_choice = new Y.ChoiceSource(conf); |
65 | + status_choice.showError = function(err) { |
66 | + display_error(null, err); |
67 | + }; |
68 | + var entry_id = content_box.ancestor(function(node){ |
69 | + return node.hasClass('import_entry_row'); |
70 | + }).get('id'); |
71 | + status_choice.on('save', function(e) { |
72 | + var value_box = content_box.query('.value'); |
73 | + var new_status = status_choice.get('value'); |
74 | + value_box.setContent(new_status); |
75 | + config = { |
76 | + on: { |
77 | + success: function(entry) { |
78 | + Y.Array.each(conf.items, function(item) { |
79 | + if (item.value == new_status) { |
80 | + value_box.addClass(item.css_class); |
81 | + } else { |
82 | + value_box.removeClass(item.css_class); |
83 | + } |
84 | + }); |
85 | + } |
86 | + }, |
87 | + parameters: { |
88 | + new_status: new_status |
89 | + } |
90 | + }; |
91 | + Y.log(config); |
92 | + lp_client = new LP.client.Launchpad(); |
93 | + var entry_uri = '+imports/' + entry_id; |
94 | + lp_client.named_post(entry_uri, 'setStatus', config); |
95 | + }); |
96 | + status_choice.render(); |
97 | +}; |
98 | |
99 | /** |
100 | - * Set up the import queue page. Replace placeholders for error-output buttons |
101 | + * Replace placeholders for error-output buttons |
102 | * with actual buttons, and make them functional. |
103 | */ |
104 | +var init_error_output_buttons = function() { |
105 | + var button_markers = Y.all('.show-output'); |
106 | + button_markers.set('innerHTML', compose_button(false)); |
107 | + button_markers.on('click', show_output); |
108 | +}; |
109 | + |
110 | +/** |
111 | + * Set up the import queue page. |
112 | + */ |
113 | translations.initialize_import_queue_page = function (Y) { |
114 | // Show spinner while patching up the page, pre-fetchig the image. |
115 | var spinner_loader = Y.all('#spinner-loader'); |
116 | spinner_loader.set('innerHTML', '<img src="/@@/spinner" alt="" />'); |
117 | - var button_markers = Y.all('.show-output'); |
118 | - if (button_markers) { |
119 | - button_markers.set('innerHTML', compose_button(false)); |
120 | - button_markers.attach('click', show_output); |
121 | - } |
122 | + |
123 | + // Set up error output buttons. |
124 | + init_error_output_buttons(); |
125 | + |
126 | + // Set up status pickers. |
127 | + Y.all('.status-choice').each(init_status_choice); |
128 | + |
129 | // Remove the spinner again. |
130 | spinner_loader.set('innerHTML', ''); |
131 | }; |
132 | @@ -129,4 +186,5 @@ |
133 | }, '0.1', { |
134 | // "oop" and "event" are required to fix known bugs in YUI, which |
135 | // are apparently fixed in a later version. |
136 | - requires: ['oop', 'event', 'node']}); |
137 | + requires: ['oop', 'event', 'node', 'widget', 'plugin', 'overlay', |
138 | + 'lazr.choiceedit', 'lp.client.plugins', 'lp.errors']}); |
139 | |
140 | === modified file 'lib/canonical/launchpad/security.py' |
141 | --- lib/canonical/launchpad/security.py 2009-11-17 09:51:40 +0000 |
142 | +++ lib/canonical/launchpad/security.py 2009-11-21 09:15:26 +0000 |
143 | @@ -10,8 +10,8 @@ |
144 | from zope.component import getUtility |
145 | |
146 | from canonical.launchpad.interfaces.account import IAccount |
147 | +from lp.archiveuploader.permission import can_upload_to_archive |
148 | from canonical.launchpad.interfaces.emailaddress import IEmailAddress |
149 | -from lp.archiveuploader.permission import verify_upload |
150 | from lp.registry.interfaces.announcement import IAnnouncement |
151 | from lp.soyuz.interfaces.archive import IArchive |
152 | from lp.soyuz.interfaces.archivepermission import ( |
153 | @@ -1578,17 +1578,6 @@ |
154 | |
155 | def can_upload_linked_package(person, branch): |
156 | """True if person may upload the package linked to `branch`.""" |
157 | - |
158 | - def get_current_release(ssp): |
159 | - """Get current release for the source package linked to branch. |
160 | - |
161 | - This function uses the `ISuiteSourcePackage` instance supplied. |
162 | - """ |
163 | - package = ssp.sourcepackage |
164 | - releases = ssp.distroseries.getCurrentSourceReleases( |
165 | - [package.sourcepackagename]) |
166 | - return releases.get(package, None) |
167 | - |
168 | # No associated `ISuiteSourcePackage` data -> not an official branch. |
169 | # Abort. |
170 | ssp_list = branch.associatedSuiteSourcePackages() |
171 | @@ -1601,31 +1590,10 @@ |
172 | # around this by assuming that things are fine as long as we find at least |
173 | # one combination that allows us to upload the corresponding source |
174 | # package. |
175 | - |
176 | - # Go through the associated `ISuiteSourcePackage` instances and see |
177 | - # whether we can upload to any of the distro series/pocket combinations. |
178 | - ssp = None |
179 | for ssp in ssp_list: |
180 | - # Can we upload to the respective pocket? |
181 | - if ssp.distroseries.canUploadToPocket(ssp.pocket): |
182 | - break |
183 | - else: |
184 | - # Loop terminated normally i.e. we could not upload to any of the |
185 | - # (distroseries, pocket) combinations found. |
186 | - return False |
187 | - |
188 | - archive = ssp.distroseries.distribution.main_archive |
189 | - # Find the component the linked source package was published in. |
190 | - current_release = get_current_release(ssp) |
191 | - component = getattr(current_release, 'component', None) |
192 | - |
193 | - # Is person authorised to upload the source package this branch |
194 | - # is targeting? |
195 | - result = verify_upload( |
196 | - person, ssp.sourcepackagename, archive, component, ssp.distroseries) |
197 | - # verify_upload() indicates that person *is* allowed to upload by |
198 | - # returning None. |
199 | - return result is None |
200 | + if can_upload_to_archive(person, ssp): |
201 | + return True |
202 | + return False |
203 | |
204 | |
205 | class AdminBranch(AuthorizationBase): |
206 | |
207 | === modified file 'lib/lp/archiveuploader/nascentupload.py' |
208 | --- lib/lp/archiveuploader/nascentupload.py 2009-11-16 21:53:47 +0000 |
209 | +++ lib/lp/archiveuploader/nascentupload.py 2009-11-21 09:15:26 +0000 |
210 | @@ -28,8 +28,8 @@ |
211 | from lp.archiveuploader.nascentuploadfile import ( |
212 | UploadError, UploadWarning, CustomUploadFile, SourceUploadFile, |
213 | BaseBinaryUploadFile) |
214 | +from lp.archiveuploader.permission import check_upload_to_archive |
215 | from lp.archiveuploader.utils import determine_source_file_type |
216 | -from lp.archiveuploader.permission import verify_upload |
217 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
218 | from lp.registry.interfaces.sourcepackage import SourcePackageFileType |
219 | from lp.soyuz.interfaces.archive import ArchivePurpose, MAIN_ARCHIVE_PURPOSES |
220 | @@ -488,9 +488,10 @@ |
221 | source_name = getUtility( |
222 | ISourcePackageNameSet).queryByName(self.changes.dsc.package) |
223 | |
224 | - rejection_reason = verify_upload( |
225 | - signer, source_name, archive, self.changes.dsc.component, |
226 | - self.policy.distroseries, not self.is_new) |
227 | + rejection_reason = check_upload_to_archive( |
228 | + signer, self.policy.distroseries, source_name, archive, |
229 | + self.changes.dsc.component, self.policy.pocket, not self.is_new) |
230 | + |
231 | if rejection_reason is not None: |
232 | self.reject(str(rejection_reason)) |
233 | |
234 | @@ -996,6 +997,11 @@ |
235 | if self.is_ppa: |
236 | return |
237 | |
238 | + # XXX: JonathanLange 2009-09-16: It would be nice to use |
239 | + # ISourcePackage.get_default_archive here, since it has the same |
240 | + # logic. However, I'm not sure whether we can get a SourcePackage |
241 | + # object. |
242 | + |
243 | # See if there is an archive to override with. |
244 | distribution = self.policy.distroseries.distribution |
245 | archive = distribution.getArchiveByComponent( |
246 | @@ -1011,4 +1017,3 @@ |
247 | else: |
248 | # Reset the archive in the policy to the partner archive. |
249 | self.policy.archive = archive |
250 | - |
251 | |
252 | === modified file 'lib/lp/archiveuploader/permission.py' |
253 | --- lib/lp/archiveuploader/permission.py 2009-11-09 17:50:23 +0000 |
254 | +++ lib/lp/archiveuploader/permission.py 2009-11-21 09:15:26 +0000 |
255 | @@ -7,14 +7,17 @@ |
256 | __all__ = [ |
257 | 'CannotUploadToArchive', |
258 | 'CannotUploadToPPA', |
259 | + 'can_upload_to_archive', |
260 | + 'check_upload_to_archive', |
261 | 'components_valid_for', |
262 | 'verify_upload', |
263 | ] |
264 | |
265 | from zope.component import getUtility |
266 | |
267 | -from lp.registry.interfaces.distribution import IDistributionSet |
268 | +from lp.registry.interfaces.pocket import PackagePublishingPocket |
269 | from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet |
270 | +from lp.soyuz.interfaces.archive import ArchivePurpose |
271 | |
272 | |
273 | class CannotUploadToArchive: |
274 | @@ -30,6 +33,15 @@ |
275 | return self._message |
276 | |
277 | |
278 | +class CannotUploadToPocket: |
279 | + """Returned when a pocket is closed for uploads.""" |
280 | + |
281 | + def __init__(self, distroseries, pocket): |
282 | + super(CannotUploadToPocket, self).__init__( |
283 | + "Not permitted to upload to the %s pocket in a series in the " |
284 | + "'%s' state." % (pocket.name, distroseries.status.name)) |
285 | + |
286 | + |
287 | class CannotUploadToPPA(CannotUploadToArchive): |
288 | """Raised when a person cannot upload to a PPA.""" |
289 | |
290 | @@ -63,6 +75,18 @@ |
291 | super(NoRightsForComponent, self).__init__(component=component.name) |
292 | |
293 | |
294 | +class InvalidPocketForPPA(CannotUploadToArchive): |
295 | + """PPAs only support some pockets.""" |
296 | + |
297 | + _fmt = "PPA uploads must be for the RELEASE pocket." |
298 | + |
299 | + |
300 | +class InvalidPocketForPartnerArchive(CannotUploadToArchive): |
301 | + """Partner archives only support some pockets.""" |
302 | + |
303 | + _fmt = "Partner uploads must be for the RELEASE or PROPOSED pocket." |
304 | + |
305 | + |
306 | def components_valid_for(archive, person): |
307 | """Return the components that 'person' can upload to 'archive'. |
308 | |
309 | @@ -75,6 +99,68 @@ |
310 | return set(permission.component for permission in permissions) |
311 | |
312 | |
313 | +def can_upload_to_archive(person, suitesourcepackage, archive=None): |
314 | + """Check if 'person' upload 'suitesourcepackage' to 'archive'. |
315 | + |
316 | + :param person: An `IPerson` who might be uploading. |
317 | + :param suitesourcepackage: An `ISuiteSourcePackage` to be uploaded. |
318 | + :param archive: The `IArchive` to upload to. If not provided, defaults |
319 | + to the default archive for the source package. (See |
320 | + `ISourcePackage.get_default_archive`). |
321 | + :return: True if they can, False if they cannot. |
322 | + """ |
323 | + sourcepackage = suitesourcepackage.sourcepackage |
324 | + if archive is None: |
325 | + archive = sourcepackage.get_default_archive() |
326 | + pocket = suitesourcepackage.pocket |
327 | + distroseries = sourcepackage.distroseries |
328 | + sourcepackagename = sourcepackage.sourcepackagename |
329 | + component = sourcepackage.latest_published_component |
330 | + # strict_component is True because the source package already exists |
331 | + # (otherwise we couldn't have a suitesourcepackage object) and |
332 | + # nascentupload passes True as a matter of policy when the package exists. |
333 | + reason = check_upload_to_archive( |
334 | + person, distroseries, sourcepackagename, archive, component, pocket, |
335 | + strict_component=True) |
336 | + return reason is None |
337 | + |
338 | + |
339 | +def check_upload_to_archive(person, distroseries, sourcepackagename, archive, |
340 | + component, pocket, strict_component=True): |
341 | + """Check if 'person' upload 'suitesourcepackage' to 'archive'. |
342 | + |
343 | + :param person: An `IPerson` who might be uploading. |
344 | + :param distroseries: The `IDistroSeries` being uploaded to. |
345 | + :param sourcepackagename: The `ISourcePackageName` being uploaded. |
346 | + :param archive: The `IArchive` to upload to. If not provided, defaults |
347 | + to the default archive for the source package. (See |
348 | + `ISourcePackage.get_default_archive`). |
349 | + :param component: The `Component` being uploaded to. |
350 | + :param pocket: The `PackagePublishingPocket` of 'distroseries' being |
351 | + uploaded to. |
352 | + :return: The reason for not being able to upload, None otherwise. |
353 | + """ |
354 | + if archive.purpose == ArchivePurpose.PARTNER: |
355 | + if pocket not in ( |
356 | + PackagePublishingPocket.RELEASE, |
357 | + PackagePublishingPocket.PROPOSED): |
358 | + return InvalidPocketForPartnerArchive() |
359 | + elif archive.is_ppa: |
360 | + if pocket != PackagePublishingPocket.RELEASE: |
361 | + return InvalidPocketForPPA() |
362 | + else: |
363 | + # Uploads to the partner archive are allowed in any distroseries |
364 | + # state. |
365 | + # XXX julian 2005-05-29 bug=117557: |
366 | + # This is a greasy hack until bug #117557 is fixed. |
367 | + if not distroseries.canUploadToPocket(pocket): |
368 | + return CannotUploadToPocket(distroseries, pocket) |
369 | + |
370 | + return verify_upload( |
371 | + person, sourcepackagename, archive, component, distroseries, |
372 | + strict_component) |
373 | + |
374 | + |
375 | def packagesets_valid_for(archive, person): |
376 | """Return the package sets that 'person' can upload to 'archive'. |
377 | |
378 | |
379 | === modified file 'lib/lp/archiveuploader/tests/test_permission.py' |
380 | --- lib/lp/archiveuploader/tests/test_permission.py 2009-11-10 09:58:28 +0000 |
381 | +++ lib/lp/archiveuploader/tests/test_permission.py 2009-11-21 09:15:26 +0000 |
382 | @@ -13,7 +13,9 @@ |
383 | from canonical.testing import DatabaseFunctionalLayer |
384 | |
385 | from lp.archiveuploader.permission import ( |
386 | - components_valid_for, verify_upload) |
387 | + check_upload_to_archive, components_valid_for) |
388 | +from lp.registry.interfaces.distroseries import DistroSeriesStatus |
389 | +from lp.registry.interfaces.pocket import PackagePublishingPocket |
390 | from lp.soyuz.interfaces.archive import ArchivePurpose |
391 | from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet |
392 | from lp.testing import TestCaseWithFactory |
393 | @@ -61,10 +63,14 @@ |
394 | # For now, just check that doesn't raise an exception. |
395 | if distroseries is None: |
396 | distroseries = archive.distribution.currentseries |
397 | + if distroseries is None: |
398 | + distroseries = self.factory.makeDistroSeries( |
399 | + distribution=archive.distribution) |
400 | + pocket = PackagePublishingPocket.RELEASE |
401 | self.assertIs( |
402 | None, |
403 | - verify_upload( |
404 | - person, spn, archive, component, distroseries, |
405 | + check_upload_to_archive( |
406 | + person, distroseries, spn, archive, component, pocket, |
407 | strict_component)) |
408 | |
409 | def assertCannotUpload(self, reason, person, spn, archive, component, |
410 | @@ -82,8 +88,11 @@ |
411 | """ |
412 | if distroseries is None: |
413 | distroseries = archive.distribution.currentseries |
414 | - exception = verify_upload( |
415 | - person, spn, archive, component, distroseries) |
416 | + if distroseries is None: |
417 | + distroseries = self.factory.makeDistroSeries() |
418 | + pocket = PackagePublishingPocket.RELEASE |
419 | + exception = check_upload_to_archive( |
420 | + person, distroseries, spn, archive, component, pocket) |
421 | self.assertEqual(reason, str(exception)) |
422 | |
423 | def test_random_person_cannot_upload_to_ppa(self): |
424 | @@ -114,6 +123,18 @@ |
425 | removeSecurityProxy(team).addMember(person, team.teamowner) |
426 | self.assertCanUpload(person, None, ppa, None) |
427 | |
428 | + def test_can_upload_to_ppa_for_old_series(self): |
429 | + # You can upload whatever you want to a PPA, regardless of the upload |
430 | + # policy. |
431 | + person = self.factory.makePerson() |
432 | + ppa = self.factory.makeArchive( |
433 | + purpose=ArchivePurpose.PPA, owner=person) |
434 | + spn = self.factory.makeSourcePackageName() |
435 | + distroseries = self.factory.makeDistroSeries( |
436 | + status=DistroSeriesStatus.CURRENT) |
437 | + self.assertCanUpload( |
438 | + person, spn, ppa, None, distroseries=distroseries) |
439 | + |
440 | def test_arbitrary_person_cannot_upload_to_primary_archive(self): |
441 | # By default, you can't upload to the primary archive. |
442 | person = self.factory.makePerson() |
443 | @@ -144,10 +165,13 @@ |
444 | person = self.factory.makePerson() |
445 | archive = self.factory.makeArchive(purpose=ArchivePurpose.PRIMARY) |
446 | spn = self.factory.makeSourcePackageName() |
447 | - package_set = self.factory.makePackageset(packages=[spn]) |
448 | + distroseries = self.factory.makeDistroSeries() |
449 | + package_set = self.factory.makePackageset( |
450 | + packages=[spn], distroseries=distroseries) |
451 | self.permission_set.newPackagesetUploader( |
452 | archive, person, package_set) |
453 | - self.assertCanUpload(person, spn, archive, None) |
454 | + self.assertCanUpload( |
455 | + person, spn, archive, None, distroseries=distroseries) |
456 | |
457 | def test_packageset_wrong_distroseries(self): |
458 | # A person with rights to upload to the package set in distro |
459 | |
460 | === modified file 'lib/lp/archiveuploader/uploadpolicy.py' |
461 | --- lib/lp/archiveuploader/uploadpolicy.py 2009-08-28 06:39:38 +0000 |
462 | +++ lib/lp/archiveuploader/uploadpolicy.py 2009-11-21 09:15:26 +0000 |
463 | @@ -130,29 +130,6 @@ |
464 | # Buildd binary uploads (resulting from successful builds) |
465 | # to copy archives may go into *any* pocket. |
466 | return |
467 | - if upload.is_ppa: |
468 | - if self.pocket != PackagePublishingPocket.RELEASE: |
469 | - upload.reject( |
470 | - "PPA uploads must be for the RELEASE pocket.") |
471 | - elif (self.archive.purpose == ArchivePurpose.PARTNER and |
472 | - self.pocket != PackagePublishingPocket.RELEASE and |
473 | - self.pocket != PackagePublishingPocket.PROPOSED): |
474 | - # Partner uploads can only go to the release or proposed |
475 | - # pockets. |
476 | - upload.reject( |
477 | - "Partner uploads must be for the RELEASE or PROPOSED pocket.") |
478 | - else: |
479 | - # Uploads to the partner archive are allowed in any distroseries |
480 | - # state. |
481 | - # XXX julian 2005-05-29 bug=117557: |
482 | - # This is a greasy hack until bug #117557 is fixed. |
483 | - if (self.distroseries and |
484 | - self.archive.purpose != ArchivePurpose.PARTNER and |
485 | - not self.distroseries.canUploadToPocket(self.pocket)): |
486 | - upload.reject( |
487 | - "Not permitted to upload to the %s pocket in a " |
488 | - "series in the '%s' state." % ( |
489 | - self.pocket.name, self.distroseries.status.name)) |
490 | |
491 | # reject PPA uploads by default |
492 | self.rejectPPAUploads(upload) |
493 | |
494 | === modified file 'lib/lp/registry/browser/person.py' |
495 | --- lib/lp/registry/browser/person.py 2009-11-17 13:37:49 +0000 |
496 | +++ lib/lp/registry/browser/person.py 2009-11-21 09:15:26 +0000 |
497 | @@ -2857,13 +2857,15 @@ |
498 | else: |
499 | raise AssertionError('Unknown group to contact.') |
500 | |
501 | + @property |
502 | def should_show_polls_portlet(self): |
503 | menu = TeamOverviewMenu(self.context) |
504 | return ( |
505 | - self.hasCurrentPolls() or self.closedpolls |
506 | + self.has_current_polls or self.closedpolls |
507 | or menu.add_poll().enabled) |
508 | |
509 | - def hasCurrentPolls(self): |
510 | + @property |
511 | + def has_current_polls(self): |
512 | """Return True if this team has any non-closed polls.""" |
513 | assert self.context.isTeam() |
514 | return bool(self.openpolls) or bool(self.notyetopenedpolls) |
515 | |
516 | === added file 'lib/lp/registry/browser/tests/poll-views.txt' |
517 | --- lib/lp/registry/browser/tests/poll-views.txt 1970-01-01 00:00:00 +0000 |
518 | +++ lib/lp/registry/browser/tests/poll-views.txt 2009-11-21 09:15:26 +0000 |
519 | @@ -0,0 +1,134 @@ |
520 | +Poll views |
521 | +---------- |
522 | + |
523 | +The polls portlet shows the state of current polls, and links to past |
524 | +polls. |
525 | + |
526 | + >>> from canonical.launchpad.testing.pages import extract_text |
527 | + |
528 | + >>> user = factory.makePerson() |
529 | + >>> team = factory.makeTeam(name='team') |
530 | + >>> owner = team.teamowner |
531 | + |
532 | + >>> def create_team_view(team, name=None, principal=None): |
533 | + ... # XRDS inheritance requires a lot of setup. |
534 | + ... path_info = '/~%s' % team.name |
535 | + ... server_url = 'http://launchpad.dev' |
536 | + ... view = create_view( |
537 | + ... team, name=name, principal=principal, |
538 | + ... server_url=server_url, PATH_INFO=path_info) |
539 | + ... view.initialize() |
540 | + ... return view |
541 | + |
542 | +The portlet does not render any markup when there are no polls... |
543 | + |
544 | + >>> login_person(user) |
545 | + >>> view = create_team_view(team, name='+portlet-polls', principal=user) |
546 | + >>> view.has_current_polls |
547 | + False |
548 | + |
549 | + >>> view.should_show_polls_portlet |
550 | + False |
551 | + |
552 | + >>> print extract_text(view.render()) |
553 | + <BLANKLINE> |
554 | + |
555 | +Unless the user is a team owner. |
556 | + |
557 | + >>> login_person(owner) |
558 | + >>> view = create_team_view(team, name='+portlet-polls', principal=owner) |
559 | + >>> view.has_current_polls |
560 | + False |
561 | + |
562 | + >>> view.should_show_polls_portlet |
563 | + True |
564 | + |
565 | + >>> print extract_text(view.render()) |
566 | + Polls |
567 | + No current polls. |
568 | + Show polls |
569 | + Create a poll |
570 | + |
571 | +The portlet shows a link to polls to all users when there is a poll, but it |
572 | +has not opened. |
573 | + |
574 | + >>> import pytz |
575 | + >>> from datetime import datetime, timedelta |
576 | + >>> from lp.registry.interfaces.poll import IPollSubset, PollSecrecy |
577 | + |
578 | + >>> open_date = datetime.now().replace(tzinfo=pytz.timezone('UTC')) |
579 | + >>> close_date = open_date + timedelta(weeks=1) |
580 | + >>> poll_subset = IPollSubset(team) |
581 | + >>> poll = poll_subset.new( |
582 | + ... 'name', 'title', 'proposition', open_date, close_date, |
583 | + ... PollSecrecy.OPEN, False) |
584 | + |
585 | + >>> login_person(user) |
586 | + >>> view = create_team_view(team, name='+portlet-polls', principal=user) |
587 | + >>> view.has_current_polls |
588 | + True |
589 | + |
590 | + >>> view.should_show_polls_portlet |
591 | + True |
592 | + |
593 | + >>> print extract_text(view.render()) |
594 | + Polls |
595 | + Show polls |
596 | + |
597 | +The portlet shows more details to the poll owner. |
598 | + |
599 | + >>> login_person(owner) |
600 | + >>> view = create_team_view(team, name='+portlet-polls', principal=owner) |
601 | + >>> view.has_current_polls |
602 | + True |
603 | + |
604 | + >>> view.should_show_polls_portlet |
605 | + True |
606 | + |
607 | + >>> print extract_text(view.render()) |
608 | + Polls |
609 | + title - opens in 5 hours |
610 | + Show polls |
611 | + Create a poll |
612 | + |
613 | +Current polls are listed in the portlet, the only difference between a user |
614 | +and an owner is the owner has a link to create more polls. |
615 | + |
616 | + >>> poll.dateopens = open_date - timedelta(weeks=2) |
617 | + |
618 | + >>> login_person(user) |
619 | + >>> view = create_team_view(team, name='+portlet-polls', principal=user) |
620 | + >>> print extract_text(view.render()) |
621 | + Polls |
622 | + title - closes on ... |
623 | + You have seven days left to vote in this poll. |
624 | + Show polls |
625 | + |
626 | + >>> login_person(owner) |
627 | + >>> view = create_team_view(team, name='+portlet-polls', principal=owner) |
628 | + >>> print extract_text(view.render()) |
629 | + Polls |
630 | + title - closes on ... |
631 | + You have seven days left to vote in this poll. |
632 | + Show polls |
633 | + Create a poll |
634 | + |
635 | +When all the polls are closed, the portlet states the case and has a link to |
636 | +see the polls. |
637 | + |
638 | + >>> poll.datecloses = close_date - timedelta(weeks=2) |
639 | + |
640 | + >>> login_person(user) |
641 | + >>> view = create_team_view(team, name='+portlet-polls', principal=user) |
642 | + >>> print extract_text(view.render()) |
643 | + Polls |
644 | + No current polls. |
645 | + Show polls |
646 | + |
647 | + >>> login_person(owner) |
648 | + >>> view = create_team_view(team, name='+portlet-polls', principal=owner) |
649 | + >>> print extract_text(view.render()) |
650 | + Polls |
651 | + No current polls. |
652 | + Show polls |
653 | + Create a poll |
654 | |
655 | === modified file 'lib/lp/registry/configure.zcml' |
656 | --- lib/lp/registry/configure.zcml 2009-11-19 15:23:25 +0000 |
657 | +++ lib/lp/registry/configure.zcml 2009-11-21 09:15:26 +0000 |
658 | @@ -1603,12 +1603,10 @@ |
659 | permission="launchpad.Edit" |
660 | set_attributes="name displayname description http_base_url |
661 | ftp_base_url rsync_base_url enabled speed |
662 | - country content official_candidate" /> |
663 | - <require |
664 | - permission="launchpad.Admin" |
665 | - set_attributes="status owner whiteboard reviewer date_reviewed" /> |
666 | - <require |
667 | - permission="launchpad.Admin" |
668 | + country content official_candidate owner" /> |
669 | + <require |
670 | + permission="launchpad.Admin" |
671 | + set_attributes="status whiteboard reviewer date_reviewed" |
672 | attributes="destroySelf" /> |
673 | </class> |
674 | |
675 | |
676 | === modified file 'lib/lp/registry/doc/distribution-mirror.txt' |
677 | --- lib/lp/registry/doc/distribution-mirror.txt 2009-10-26 18:40:04 +0000 |
678 | +++ lib/lp/registry/doc/distribution-mirror.txt 2009-11-21 09:15:26 +0000 |
679 | @@ -761,7 +761,26 @@ |
680 | >>> print view._sum_throughput(mirrors) |
681 | 37 Gbps |
682 | |
683 | -== Deleting mirrors == |
684 | + |
685 | +Changing mirror owners |
686 | +---------------------- |
687 | + |
688 | +The mirror owner can change the owner to another user. |
689 | + |
690 | + >>> owner = factory.makePerson(name='raccoon') |
691 | + >>> new_owner = factory.makePerson(name='bear') |
692 | + >>> mirror = ubuntu.newMirror( |
693 | + ... owner, speed, brazil, content, http_base_url='http://a.ab/') |
694 | + |
695 | + |
696 | + >>> login_person(mirror.owner) |
697 | + >>> mirror.owner = new_owner |
698 | + >>> print mirror.owner.name |
699 | + bear |
700 | + |
701 | + |
702 | +Deleting mirrors |
703 | +---------------- |
704 | |
705 | Mirrors are sometimes created by spammers hoping that we'll publicize |
706 | whatever links they put there. Although we only publicize mirrors that |
707 | @@ -771,7 +790,7 @@ |
708 | |
709 | Only mirrors which have never been probed can be deleted this way. |
710 | |
711 | - >>> login(cdimage_mirror.owner.preferredemail.email) |
712 | + >>> login_person(cdimage_mirror.owner) |
713 | >>> cdimage_mirror.destroySelf() |
714 | Traceback (most recent call last): |
715 | ... |
716 | |
717 | === modified file 'lib/lp/registry/interfaces/distribution.py' |
718 | --- lib/lp/registry/interfaces/distribution.py 2009-11-13 16:24:01 +0000 |
719 | +++ lib/lp/registry/interfaces/distribution.py 2009-11-21 09:15:26 +0000 |
720 | @@ -16,6 +16,7 @@ |
721 | 'IDistributionMirrorMenuMarker', |
722 | 'IDistributionPublic', |
723 | 'IDistributionSet', |
724 | + 'NoPartnerArchive', |
725 | 'NoSuchDistribution', |
726 | ] |
727 | |
728 | @@ -580,5 +581,14 @@ |
729 | _message_prefix = "No such distribution" |
730 | |
731 | |
732 | +class NoPartnerArchive(Exception): |
733 | + """Raised when a partner archive is needed, but none exists.""" |
734 | + |
735 | + def __init__(self, distribution): |
736 | + Exception.__init__( |
737 | + self, "Partner archive for distro '%s' not found" |
738 | + % (distribution.name,)) |
739 | + |
740 | + |
741 | # Monkey patching to fix circular imports done in |
742 | # _schema_circular_imports.py |
743 | |
744 | === modified file 'lib/lp/registry/interfaces/sourcepackage.py' |
745 | --- lib/lp/registry/interfaces/sourcepackage.py 2009-11-20 14:02:54 +0000 |
746 | +++ lib/lp/registry/interfaces/sourcepackage.py 2009-11-21 09:15:26 +0000 |
747 | @@ -224,6 +224,20 @@ |
748 | title=u'The component in which the package was last published.', |
749 | schema=IComponent, readonly=True, required=False) |
750 | |
751 | + def get_default_archive(component=None): |
752 | + """Get the default archive of this package. |
753 | + |
754 | + If 'component' is a partner component, then the default archive is the |
755 | + partner archive. Otherwise, the primary archive of the associated |
756 | + distribution. |
757 | + |
758 | + :param component: The `IComponent` to base the default archive |
759 | + decision on. If None, defaults to the last published component. |
760 | + :raise NoPartnerArchive: If returning the partner archive is |
761 | + appropriate, but no partner archive exists. |
762 | + :return: `IArchive`. |
763 | + """ |
764 | + |
765 | def getLatestTranslationsUploads(): |
766 | """Find latest Translations tarballs as produced by Soyuz. |
767 | |
768 | |
769 | === modified file 'lib/lp/registry/model/sourcepackage.py' |
770 | --- lib/lp/registry/model/sourcepackage.py 2009-11-20 09:26:43 +0000 |
771 | +++ lib/lp/registry/model/sourcepackage.py 2009-11-21 09:15:26 +0000 |
772 | @@ -23,10 +23,11 @@ |
773 | from canonical.database.sqlbase import flush_database_updates, sqlvalues |
774 | from canonical.lazr.utils import smartquote |
775 | from lp.code.model.branch import Branch |
776 | +from lp.code.model.hasbranches import HasBranchesMixin, HasMergeProposalsMixin |
777 | from lp.bugs.model.bug import get_bug_tags_open_count |
778 | from lp.bugs.model.bugtarget import BugTargetBase |
779 | from lp.bugs.model.bugtask import BugTask |
780 | -from lp.code.model.hasbranches import HasBranchesMixin, HasMergeProposalsMixin |
781 | +from lp.soyuz.interfaces.archive import IArchiveSet, ArchivePurpose |
782 | from lp.soyuz.model.build import Build, BuildSet |
783 | from lp.soyuz.model.distributionsourcepackagerelease import ( |
784 | DistributionSourcePackageRelease) |
785 | @@ -52,6 +53,7 @@ |
786 | from lp.soyuz.interfaces.buildrecords import IHasBuildRecords |
787 | from lp.registry.interfaces.packaging import PackagingType |
788 | from lp.translations.interfaces.potemplate import IHasTranslationTemplates |
789 | +from lp.registry.interfaces.distribution import NoPartnerArchive |
790 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
791 | from lp.soyuz.interfaces.publishing import PackagePublishingStatus |
792 | from lp.soyuz.interfaces.queue import PackageUploadCustomFormat |
793 | @@ -581,6 +583,21 @@ |
794 | else: |
795 | return None |
796 | |
797 | + def get_default_archive(self, component=None): |
798 | + """See `ISourcePackage`.""" |
799 | + if component is None: |
800 | + component = self.latest_published_component |
801 | + distribution = self.distribution |
802 | + if component is not None and component.name == 'partner': |
803 | + archive = getUtility(IArchiveSet).getByDistroPurpose( |
804 | + distribution, ArchivePurpose.PARTNER) |
805 | + if archive is None: |
806 | + raise NoPartnerArchive(distribution) |
807 | + else: |
808 | + return archive |
809 | + else: |
810 | + return distribution.main_archive |
811 | + |
812 | def getTranslationTemplates(self): |
813 | """See `IHasTranslationTemplates`.""" |
814 | result = POTemplate.selectBy( |
815 | |
816 | === modified file 'lib/lp/registry/templates/people-requestmerge.pt' |
817 | --- lib/lp/registry/templates/people-requestmerge.pt 2009-11-15 20:42:16 +0000 |
818 | +++ lib/lp/registry/templates/people-requestmerge.pt 2009-11-21 09:15:26 +0000 |
819 | @@ -29,10 +29,9 @@ |
820 | <p class="discreet"> |
821 | Note: If you no longer have access to the duplicated account's |
822 | email address, you may place a |
823 | - <a href="/launchpad/+addquestion">support request</a> or contact us |
824 | - at <a href="mailto:feedback@launchpad.net" |
825 | - >feedback@launchpad.net</a>. The Launchpad administrators will |
826 | - tend to your request as soon as possible. |
827 | + <a href="/launchpad/+addquestion">support request</a>. |
828 | + The Launchpad administrators will tend to your request as soon |
829 | + as possible. |
830 | </p> |
831 | </tal:extra_info> |
832 | |
833 | |
834 | === modified file 'lib/lp/registry/templates/team-polls.pt' |
835 | --- lib/lp/registry/templates/team-polls.pt 2009-08-21 18:18:04 +0000 |
836 | +++ lib/lp/registry/templates/team-polls.pt 2009-11-21 09:15:26 +0000 |
837 | @@ -16,11 +16,11 @@ |
838 | |
839 | <h2>Current polls</h2> |
840 | |
841 | - <p tal:condition="not: view/hasCurrentPolls"> |
842 | + <p tal:condition="not: view/has_current_polls"> |
843 | This team has no open polls nor polls that are not yet opened. |
844 | </p> |
845 | |
846 | - <ul tal:condition="view/hasCurrentPolls"> |
847 | + <ul tal:condition="view/has_current_polls"> |
848 | <li tal:repeat="poll view/openpolls"> |
849 | <a tal:attributes="href poll/fmt:url"> |
850 | <span tal:replace="poll/title" /> |
851 | |
852 | === modified file 'lib/lp/registry/templates/team-portlet-polls.pt' |
853 | --- lib/lp/registry/templates/team-portlet-polls.pt 2009-09-09 18:50:13 +0000 |
854 | +++ lib/lp/registry/templates/team-portlet-polls.pt 2009-11-21 09:15:26 +0000 |
855 | @@ -8,11 +8,11 @@ |
856 | tal:define="overview_menu context/menu:overview" |
857 | tal:condition="view/should_show_polls_portlet"> |
858 | <h2>Polls</h2> |
859 | - <p tal:condition="not: view/hasCurrentPolls"> |
860 | - No polls created. |
861 | + <p tal:condition="not: view/has_current_polls"> |
862 | + No current polls. |
863 | </p> |
864 | |
865 | - <ul tal:condition="view/hasCurrentPolls"> |
866 | + <ul tal:condition="view/has_current_polls"> |
867 | <li tal:repeat="poll view/openpolls"> |
868 | <a tal:attributes="href poll/fmt:url"> |
869 | <span tal:replace="poll/title" /> |
870 | @@ -48,7 +48,7 @@ |
871 | </li> |
872 | </ul> |
873 | |
874 | - <a tal:condition="view/hasCurrentPolls" |
875 | + <a tal:condition="view/should_show_polls_portlet" |
876 | tal:replace="structure overview_menu/polls/fmt:link" /> |
877 | <a tal:replace="structure overview_menu/add_poll/fmt:link" /> |
878 | |
879 | |
880 | === modified file 'lib/lp/registry/tests/test_sourcepackage.py' |
881 | --- lib/lp/registry/tests/test_sourcepackage.py 2009-08-28 06:39:38 +0000 |
882 | +++ lib/lp/registry/tests/test_sourcepackage.py 2009-11-21 09:15:26 +0000 |
883 | @@ -12,9 +12,13 @@ |
884 | from zope.security.proxy import removeSecurityProxy |
885 | |
886 | from canonical.launchpad.ftests import login_person, logout |
887 | +from lp.registry.interfaces.distribution import NoPartnerArchive |
888 | from lp.registry.interfaces.distroseries import DistroSeriesStatus |
889 | from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities |
890 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
891 | +from lp.soyuz.interfaces.archive import ArchivePurpose |
892 | +from lp.soyuz.interfaces.component import IComponentSet |
893 | +from lp.soyuz.interfaces.publishing import PackagePublishingStatus |
894 | from lp.code.interfaces.seriessourcepackagebranch import ( |
895 | IMakeOfficialBranchLinks) |
896 | from lp.testing import TestCaseWithFactory |
897 | @@ -166,6 +170,59 @@ |
898 | distribution_sourcepackage, |
899 | sourcepackage.distribution_sourcepackage) |
900 | |
901 | + def test_default_archive(self): |
902 | + # The default archive of a source package is the primary archive of |
903 | + # its distribution. |
904 | + sourcepackage = self.factory.makeSourcePackage() |
905 | + distribution = sourcepackage.distribution |
906 | + self.assertEqual( |
907 | + distribution.main_archive, sourcepackage.get_default_archive()) |
908 | + |
909 | + def test_default_archive_partner(self): |
910 | + # If the source package was most recently uploaded to a partner |
911 | + # component, then its default archive is the partner archive for the |
912 | + # distribution. |
913 | + sourcepackage = self.factory.makeSourcePackage() |
914 | + partner = getUtility(IComponentSet)['partner'] |
915 | + self.factory.makeSourcePackagePublishingHistory( |
916 | + sourcepackagename=sourcepackage.sourcepackagename, |
917 | + distroseries=sourcepackage.distroseries, |
918 | + component=partner, |
919 | + status=PackagePublishingStatus.PUBLISHED) |
920 | + distribution = sourcepackage.distribution |
921 | + expected_archive = self.factory.makeArchive( |
922 | + distribution=distribution, |
923 | + purpose=ArchivePurpose.PARTNER) |
924 | + self.assertEqual( |
925 | + expected_archive, sourcepackage.get_default_archive()) |
926 | + |
927 | + def test_default_archive_specified_component(self): |
928 | + # If the component is explicitly specified as partner, then we return |
929 | + # the partner archive. |
930 | + sourcepackage = self.factory.makeSourcePackage() |
931 | + partner = getUtility(IComponentSet)['partner'] |
932 | + distribution = sourcepackage.distribution |
933 | + expected_archive = self.factory.makeArchive( |
934 | + distribution=distribution, |
935 | + purpose=ArchivePurpose.PARTNER) |
936 | + self.assertEqual( |
937 | + expected_archive, |
938 | + sourcepackage.get_default_archive(component=partner)) |
939 | + |
940 | + def test_default_archive_partner_doesnt_exist(self): |
941 | + # If the default archive ought to be the partner archive (because the |
942 | + # last published upload was to a partner component) then |
943 | + # default_archive will raise an exception. |
944 | + sourcepackage = self.factory.makeSourcePackage() |
945 | + partner = getUtility(IComponentSet)['partner'] |
946 | + self.factory.makeSourcePackagePublishingHistory( |
947 | + sourcepackagename=sourcepackage.sourcepackagename, |
948 | + distroseries=sourcepackage.distroseries, |
949 | + component=partner, |
950 | + status=PackagePublishingStatus.PUBLISHED) |
951 | + self.assertRaises( |
952 | + NoPartnerArchive, sourcepackage.get_default_archive) |
953 | + |
954 | |
955 | class TestSourcePackageSecurity(TestCaseWithFactory): |
956 | """Tests for source package branch linking security.""" |
957 | |
958 | === modified file 'lib/lp/soyuz/interfaces/component.py' |
959 | --- lib/lp/soyuz/interfaces/component.py 2009-06-25 04:06:00 +0000 |
960 | +++ lib/lp/soyuz/interfaces/component.py 2009-11-21 09:15:26 +0000 |
961 | @@ -10,7 +10,7 @@ |
962 | __all__ = [ |
963 | 'IComponent', |
964 | 'IComponentSelection', |
965 | - 'IComponentSet' |
966 | + 'IComponentSet', |
967 | ] |
968 | |
969 | from zope.interface import Interface, Attribute |
970 | @@ -18,6 +18,7 @@ |
971 | |
972 | from canonical.launchpad import _ |
973 | |
974 | + |
975 | class IComponent(Interface): |
976 | """Represents the Component table. |
977 | |
978 | @@ -54,4 +55,3 @@ |
979 | |
980 | def new(name): |
981 | """Create a new component.""" |
982 | - |
983 | |
984 | === modified file 'lib/lp/testing/factory.py' |
985 | --- lib/lp/testing/factory.py 2009-11-19 15:23:25 +0000 |
986 | +++ lib/lp/testing/factory.py 2009-11-21 09:15:26 +0000 |
987 | @@ -34,17 +34,25 @@ |
988 | |
989 | from canonical.autodecorate import AutoDecorate |
990 | from canonical.config import config |
991 | +from canonical.database.constants import UTC_NOW |
992 | from lp.codehosting.codeimport.worker import CodeImportSourceDetails |
993 | from canonical.database.sqlbase import flush_database_updates |
994 | from lp.soyuz.adapters.packagelocation import PackageLocation |
995 | +from lp.soyuz.interfaces.publishing import PackagePublishingStatus |
996 | +from lp.soyuz.interfaces.section import ISectionSet |
997 | from canonical.launchpad.database.account import Account |
998 | from canonical.launchpad.database.emailaddress import EmailAddress |
999 | from canonical.launchpad.database.message import Message, MessageChunk |
1000 | from lp.soyuz.model.processor import ProcessorFamilySet |
1001 | +from lp.soyuz.model.publishing import ( |
1002 | + SecureSourcePackagePublishingHistory, |
1003 | + SourcePackagePublishingHistory, |
1004 | + ) |
1005 | from canonical.launchpad.interfaces import IMasterStore |
1006 | from canonical.launchpad.interfaces.account import ( |
1007 | AccountCreationRationale, AccountStatus, IAccountSet) |
1008 | -from lp.soyuz.interfaces.archive import IArchiveSet, ArchivePurpose |
1009 | +from lp.soyuz.interfaces.archive import ( |
1010 | + default_name_by_purpose, IArchiveSet, ArchivePurpose) |
1011 | from lp.blueprints.interfaces.sprint import ISprintSet |
1012 | from lp.bugs.interfaces.bug import CreateBugParams, IBugSet |
1013 | from lp.bugs.interfaces.bugtask import BugTaskStatus |
1014 | @@ -104,7 +112,8 @@ |
1015 | from lp.registry.interfaces.product import IProductSet, License |
1016 | from lp.registry.interfaces.productseries import IProductSeries |
1017 | from lp.registry.interfaces.project import IProjectSet |
1018 | -from lp.registry.interfaces.sourcepackage import ISourcePackage |
1019 | +from lp.registry.interfaces.sourcepackage import ( |
1020 | + ISourcePackage, SourcePackageUrgency) |
1021 | from lp.registry.interfaces.sourcepackagename import ( |
1022 | ISourcePackageNameSet) |
1023 | from lp.registry.interfaces.ssh import ISSHKeySet, SSHKeyType |
1024 | @@ -126,6 +135,7 @@ |
1025 | password that needs to happen on the master store and this is forced. |
1026 | However, if we then read it back the default Store has to be used. |
1027 | """ |
1028 | + |
1029 | def with_default_master_store(*args, **kw): |
1030 | try: |
1031 | store_selector = getUtility(IStoreSelector) |
1032 | @@ -140,9 +150,9 @@ |
1033 | return mergeFunctionMetadata(func, with_default_master_store) |
1034 | |
1035 | |
1036 | -# We use this for default paramters where None has a specific meaning. For |
1037 | -# example, makeBranch(product=None) means "make a junk branch". |
1038 | -# None, because None means "junk branch". |
1039 | +# We use this for default parameters where None has a specific meaning. For |
1040 | +# example, makeBranch(product=None) means "make a junk branch". None, because |
1041 | +# None means "junk branch". |
1042 | _DEFAULT = object() |
1043 | |
1044 | |
1045 | @@ -179,10 +189,9 @@ |
1046 | :return: A hexadecimal string, with 'a'-'f' in lower case. |
1047 | """ |
1048 | hex_number = '%x' % self.getUniqueInteger() |
1049 | - if digits is None: |
1050 | - return hex_number |
1051 | - else: |
1052 | - return hex_number.zfill(digits) |
1053 | + if digits is not None: |
1054 | + hex_number = hex_number.zfill(digits) |
1055 | + return hex_number |
1056 | |
1057 | def getUniqueString(self, prefix=None): |
1058 | """Return a string unique to this factory instance. |
1059 | @@ -266,9 +275,10 @@ |
1060 | |
1061 | def makeGPGKey(self, owner): |
1062 | """Give 'owner' a crappy GPG key for the purposes of testing.""" |
1063 | + key_id = self.getUniqueHexString(digits=8).upper() |
1064 | return getUtility(IGPGKeySet).new( |
1065 | owner.id, |
1066 | - keyid=self.getUniqueHexString(digits=8).upper(), |
1067 | + keyid=key_id, |
1068 | fingerprint='A' * 40, |
1069 | keysize=self.getUniqueInteger(), |
1070 | algorithm=GPGKeyAlgorithm.R, |
1071 | @@ -1470,7 +1480,7 @@ |
1072 | return getUtility(IComponentSet).ensure(name) |
1073 | |
1074 | def makeArchive(self, distribution=None, owner=None, name=None, |
1075 | - purpose = None): |
1076 | + purpose=None): |
1077 | """Create and return a new arbitrary archive. |
1078 | |
1079 | :param distribution: Supply IDistribution, defaults to a new one |
1080 | @@ -1484,10 +1494,13 @@ |
1081 | distribution = self.makeDistribution() |
1082 | if owner is None: |
1083 | owner = self.makePerson() |
1084 | - if name is None: |
1085 | - name = self.getUniqueString() |
1086 | if purpose is None: |
1087 | purpose = ArchivePurpose.PPA |
1088 | + if name is None: |
1089 | + try: |
1090 | + name = default_name_by_purpose[purpose] |
1091 | + except KeyError: |
1092 | + name = self.getUniqueString() |
1093 | |
1094 | # Making a distribution makes an archive, and there can be only one |
1095 | # per distribution. |
1096 | @@ -1750,6 +1763,111 @@ |
1097 | distroseries = self.makeDistroRelease() |
1098 | return distroseries.getSourcePackage(sourcepackagename) |
1099 | |
1100 | + def getAnySourcePackageUrgency(self): |
1101 | + return SourcePackageUrgency.MEDIUM |
1102 | + |
1103 | + def makeSourcePackagePublishingHistory(self, sourcepackagename=None, |
1104 | + distroseries=None, maintainer=None, |
1105 | + creator=None, component=None, |
1106 | + section=None, urgency=None, |
1107 | + version=None, archive=None, |
1108 | + builddepends=None, |
1109 | + builddependsindep=None, |
1110 | + build_conflicts=None, |
1111 | + build_conflicts_indep=None, |
1112 | + architecturehintlist='all', |
1113 | + dateremoved=None, |
1114 | + date_uploaded=UTC_NOW, |
1115 | + pocket=None, |
1116 | + status=None, |
1117 | + scheduleddeletiondate=None, |
1118 | + dsc_standards_version='3.6.2', |
1119 | + dsc_format='1.0', |
1120 | + dsc_binaries='foo-bin', |
1121 | + ): |
1122 | + if sourcepackagename is None: |
1123 | + sourcepackagename = self.makeSourcePackageName() |
1124 | + spn = sourcepackagename |
1125 | + |
1126 | + if distroseries is None: |
1127 | + distroseries = self.makeDistroRelease() |
1128 | + |
1129 | + if archive is None: |
1130 | + archive = self.makeArchive( |
1131 | + distribution=distroseries.distribution, |
1132 | + purpose=ArchivePurpose.PRIMARY) |
1133 | + |
1134 | + if component is None: |
1135 | + component = self.makeComponent() |
1136 | + |
1137 | + if pocket is None: |
1138 | + pocket = self.getAnyPocket() |
1139 | + |
1140 | + if status is None: |
1141 | + status = PackagePublishingStatus.PENDING |
1142 | + |
1143 | + if urgency is None: |
1144 | + urgency = self.getAnySourcePackageUrgency() |
1145 | + |
1146 | + if section is None: |
1147 | + section = self.getUniqueString('section') |
1148 | + section = getUtility(ISectionSet).ensure(section) |
1149 | + |
1150 | + if maintainer is None: |
1151 | + maintainer = self.makePerson() |
1152 | + |
1153 | + maintainer_email = '%s <%s>' % ( |
1154 | + maintainer.displayname, |
1155 | + maintainer.preferredemail.email) |
1156 | + |
1157 | + if creator is None: |
1158 | + creator = self.makePerson() |
1159 | + |
1160 | + if version is None: |
1161 | + version = self.getUniqueString('version') |
1162 | + |
1163 | + gpg_key = self.makeGPGKey(creator) |
1164 | + |
1165 | + spr = distroseries.createUploadedSourcePackageRelease( |
1166 | + sourcepackagename=spn, |
1167 | + maintainer=maintainer, |
1168 | + creator=creator, |
1169 | + component=component, |
1170 | + section=section, |
1171 | + urgency=urgency, |
1172 | + version=version, |
1173 | + builddepends=builddepends, |
1174 | + builddependsindep=builddependsindep, |
1175 | + build_conflicts=build_conflicts, |
1176 | + build_conflicts_indep=build_conflicts_indep, |
1177 | + architecturehintlist=architecturehintlist, |
1178 | + changelog_entry=None, |
1179 | + dsc=None, |
1180 | + copyright=self.getUniqueString(), |
1181 | + dscsigningkey=gpg_key, |
1182 | + dsc_maintainer_rfc822=maintainer_email, |
1183 | + dsc_standards_version=dsc_standards_version, |
1184 | + dsc_format=dsc_format, |
1185 | + dsc_binaries=dsc_binaries, |
1186 | + archive=archive, dateuploaded=date_uploaded) |
1187 | + |
1188 | + sspph = SecureSourcePackagePublishingHistory( |
1189 | + distroseries=distroseries, |
1190 | + sourcepackagerelease=spr, |
1191 | + component=spr.component, |
1192 | + section=spr.section, |
1193 | + status=status, |
1194 | + datecreated=date_uploaded, |
1195 | + dateremoved=dateremoved, |
1196 | + scheduleddeletiondate=scheduleddeletiondate, |
1197 | + pocket=pocket, |
1198 | + embargo=False, |
1199 | + archive=archive) |
1200 | + |
1201 | + # SPPH and SSPPH IDs are the same, since they are SPPH is a SQLVIEW |
1202 | + # of SSPPH and other useful attributes. |
1203 | + return SourcePackagePublishingHistory.get(sspph.id) |
1204 | + |
1205 | def makePackageset(self, name=None, description=None, owner=None, |
1206 | packages=(), distroseries=None): |
1207 | """Make an `IPackageset`.""" |
1208 | |
1209 | === modified file 'lib/lp/translations/browser/hastranslationimports.py' |
1210 | --- lib/lp/translations/browser/hastranslationimports.py 2009-11-17 09:50:33 +0000 |
1211 | +++ lib/lp/translations/browser/hastranslationimports.py 2009-11-21 09:15:26 +0000 |
1212 | @@ -11,6 +11,7 @@ |
1213 | |
1214 | import datetime |
1215 | import pytz |
1216 | +import simplejson |
1217 | from zope.app.form.browser import DropdownWidget |
1218 | from zope.component import getUtility |
1219 | from zope.formlib import form |
1220 | @@ -33,6 +34,7 @@ |
1221 | LaunchpadFormView, action, custom_widget, safe_action) |
1222 | from canonical.launchpad.webapp.authorization import check_permission |
1223 | from canonical.launchpad.webapp.batching import TableBatchNavigator |
1224 | +from canonical.widgets.lazrjs import vocabulary_to_choice_edit_items |
1225 | |
1226 | |
1227 | class HasTranslationImportsView(LaunchpadFormView): |
1228 | @@ -115,7 +117,7 @@ |
1229 | return form.Fields( |
1230 | Choice( |
1231 | __name__=name, |
1232 | - source=EntryImportStatusVocabularyFactory(entry), |
1233 | + source=EntryImportStatusVocabularyFactory(entry, self.user), |
1234 | title=_('Select import status')), |
1235 | custom_widget=self.custom_widgets['status'], |
1236 | render_context=self.render_context) |
1237 | @@ -219,28 +221,21 @@ |
1238 | # The status changed. |
1239 | number_of_changes += 1 |
1240 | |
1241 | - # Only the importer, launchpad admins or Rosetta experts have |
1242 | - # special permissions to change status. |
1243 | - if (new_status_name == RosettaImportStatus.DELETED.name and |
1244 | - check_permission('launchpad.Edit', entry)): |
1245 | - entry.setStatus(RosettaImportStatus.DELETED, self.user) |
1246 | - elif (new_status_name == RosettaImportStatus.BLOCKED.name and |
1247 | - check_permission('launchpad.Admin', entry)): |
1248 | - entry.setStatus(RosettaImportStatus.BLOCKED, self.user) |
1249 | - elif (new_status_name == RosettaImportStatus.APPROVED.name and |
1250 | - check_permission('launchpad.Admin', entry) and |
1251 | - entry.import_into is not None): |
1252 | - entry.setStatus(RosettaImportStatus.APPROVED, self.user) |
1253 | - elif (new_status_name == RosettaImportStatus.NEEDS_REVIEW.name and |
1254 | - check_permission('launchpad.Admin', entry)): |
1255 | - entry.setStatus(RosettaImportStatus.NEEDS_REVIEW, self.user) |
1256 | - else: |
1257 | - # The user was not the importer or we are trying to set a |
1258 | - # status that must not be set from this form. That means that |
1259 | - # it's a broken request. |
1260 | + # Determine status enum from from value. |
1261 | + new_status = None |
1262 | + for status in RosettaImportStatus.items: |
1263 | + if new_status_name == status.name: |
1264 | + new_status = status |
1265 | + break |
1266 | + if new_status is None: |
1267 | + # We are trying to set a bogus status. |
1268 | + # That means that it's a broken request. |
1269 | raise UnexpectedFormData( |
1270 | 'Ignored the request to change the status from %s to %s.' |
1271 | % (entry.status.name, new_status_name)) |
1272 | + else: |
1273 | + # This will raise an exception if the user is not authorized. |
1274 | + entry.setStatus(new_status, self.user) |
1275 | |
1276 | # Update the date_status_change field. |
1277 | UTC = pytz.timezone('UTC') |
1278 | @@ -330,44 +325,50 @@ |
1279 | """Return batch object for this page.""" |
1280 | return TableBatchNavigator(self.entries, self.request) |
1281 | |
1282 | + @property |
1283 | + def choice_confs_js(self): |
1284 | + """"Generate configuration for lazr-js widget. |
1285 | + |
1286 | + Only editable items are included in the list. |
1287 | + """ |
1288 | + confs = [] |
1289 | + for entry in self.batchnav.batch: |
1290 | + if check_permission('launchpad.Edit', entry): |
1291 | + confs.append(self.generateChoiceConfForEntry(entry)) |
1292 | + return 'var choice_confs = %s;' % simplejson.dumps(confs) |
1293 | + |
1294 | + def generateChoiceConfForEntry(self, entry): |
1295 | + disabled_items = [ |
1296 | + item.value for item in RosettaImportStatus |
1297 | + if not entry.canSetStatus(item.value, self.user)] |
1298 | + items = vocabulary_to_choice_edit_items( |
1299 | + RosettaImportStatus, disabled_items=disabled_items, |
1300 | + css_class_prefix='translationimportstatus') |
1301 | + return { |
1302 | + 'value': entry.status.title, |
1303 | + 'items': items} |
1304 | |
1305 | class EntryImportStatusVocabularyFactory: |
1306 | """Factory for a vocabulary containing a list of statuses for import.""" |
1307 | |
1308 | implements(IContextSourceBinder) |
1309 | |
1310 | - def __init__(self, entry): |
1311 | + def __init__(self, entry, user): |
1312 | """Create a EntryImportStatusVocabularyFactory. |
1313 | |
1314 | :param entry: The ITranslationImportQueueEntry related with this |
1315 | vocabulary. |
1316 | """ |
1317 | self.entry = entry |
1318 | + self.user = user |
1319 | |
1320 | def __call__(self, context): |
1321 | terms = [] |
1322 | for status in RosettaImportStatus.items: |
1323 | - if (status in (RosettaImportStatus.FAILED, |
1324 | - RosettaImportStatus.IMPORTED) and |
1325 | - self.entry.status != status): |
1326 | - # FAILED and IMPORTED status cannot be set by hand so we |
1327 | - # don't give that choice. |
1328 | - continue |
1329 | - if (status == RosettaImportStatus.APPROVED and |
1330 | - self.entry.status != status and |
1331 | - (self.entry.import_into is None or |
1332 | - not check_permission('launchpad.Admin', self.entry))): |
1333 | - # Only administrators are able to set the APPROVED status, and |
1334 | - # that's only possible if we know where to import it |
1335 | - # (import_into not None). |
1336 | - continue |
1337 | - if (status == RosettaImportStatus.BLOCKED and |
1338 | - self.entry.status != status and |
1339 | - not check_permission('launchpad.Admin', self.entry)): |
1340 | - # Only administrators are able to set the BLOCKED status |
1341 | - continue |
1342 | - |
1343 | - terms.append(SimpleTerm(status.name, status.name, status.title)) |
1344 | + if (status == self.entry.status or |
1345 | + self.entry.canSetStatus(status, self.user)): |
1346 | + terms.append( |
1347 | + SimpleTerm(status.name, status.name, status.title)) |
1348 | return SimpleVocabulary(terms) |
1349 | |
1350 | |
1351 | |
1352 | === modified file 'lib/lp/translations/model/translationimportqueue.py' |
1353 | --- lib/lp/translations/model/translationimportqueue.py 2009-11-19 07:45:09 +0000 |
1354 | +++ lib/lp/translations/model/translationimportqueue.py 2009-11-21 09:15:26 +0000 |
1355 | @@ -296,9 +296,6 @@ |
1356 | |
1357 | def canSetStatus(self, new_status, user): |
1358 | """See `ITranslationImportQueueEntry`.""" |
1359 | - if new_status == self.status: |
1360 | - # Leaving status as it is is always allowed. |
1361 | - return True |
1362 | if user is None: |
1363 | # Anonymous user cannot do anything. |
1364 | return False |
1365 | |
1366 | === modified file 'lib/lp/translations/stories/importqueue/xx-translation-import-queue.txt' |
1367 | --- lib/lp/translations/stories/importqueue/xx-translation-import-queue.txt 2009-09-07 12:16:29 +0000 |
1368 | +++ lib/lp/translations/stories/importqueue/xx-translation-import-queue.txt 2009-11-21 09:15:26 +0000 |
1369 | @@ -50,12 +50,11 @@ |
1370 | ... 'http://translations.launchpad.dev/firefox/1.0/+imports') |
1371 | >>> print anon_browser.url |
1372 | http://translations.launchpad.dev/firefox/1.0/+imports |
1373 | - >>> 'po/es.po' in anon_browser.contents |
1374 | - True |
1375 | - >>> 'Mozilla Firefox 1.0 series' in anon_browser.contents |
1376 | - True |
1377 | - >>> '<td>Needs Review</td>' in anon_browser.contents |
1378 | - True |
1379 | + >>> row = find_tags_by_class(anon_browser.contents, 'import_entry_row')[1] |
1380 | + >>> print extract_text(row.find('', 'import_source')) |
1381 | + po/es.po in Mozilla Firefox 1.0 series |
1382 | + >>> print extract_text(row.find('', 'import_status')) |
1383 | + Needs Review |
1384 | |
1385 | Some tarballs contain files whose names look like PO or POT files, but |
1386 | that are really editor backups whose name begin with a dot. These are |
1387 | @@ -131,9 +130,8 @@ |
1388 | ... 'batch-navigation-index') |
1389 | >>> print extract_text(nav_index) |
1390 | 1 → 5 of 5 results |
1391 | - >>> table = find_tag_by_id(anon_browser.contents, 'import-entries-list') |
1392 | - >>> last_row = table.findAll('tr')[10] |
1393 | - >>> print extract_text(last_row) |
1394 | + >>> rows = find_tags_by_class(anon_browser.contents, 'import_entry_row') |
1395 | + >>> print extract_text(rows[4]) |
1396 | evolution.pot in |
1397 | evolution in Ubuntu Hoary |
1398 | Needs Review |
1399 | @@ -208,7 +206,7 @@ |
1400 | |
1401 | There is an option to remove entries from the queue. |
1402 | |
1403 | -No Privileges Person will try to remove it, but will fail. |
1404 | +No Privileges Person tries to remove entries but to no effect. |
1405 | |
1406 | >>> import urllib |
1407 | >>> post_data = urllib.urlencode( |
1408 | @@ -226,9 +224,13 @@ |
1409 | >>> user_browser.open( |
1410 | ... 'http://translations.launchpad.dev/+imports', |
1411 | ... data=post_data) |
1412 | - Traceback (most recent call last): |
1413 | - ... |
1414 | - UnexpectedFormData... |
1415 | + >>> for status in find_tags_by_class(user_browser.contents, 'import_status'): |
1416 | + ... print extract_text(status) |
1417 | + Approved |
1418 | + Approved |
1419 | + Imported |
1420 | + Imported |
1421 | + Needs Review |
1422 | |
1423 | But Jordi, a Rosetta expert, will be allowed to remove it. |
1424 | |
1425 | |
1426 | === modified file 'lib/lp/translations/templates/translation-import-queue-macros.pt' |
1427 | --- lib/lp/translations/templates/translation-import-queue-macros.pt 2009-11-12 17:09:20 +0000 |
1428 | +++ lib/lp/translations/templates/translation-import-queue-macros.pt 2009-11-21 09:15:26 +0000 |
1429 | @@ -10,11 +10,12 @@ |
1430 | > |
1431 | <body> |
1432 | <metal:translation-import-queue define-macro="translation-import-queue-content"> |
1433 | - <script |
1434 | - type="text/javascript" |
1435 | - tal:condition="devmode" |
1436 | - tal:attributes="src string:${icingroot}/build/translations/translations.js"> |
1437 | - </script> |
1438 | + <script type="text/javascript" tal:content="view/choice_confs_js" /> |
1439 | + <script |
1440 | + type="text/javascript" |
1441 | + tal:condition="devmode" |
1442 | + tal:attributes="src string:${icingroot}/build/translations/translations.js"> |
1443 | + </script> |
1444 | |
1445 | <script type="text/javascript"> |
1446 | YUI().use( 'translations', 'event', function(Y) { |
1447 | @@ -72,7 +73,7 @@ |
1448 | |
1449 | <table id="import-entries-list" class="listing" width="100%"> |
1450 | <tal:loop repeat="entry view/batchnav/currentBatch"> |
1451 | - <tr tal:attributes="id entry/id"> |
1452 | + <tr class="import_entry_row" tal:attributes="id entry/id"> |
1453 | <tal:XXX replace="nothing"> |
1454 | # XXX: CarlosPerelloMarin 2006-03-09 bug=34213 |
1455 | # We are using here |
1456 | @@ -80,13 +81,10 @@ |
1457 | # sometimes, entry/import_into evaluates as false because |
1458 | # __len__ method returns 0. |
1459 | </tal:XXX> |
1460 | - <td tal:condition="entry/import_into/title|nothing" rowspan="3"> |
1461 | - <span class="download sprite"></span> |
1462 | - </td> |
1463 | - <td tal:condition="not: entry/import_into/title|nothing" rowspan="2"> |
1464 | - <span class="download sprite"></span> |
1465 | - </td> |
1466 | - <td> |
1467 | + <td rowspan="3"> |
1468 | + <span class="download sprite"></span> |
1469 | + </td> |
1470 | + <td class="import_source"> |
1471 | <a tal:attributes="href entry/content/http_url" |
1472 | tal:content="entry/path">po/foo.pot</a> in |
1473 | <a tal:condition="entry/sourcepackage" |
1474 | @@ -96,21 +94,29 @@ |
1475 | tal:replace="structure entry/productseries/fmt:link"> |
1476 | Evolution Series: MAIN</a> |
1477 | </td> |
1478 | - <td tal:condition="entry/required:launchpad.Edit"> |
1479 | - <tal:status define="name string:status_${entry/id}" |
1480 | - replace="structure view/widgets/?name" /> |
1481 | - </td> |
1482 | - <td tal:condition="not: entry/required:launchpad.Edit" |
1483 | - tal:content="entry/status/title">Needs Review |
1484 | - </td> |
1485 | - <td> |
1486 | - <tal:block condition="entry/required:launchpad.Admin"> |
1487 | - <a class="sprite edit" |
1488 | - tal:attributes="href entry/fmt:url"> |
1489 | - <span class="invisible-link">Change this entry</span> |
1490 | - </a> |
1491 | - </tal:block> |
1492 | - </td> |
1493 | + <tal:status define="css_status string:translationimportstatus${entry/status/name}" |
1494 | + omit-tag=""> |
1495 | + <td class="import_status" |
1496 | + tal:condition="entry/required:launchpad.Edit"> |
1497 | + <noscript> |
1498 | + <tal:status define="name string:status_${entry/id}" |
1499 | + replace="structure view/widgets/?name" /> |
1500 | + </noscript> |
1501 | + <span class="status-choice" style="display:none"> |
1502 | + <span tal:attributes="class string:value ${css_status}" |
1503 | + tal:content="entry/status/title"> |
1504 | + Needs review</span> |
1505 | + <img class="editicon" src="/@@/edit" /> |
1506 | + </span> |
1507 | + </td> |
1508 | + <td tal:condition="not: entry/required:launchpad.Edit" |
1509 | + class="import_status"> |
1510 | + <span tal:attributes="class css_status" |
1511 | + tal:content="entry/status/title"> |
1512 | + Needs Review |
1513 | + </span> |
1514 | + </td> |
1515 | + </tal:status> |
1516 | <td> |
1517 | <tal:block condition="view/user"> |
1518 | <tal:comment replace="nothing"> |
1519 | @@ -140,12 +146,25 @@ |
1520 | # sometimes, entry/import_into evaluates as false because |
1521 | # __len__ method returns 0. |
1522 | </tal:XXX> |
1523 | - <tr tal:condition="entry/import_into/title|nothing" class="discreet secondary"> |
1524 | + <tr class="discreet secondary"> |
1525 | <td colspan="3"> |
1526 | - Will be imported into |
1527 | - <a tal:content="entry/import_into/title" |
1528 | - tal:attributes="href entry/import_into/fmt:url" |
1529 | - >Foo</a> |
1530 | + <span style="margin-right: 0.5em" |
1531 | + tal:condition="entry/import_into/title|nothing" > |
1532 | + Will be imported into |
1533 | + <a tal:content="entry/import_into/title" |
1534 | + tal:attributes="href entry/import_into/fmt:url" |
1535 | + >Foo</a> |
1536 | + </span> |
1537 | + <span style="margin-right: 0.5em" |
1538 | + tal:condition="not:entry/import_into/title|nothing"> |
1539 | + No import target selected yet. |
1540 | + </span> |
1541 | + <tal:block condition="entry/required:launchpad.Admin"> |
1542 | + <a class="sprite edit" |
1543 | + tal:attributes="href entry/fmt:url"> |
1544 | + <span class="invisible-link">Change this entry</span> |
1545 | + </a> |
1546 | + </tal:block> |
1547 | </td> |
1548 | </tr> |
1549 | </tal:loop> |
1550 | @@ -158,10 +177,12 @@ |
1551 | tal:condition="script" |
1552 | tal:content="structure script" /> |
1553 | <tal:block define="user request/lp:person" condition="user"> |
1554 | - <div class="actions"> |
1555 | - <input |
1556 | - tal:replace="structure view/change_status_action/render" /> |
1557 | - </div> |
1558 | + <noscript> |
1559 | + <div class="actions"> |
1560 | + <input |
1561 | + tal:replace="structure view/change_status_action/render" /> |
1562 | + </div> |
1563 | + </noscript> |
1564 | </tal:block> |
1565 | </form> |
1566 | </tal:block> |
1567 | |
1568 | === modified file 'lib/lp/translations/tests/test_translationimportqueue.py' |
1569 | --- lib/lp/translations/tests/test_translationimportqueue.py 2009-11-17 11:05:58 +0000 |
1570 | +++ lib/lp/translations/tests/test_translationimportqueue.py 2009-11-21 09:15:26 +0000 |
1571 | @@ -55,17 +55,11 @@ |
1572 | for status in possible_statuses]) |
1573 | |
1574 | def test_canSetStatus_non_admin(self): |
1575 | - # A non-privileged users cannot set any status except for retaining |
1576 | - # the current status of an entry. |
1577 | + # A non-privileged users cannot set any status. |
1578 | some_user = self.factory.makePerson() |
1579 | self._assertCanSetStatus(some_user, self.entry, |
1580 | # A B D F I NR |
1581 | - [False, False, False, False, False, True]) |
1582 | - self.entry.setStatus( |
1583 | - RosettaImportStatus.DELETED, self.rosetta_experts) |
1584 | - self._assertCanSetStatus(some_user, self.entry, |
1585 | - # A B D F I NR |
1586 | - [False, False, True, False, False, False]) |
1587 | + [False, False, False, False, False, False]) |
1588 | |
1589 | def test_canSetStatus_rosetta_expert(self): |
1590 | # Rosetta experts are all-powerful, didn't you know that? |
1591 | @@ -118,7 +112,7 @@ |
1592 | self._setUpUbuntu() |
1593 | self._assertCanSetStatus(self.ubuntu_group_owner, self.entry, |
1594 | # A B D F I NR |
1595 | - [False, False, False, False, False, True]) |
1596 | + [False, False, False, False, False, False]) |
1597 | |
1598 | |
1599 | def test_suite(): |
1600 | |
1601 | === modified file 'lib/lp/translations/windmill/tests/test_import_queue.py' |
1602 | --- lib/lp/translations/windmill/tests/test_import_queue.py 2009-10-30 17:09:59 +0000 |
1603 | +++ lib/lp/translations/windmill/tests/test_import_queue.py 2009-11-21 09:15:26 +0000 |
1604 | @@ -6,9 +6,18 @@ |
1605 | __metaclass__ = type |
1606 | __all__ = [] |
1607 | |
1608 | +import transaction |
1609 | + |
1610 | from windmill.authoring import WindmillTestClient |
1611 | +from zope.component import getUtility |
1612 | +from zope.security.proxy import removeSecurityProxy |
1613 | |
1614 | +from canonical.launchpad.windmill.testing.constants import ( |
1615 | + FOR_ELEMENT, PAGE_LOAD, SLEEP) |
1616 | from canonical.launchpad.windmill.testing import lpuser |
1617 | +from canonical.launchpad.windmill.testing.lpuser import login_person |
1618 | +from lp.translations.interfaces.translationimportqueue import ( |
1619 | + ITranslationImportQueue) |
1620 | from lp.translations.windmill.testing import TranslationsWindmillLayer |
1621 | from lp.testing import TestCaseWithFactory |
1622 | |
1623 | @@ -69,7 +78,7 @@ |
1624 | user = lpuser.TRANSLATIONS_ADMIN |
1625 | # Go to import queue page logged in as translations admin. |
1626 | client.open(url=start_url) |
1627 | - client.waits.forPageLoad(timeout=u'20000') |
1628 | + client.waits.forPageLoad(timeout=PAGE_LOAD) |
1629 | user.ensure_login(client) |
1630 | |
1631 | # When the page is first called the file_type is set to POT and |
1632 | @@ -90,3 +99,86 @@ |
1633 | client.select(id=u'field.file_type', val=u'UNSPEC') |
1634 | self._assertAllFieldsHidden(client, 'POT') |
1635 | self._assertAllFieldsHidden(client, 'PO') |
1636 | + |
1637 | + |
1638 | +IMPORT_STATUS = u"//tr[@id='%d']//span[contains(@class,'status-choice')]" |
1639 | +IMPORT_STATUS_1 = IMPORT_STATUS % 1 |
1640 | +OPEN_CHOICELIST = u"//div[contains(@class, 'yui-ichoicelist-content')]" |
1641 | + |
1642 | +class ImportQueueStatusTest(TestCaseWithFactory): |
1643 | + """Test that the entries in the import queue can switch types.""" |
1644 | + |
1645 | + layer = TranslationsWindmillLayer |
1646 | + |
1647 | + def test_import_queue_status_admin(self): |
1648 | + """Tests that the admin can use the status picker.""" |
1649 | + client = WindmillTestClient('Translations import queue status') |
1650 | + queue_url = self.layer.base_url+'/+imports' |
1651 | + user = lpuser.TRANSLATIONS_ADMIN |
1652 | + # Go to import queue page logged in as translations admin. |
1653 | + client.open(url=queue_url) |
1654 | + client.waits.forPageLoad(timeout=PAGE_LOAD) |
1655 | + user.ensure_login(client) |
1656 | + |
1657 | + # Click on the element containing the import status. |
1658 | + client.waits.forElement(xpath=IMPORT_STATUS_1, timeout=FOR_ELEMENT) |
1659 | + client.click(xpath=IMPORT_STATUS_1) |
1660 | + client.waits.forElement(xpath=OPEN_CHOICELIST) |
1661 | + |
1662 | + # Change the status to deleted. |
1663 | + client.click(link=u'Deleted') |
1664 | + client.waits.sleep(milliseconds=SLEEP) |
1665 | + client.asserts.assertText(xpath=IMPORT_STATUS_1, validator=u'Deleted') |
1666 | + |
1667 | + # Reload the page and make sure the change sticks. |
1668 | + client.open(url=queue_url) |
1669 | + client.waits.forPageLoad(timeout=PAGE_LOAD) |
1670 | + client.waits.forElement(xpath=IMPORT_STATUS_1, timeout=FOR_ELEMENT) |
1671 | + client.asserts.assertText(xpath=IMPORT_STATUS_1, validator=u'Deleted') |
1672 | + |
1673 | + def test_import_queue_status_nopriv(self): |
1674 | + """Tests that a none-admin will have less choices.""" |
1675 | + client = WindmillTestClient('Translations import queue status') |
1676 | + queue_url = self.layer.base_url+'/+imports' |
1677 | + hubert = self.factory.makePerson( |
1678 | + name="hubert", displayname="Hubert Hunt", password="test", |
1679 | + email="hubert@example.com") |
1680 | + # Create a project and an import entry with it. |
1681 | + product = self.factory.makeProduct(owner=hubert) |
1682 | + removeSecurityProxy(product).official_rosetta = True |
1683 | + productseries = product.getSeries('trunk') |
1684 | + queue = getUtility(ITranslationImportQueue) |
1685 | + potemplate = self.factory.makePOTemplate(productseries=productseries) |
1686 | + entry = queue.addOrUpdateEntry( |
1687 | + 'template.pot', '# POT content', False, hubert, |
1688 | + productseries=productseries, potemplate=potemplate) |
1689 | + transaction.commit() |
1690 | + import_status = IMPORT_STATUS % entry.id |
1691 | + |
1692 | + # Go to import queue page logged in as a normal user. |
1693 | + client.open(url=queue_url) |
1694 | + client.waits.forPageLoad(timeout=PAGE_LOAD) |
1695 | + login_person(hubert, "test", client) |
1696 | + |
1697 | + # There should be no status picker for entry 1. |
1698 | + client.waits.forElement(xpath=import_status, timeout=FOR_ELEMENT) |
1699 | + client.asserts.assertNotNode(xpath=IMPORT_STATUS_1) |
1700 | + |
1701 | + # Click on the element containing the import status. |
1702 | + client.click(xpath=import_status) |
1703 | + client.waits.forElement(xpath=OPEN_CHOICELIST) |
1704 | + |
1705 | + # There should be a link for Deleted but none for approved. |
1706 | + client.asserts.assertNode(link=u'Deleted') |
1707 | + client.asserts.assertNotNode(link=u'Approved') |
1708 | + |
1709 | + # Change the status to deleted. |
1710 | + client.click(link=u'Deleted') |
1711 | + client.waits.sleep(milliseconds=SLEEP) |
1712 | + client.asserts.assertText(xpath=import_status, validator=u'Deleted') |
1713 | + |
1714 | + # Reload the page and make sure the change sticks. |
1715 | + client.open(url=queue_url) |
1716 | + client.waits.forPageLoad(timeout=PAGE_LOAD) |
1717 | + client.waits.forElement(xpath=import_status, timeout=FOR_ELEMENT) |
1718 | + client.asserts.assertText(xpath=import_status, validator=u'Deleted') |
= Bug 422466 =
This completes my work from the LAZR-JS sprint. The setting of statuses on the translation import queue page used to be one big form with a series of drop-down boxes. The statuses were changed by clicking on a submit button at the bottom of the page.
This branch adds a choice picker for each entry in the queue which looks much better and allow for each status to be set separately.
== Implementation details ==
This solution makes use of the "disabled_items" feature of the choice edit widget instead of limiting the choices. I removed one choice from "canSetStatus" that proved problematic in practice.
== Screenshots ==
The "degraded" shots give an impression of the "before" state, too.
http:// people. canonical. com/~henninge/ screenshots/ status- picker- degraded. png people. canonical. com/~henninge/ screenshots/ status- picker- degraded- no-priv. png people. canonical. com/~henninge/ screenshots/ status- picker- after.png people. canonical. com/~henninge/ screenshots/ status- picker- after-open. png people. canonical. com/~henninge/ screenshots/ status- picker- after-no- priv.png people. canonical. com/~henninge/ screenshots/ status- picker- after-no- priv-open. png
http://
http://
http://
http://
http://
== Tests ==
Windmill test still to come.