Code review comment for lp:~sinzui/launchpad/delete-structural-target

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

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 still belongs to the project (not obsolete-junk)

== Lint ==

Linting changed files:
  database/schema/security.cfg
  lib/canonical/launchpad/icing/style-3-0.css
  lib/canonical/launchpad/templates/launchpad-login.pt
  lib/lp/registry/browser/__init__.py
  lib/lp/registry/browser/productseries.py
  lib/lp/registry/browser/tests/milestone-views.txt
  lib/lp/registry/browser/tests/productseries-views.txt
  lib/lp/registry/doc/milestone.txt
  lib/lp/registry/model/milestone.py
  lib/lp/registry/templates/productseries-delete.pt

== Test ==

    * lib/lp/registry/browser/tests/milestone-views.txt
      * Updated teh test to verify that structural subscriptions are removed.
    * lib/lp/registry/browser/tests/productseries-views.txt
      * Updated the test to verify that structural subscriptions are removed,
        branches are unlinked, specs are not series goals, and that bugtasks
        are deleted.
    * lib/lp/registry/doc/milestone.txt
      * Added a test to verify that a milestone cannot be deleted if it has
        a subscription.

== Implementation ==

    * database/schema/security.cfg
      * Allowed delete on BugTask.
    * lib/canonical/launchpad/icing/style-3-0.css
      * Fixed the indentation rule for subordinate which was indented too far.
        The rule was tuned to the ol.subordinate case which requires extra
        space because of the position of the numbers
    * lib/canonical/launchpad/templates/launchpad-login.pt
      * Fixed indentation in the templates.
    * lib/lp/registry/browser/__init__.py
      * revised the DeleteMixin bugtask and spec rules to work for series
        too.
      * Added rules to unsubscribe users and remove specs from the series
        goal. The bugtask rule requires delete since the product already
        has a bugtask.
      * Added a rule to unlink the series branch.
    * lib/lp/registry/browser/productseries.py
      * Updated the view to use the mixin's bugtask and spec lists
      * Added a property to know when to tell the user that the series branch
        will be unlinke.
    * lib/lp/registry/model/milestone.py
      * Assert that the milestone has not subscriptions before calling
        destroySelf()
    * lib/lp/registry/templates/productseries-delete.pt
      * Fixed the layout because the lists were not clearly subordinate to
        the previous paragraph. (Worked with Martin to solve this.)

« Back to merge proposal