Launchpad itself

Merge lp:~sinzui/launchpad/delete-structural-target into lp:launchpad/db-devel

  1. delete-structural-target
  2. Merge into db-devel
Proposed by Curtis Hovey
Status: Merged
Approved by: Eleanor Berger
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~sinzui/launchpad/delete-structural-target
Merge into: lp:launchpad/db-devel
Diff against target: 399 lines
10 files modified
database/schema/security.cfg (+1/-1)
lib/canonical/launchpad/icing/style-3-0.css (+3/-0)
lib/canonical/launchpad/templates/launchpad-login.pt (+5/-5)
lib/lp/registry/browser/__init__.py (+47/-9)
lib/lp/registry/browser/productseries.py (+7/-2)
lib/lp/registry/browser/tests/milestone-views.txt (+6/-0)
lib/lp/registry/browser/tests/productseries-views.txt (+40/-10)
lib/lp/registry/doc/milestone.txt (+10/-0)
lib/lp/registry/model/milestone.py (+4/-1)
lib/lp/registry/templates/productseries-delete.pt (+12/-6)
To merge this branch: bzr merge lp:~sinzui/launchpad/delete-structural-target
Reviewer Review Type Date Requested Status
Eleanor Berger (community) code Approve
Review via email: mp+12877@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :
Download full text (5.5 KiB)

This is my branch to fix the series/milestone delete oopes and the bad
lost branch experience.

    lp:~sinzui/launchpad/delete-structural-target
    Diff size: 399
    Launchpad bug: https://bugs.launchpad.net/bugs/416483
                   https://bugs.launchpad.net/bugs/402725
                   https://bugs.launchpad.net/bugs/400844
    Test command: ./bin/test -vv -t "productseries-views|milestone-views"
                                 -t "doc/milestone"
                                 -t "xx-productseries-delete"
    Pre-implementation: Deryck, beuno
    Target release: 3.1.10

= Fix the series/milestone delete oopes =

Bug 416483 [deletion of series and milestone must remove structural
    subscriptions]
    OOPS-1318EA4 shows that structural subscriptions are not removed before
    the destroySelf() method is called.

Bug 402725 [Delete series and milestones does not untarget series-targeted
    blueprints and bugs]
    As OOPS-1298B1901 shows, The delete action failed because there are still
    blueprints targeted to the series. The blueprints targeted to milestones
    were removed, but not the ones to the series. This situation may also
    happen for bugs (OOPS-1321EB223)

Bug 400844 [Deleting the "trunk" series linked to branch messes up the
    Bazaar repository]
    Deleting a series linked to a branch had the effect of renaming our
    repository from ~commonsense/divisiui/trunk to "lp:obsolete-junk/divisiui-
    trunk-20090625-210501". This is frightening, insulting, and gives the
    impression that the branch is going to be deleted soon. The branch should
    have been unlinked before the deletion/move and the UI should explain
    what it did and link to the branch.

== Rules ==

Bug 416483 [deletion of series and milestone must remove structural
    subscriptions]
    Create RegistryDeleteViewMixin._unlinkSubscription(series_or_milestone)
    that will remove the structural subscriptions for series and milestones.

Bug 402725 [Delete series and milestones does not untarget series-targeted
    blueprints and bugs]
    Exract the loop to untarget spec and bugs from a milestone to also
    handle series.
    RegistryDeleteViewMixin._untarget_bugs_and_specifications(series_or_milestone)
    of if getting the spec and bugs are very different, the 3 line loop can
    be copied and rewritten for this case

Bug 400844 [Deleting the "trunk" series linked to branch messes up the
    Bazaar repository]
    Always set the series.branch to None.

ADDENDUM

The IProductSeriesBugTask must be deleted because there is nothing to
reassign it to. This means that security.py needed a new rule, and that
means that this needs to be merged to db-devel. Delete is not exposed on
IBugtask, nor should it be because destroySelf() happens by 'need' rather
than by 'want'.

== QA ==

On Staging
    * Create a structural subscription on a milestone and series.
    * Target bugs to the series
    * Target blueprints to the series
    * Set the branch to the series.
    * Choose delete.
      * Verify the page explains what these objects will be untargeted/
        unlinked.
      * Choose delete
    * Verify it did not oops
    * Verify the branch...

Read more...

Revision history for this message
Eleanor Berger (intellectronica) wrote :

We discussed changing obj.destroySelf to Store.of(obj).remove(obj) so that we don't add new sqlobject-oriented code.

Also, we agreed that keeping the deletion work in the view (rather than in the model) is a bit unfortunate (since we won't be able to repeat the same interaction when working with the model directly or via the API) but for now that's the pragmatic solution.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/security.cfg'
2--- database/schema/security.cfg 2009-10-02 14:02:25 +0000
3+++ database/schema/security.cfg 2009-10-05 16:45:23 +0000
4@@ -891,7 +891,7 @@
5 public.bugpackageinfestation = SELECT, INSERT, UPDATE
6 public.bugproductinfestation = SELECT, INSERT, UPDATE
7 public.bugsubscription = SELECT, INSERT, UPDATE, DELETE
8-public.bugtask = SELECT, INSERT, UPDATE
9+public.bugtask = SELECT, INSERT, UPDATE, DELETE
10 public.bugtracker = SELECT, INSERT, UPDATE, DELETE
11 public.bugtrackeralias = SELECT, INSERT, UPDATE, DELETE
12 public.bugwatch = SELECT, INSERT, UPDATE, DELETE
13
14=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
15--- lib/canonical/launchpad/icing/style-3-0.css 2009-10-02 13:56:25 +0000
16+++ lib/canonical/launchpad/icing/style-3-0.css 2009-10-05 16:45:23 +0000
17@@ -446,6 +446,9 @@
18 padding: 0 1.5em 0 0;
19 }
20 .subordinate {
21+ margin-left: 2em;
22+ }
23+ol.subordinate {
24 margin-left: 4em;
25 }
26 .two-column-list li,
27
28=== modified file 'lib/canonical/launchpad/templates/launchpad-login.pt'
29--- lib/canonical/launchpad/templates/launchpad-login.pt 2009-07-17 17:59:07 +0000
30+++ lib/canonical/launchpad/templates/launchpad-login.pt 2009-10-05 16:45:22 +0000
31@@ -169,11 +169,11 @@
32 <p>
33 Creating your Launchpad account is easy. Here's what to do:</p>
34
35- <ol class="subordinate">
36- <li>Make sure cookies are enabled in your browser.</li>
37- <li>Enter your e-mail address.</li>
38- <li>Follow the URL in the confirmation e-mail that Launchpad sends and you're done!</li>
39- </ol>
40+ <ol class="subordinate">
41+ <li>Make sure cookies are enabled in your browser.</li>
42+ <li>Enter your e-mail address.</li>
43+ <li>Follow the URL in the confirmation e-mail that Launchpad sends and you're done!</li>
44+ </ol>
45
46
47
48
49=== modified file 'lib/lp/registry/browser/__init__.py'
50--- lib/lp/registry/browser/__init__.py 2009-09-22 16:21:12 +0000
51+++ lib/lp/registry/browser/__init__.py 2009-10-05 16:45:23 +0000
52@@ -18,6 +18,8 @@
53
54 from zope.component import getUtility
55
56+from storm.store import Store
57+
58 from lp.bugs.interfaces.bugtask import BugTaskSearchParams, IBugTaskSet
59 from lp.registry.interfaces.productseries import IProductSeries
60 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
61@@ -136,15 +138,22 @@
62 """The context's URL."""
63 return canonical_url(self.context)
64
65- def _getBugtasks(self, milestone):
66- """Return the list `IBugTask`s targeted to the milestone."""
67- params = BugTaskSearchParams(milestone=milestone, user=None)
68+ def _getBugtasks(self, target):
69+ """Return the list `IBugTask`s associated with the target."""
70+ if IProductSeries.providedBy(target):
71+ params = BugTaskSearchParams(user=None)
72+ params.setProductSeries(target)
73+ else:
74+ params = BugTaskSearchParams(milestone=target, user=None)
75 bugtasks = getUtility(IBugTaskSet).search(params)
76 return list(bugtasks)
77
78- def _getSpecifications(self, milestone):
79- """Return the list `ISpecification`s targeted to the milestone."""
80- return list(milestone.specifications)
81+ def _getSpecifications(self, target):
82+ """Return the list `ISpecification`s associated to the target."""
83+ if IProductSeries.providedBy(target):
84+ return list(target.all_specifications)
85+ else:
86+ return list(target.specifications)
87
88 def _getProductRelease(self, milestone):
89 """The `IProductRelease` associated with the milestone."""
90@@ -158,10 +167,37 @@
91 else:
92 return []
93
94+ def _unsubscribe_structure(self, structure):
95+ """Removed the subscriptions from structure."""
96+ for subscription in structure.getSubscriptions():
97+ # The owner of the subscription or an admin are the only users
98+ # that can destroy a subscription, but this rule cannot prevent
99+ # the owner from removing the structure.
100+ Store.of(subscription).remove(subscription)
101+
102+ def _remove_series_bugs_and_specifications(self, series):
103+ """Untarget the associated bugs and subscriptions."""
104+ for spec in self._getSpecifications(series):
105+ spec.proposeGoal(None, self.user)
106+ for bugtask in self._getBugtasks(series):
107+ # Bugtasks cannot be deleted directly. In this case, the bugtask
108+ # is already reported on the product, so the series bugtask has
109+ # no purpose without a series.
110+ Store.of(bugtask).remove(bugtask)
111+
112 def _deleteProductSeries(self, series):
113- """Remove the series and delete/unlink related objects."""
114- # Delete all milestones, releases, and files.
115- # Any associated bugtasks and specifications are untargeted.
116+ """Remove the series and delete/unlink related objects.
117+
118+ All subordinate milestones, releases, and files will be deleted.
119+ Milestone bugs and blueprints will be untargeted.
120+ Series bugs and blueprints will be untargeted.
121+ Series and milestone structural subscriptions are unsubscribed.
122+ Series branches are unlinked.
123+ """
124+ self._unsubscribe_structure(series)
125+ self._remove_series_bugs_and_specifications(series)
126+ series.branch = None
127+
128 for milestone in series.all_milestones:
129 self._deleteMilestone(milestone)
130 # Series are not deleted because some objects like translations are
131@@ -174,6 +210,7 @@
132
133 def _deleteMilestone(self, milestone):
134 """Delete a milestone and unlink related objects."""
135+ self._unsubscribe_structure(milestone)
136 for bugtask in self._getBugtasks(milestone):
137 bugtask.milestone = None
138 for spec in self._getSpecifications(milestone):
139@@ -191,6 +228,7 @@
140
141 class RegistryEditFormView(LaunchpadEditFormView):
142 """A base class that provides consistent edit form behaviour."""
143+
144 @property
145 def page_title(self):
146 """The page title."""
147
148=== modified file 'lib/lp/registry/browser/productseries.py'
149--- lib/lp/registry/browser/productseries.py 2009-09-23 14:58:12 +0000
150+++ lib/lp/registry/browser/productseries.py 2009-10-05 16:45:23 +0000
151@@ -508,7 +508,7 @@
152 @cachedproperty
153 def bugtasks(self):
154 """A list of all `IBugTask`s targeted to this series."""
155- all_bugtasks = []
156+ all_bugtasks = self._getBugtasks(self.context)
157 for milestone in self.milestones:
158 all_bugtasks.extend(self._getBugtasks(milestone))
159 return all_bugtasks
160@@ -516,7 +516,7 @@
161 @cachedproperty
162 def specifications(self):
163 """A list of all `ISpecification`s targeted to this series."""
164- all_specifications = []
165+ all_specifications = self._getSpecifications(self.context)
166 for milestone in self.milestones:
167 all_specifications.extend(self._getSpecifications(milestone))
168 return all_specifications
169@@ -526,6 +526,11 @@
170 """Does the series have any targeted bugtasks or specifications."""
171 return len(self.bugtasks) > 0 or len(self.specifications) > 0
172
173+ @property
174+ def has_linked_branch(self):
175+ """Is the series linked to a branch."""
176+ return self.context.branch is not None
177+
178 @cachedproperty
179 def product_release_files(self):
180 """A list of all `IProductReleaseFile`s that belong to this series."""
181
182=== modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
183--- lib/lp/registry/browser/tests/milestone-views.txt 2009-09-22 16:21:12 +0000
184+++ lib/lp/registry/browser/tests/milestone-views.txt 2009-10-05 16:45:22 +0000
185@@ -654,6 +654,9 @@
186 >>> bug = factory.makeBug(product=firefox)
187 >>> bugtask = bug.bugtasks[0]
188 >>> bugtask.milestone = milestone
189+ >>> subscription = milestone.addSubscription(owner, owner)
190+ >>> [subscription for subscription in owner.structural_subscriptions]
191+ [<StructuralSubscription ...>]
192
193 >>> view = create_initialized_view(milestone, '+delete')
194 >>> [bugtask.milestone.name for bugtask in view.bugtasks]
195@@ -685,6 +688,9 @@
196 >>> print bugtask.milestone
197 None
198
199+ >>> [subscription for subscription in owner.structural_subscriptions]
200+ []
201+
202 No Privileges Person cannot access this view because he is neither the
203 project owner or series driver..
204
205
206=== modified file 'lib/lp/registry/browser/tests/productseries-views.txt'
207--- lib/lp/registry/browser/tests/productseries-views.txt 2009-09-12 06:11:08 +0000
208+++ lib/lp/registry/browser/tests/productseries-views.txt 2009-10-05 16:45:23 +0000
209@@ -229,6 +229,8 @@
210 []
211 >>> view.product_release_files
212 []
213+ >>> view.has_linked_branch
214+ False
215
216 Most series that are deleted do not have any related objects, but a small
217 portion do.
218@@ -244,18 +246,38 @@
219 >>> bugtask = bug.bugtasks[0]
220 >>> bugtask.milestone = milestone_two
221
222+ >>> owner = product.owner
223+ >>> series_specification = factory.makeSpecification(product=product)
224+ >>> series_specification.proposeGoal(productseries, owner)
225+ >>> series_bugtask = factory.makeBugTask(bug=bug, target=productseries)
226+ >>> subscription = productseries.addSubscription(owner, owner)
227+ >>> productseries.branch = factory.makeBranch()
228+
229 >>> view = create_view(productseries, name='+delete')
230 >>> [milestone.name for milestone in view.milestones]
231 [u'0.2', u'0.1']
232 >>> view.has_bugtasks_and_specifications
233 True
234- >>> [bugtask.milestone.name for bugtask in view.bugtasks]
235- [u'0.2']
236- >>> [spec.milestone.name for spec in view.specifications]
237- [u'0.1']
238-
239- # Listing and deleting product release files is done in the story
240- # because they require the Librarian to be running.
241+ >>> for bugtask in view.bugtasks:
242+ ... if bugtask.milestone is not None:
243+ ... print bugtask.milestone.name
244+ ... else:
245+ ... print bugtask.target.name
246+ rabbit
247+ 0.2
248+ >>> for spec in view.specifications:
249+ ... if spec.milestone is not None:
250+ ... print spec.milestone.name
251+ ... else:
252+ ... print spec.goal.name
253+ rabbit
254+ 0.1
255+
256+ >>> view.has_linked_branch
257+ True
258+
259+ # Listing and deleting product release files is done in
260+ # product-release-views because they require the Librarian to be running.
261
262 Series that are the active focus of development cannot be deleted. The
263 view's can_delete property checks this rule.
264@@ -291,7 +313,8 @@
265 Calling the view's delete action on a series that can be deleted will
266 untarget the bugtasks and specifications that are targeted to the
267 series' milestones. The milestones, releases, and release files are
268-deleted.
269+deleted. Bugs and blueprints targeted to the series are unassigned.
270+Series structural subscriptions are removed. Branch links are removed.
271
272 >>> view = create_initialized_view(productseries, '+delete', form=form)
273 >>> for notification in view.request.response.notifications:
274@@ -308,11 +331,17 @@
275 None
276 >>> print bugtask.milestone
277 None
278+ >>> bugtask.related_tasks
279+ []
280+ >>> print series_specification.milestone
281+ None
282+ >>> [subscription for subscription in owner.structural_subscriptions]
283+ []
284
285 The series was not actually deleted because there are problematic objects
286 like translations. The series are assigned to the Obsolete Junk project.
287 The series name is changed to 'product_name-series_name-date_created' to
288-avoid conflicts.
289+avoid conflicts. The linked branch is removed.
290
291 >>> from zope.component import getUtility
292 >>> from canonical.launchpad.interfaces.launchpad import (
293@@ -323,7 +352,8 @@
294 True
295 >>> print productseries.name
296 field-rabbit-20090501-193424
297-
298+ >>> print productseries.branch
299+ None
300
301 Linking packages
302 ----------------
303
304=== modified file 'lib/lp/registry/doc/milestone.txt'
305--- lib/lp/registry/doc/milestone.txt 2009-08-13 19:03:36 +0000
306+++ lib/lp/registry/doc/milestone.txt 2009-10-05 16:45:22 +0000
307@@ -496,3 +496,13 @@
308 ...
309 AssertionError: You cannot delete a milestone which has specifications
310 targeted to it.
311+
312+If a milestone has a structural subscription, it cannot be deleted.
313+
314+ >>> milestone = ff_onedotzero.newMilestone('1.0.14')
315+ >>> subscription = milestone.addSubscription(owner, owner)
316+ >>> milestone.destroySelf()
317+ Traceback (most recent call last):
318+ ...
319+ AssertionError: You cannot delete a milestone which has structural
320+ subscriptions.
321
322=== modified file 'lib/lp/registry/model/milestone.py'
323--- lib/lp/registry/model/milestone.py 2009-08-24 04:03:27 +0000
324+++ lib/lp/registry/model/milestone.py 2009-10-05 16:45:23 +0000
325@@ -188,6 +188,9 @@
326 """See `IMilestone`."""
327 params = BugTaskSearchParams(milestone=self, user=None)
328 bugtasks = getUtility(IBugTaskSet).search(params)
329+ assert len(self.getSubscriptions()) == 0, (
330+ "You cannot delete a milestone which has structural "
331+ "subscriptions.")
332 assert bugtasks.count() == 0, (
333 "You cannot delete a milestone which has bugtasks targeted "
334 "to it.")
335@@ -238,6 +241,7 @@
336 """See lp.registry.interfaces.milestone.IMilestoneSet."""
337 return Milestone.selectBy(active=True, orderBy='id')
338
339+
340 class ProjectMilestone(HasBugsBase):
341 """A virtual milestone implementation for project.
342
343@@ -301,4 +305,3 @@
344 def official_bug_tags(self):
345 """See `IHasBugs`."""
346 return self.target.official_bug_tags
347-
348
349=== modified file 'lib/lp/registry/templates/productseries-delete.pt'
350--- lib/lp/registry/templates/productseries-delete.pt 2009-08-11 21:31:51 +0000
351+++ lib/lp/registry/templates/productseries-delete.pt 2009-10-05 16:45:23 +0000
352@@ -30,10 +30,12 @@
353 </tal:no-files>
354 </p>
355
356- <ul id="milestones" tal:condition="view/milestones">
357+ <ul id="milestones" class="subordinate"
358+ tal:condition="view/milestones">
359 <li tal:repeat="milestone view/milestones">
360 <strong>
361- <a tal:attributes="href milestone/fmt:url"><tal:name
362+ <a class="sprite milestone"
363+ tal:attributes="href milestone/fmt:url"><tal:name
364 content="milestone/name">0.9</tal:name><tal:codename
365 condition="milestone/code_name">
366 "<tal:name
367@@ -42,9 +44,8 @@
368 </li>
369 </ul>
370
371-
372-
373- <ul id="files" tal:condition="view/product_release_files">
374+ <ul id="files" class="subordinate"
375+ tal:condition="view/product_release_files">
376 <li tal:repeat="file view/product_release_files">
377 <strong tal:content="file/libraryfile/filename">foo.tgz</strong>
378 </li>
379@@ -54,7 +55,7 @@
380 The following bugs and blueprints will be <em>untargeted</em>:
381 </p>
382
383- <ul id="bugtasks-and-blueprints"
384+ <ul id="bugtasks-and-blueprints" class="subordinate"
385 tal:condition="view/has_bugtasks_and_specifications">
386 <li tal:repeat="bugtask view/bugtasks"
387 tal:content="structure bugtask/bug/fmt:link">bug 1
388@@ -64,6 +65,11 @@
389 </li>
390 </ul>
391
392+ <p tal:condition="view/has_linked_branch">
393+ The associated branch will be <em>unlinked</em>:
394+ <a tal:replace="structure view/context/branch/fmt:link" />
395+ </p>
396+
397 <p>
398 Series deletion is permanent.
399 </p>

Subscribers

People subscribed via source and target branches

to status/vote changes: