launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/delete-conjoined-bugtask-1 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #522599 delete milestone conjoined bug
  https://bugs.launchpad.net/bugs/522599


This is my branch to correct deletion of bugtasks during milestone deletion.

    lp:~sinzui/launchpad/delete-conjoined-bugtask-1
    Diff size: 25
    Launchpad bug:
          https://bugs.launchpad.net/bugs/522599
    Test command: ./bin/test -vv -t TestMilestoneDeleteView
    Pre-implementation: EdwinGrubbs
    Target release: 10.09


correct deletion of bugtasks during milestone deletion
--------------------------------------------------------------------

The fix to permit users to delete milestones with conjoined bugs is bad.
The slave was deleted in my QA on edge, but we wanted to delete the master.
ie. https://bugs.edge.launchpad.net/gdp/trunk/+bug/420173 shows a master bug
without a slave. The master is targeted to a series, which make no sense since
there is no bug in the project. This may require SQL to fix.

This bad chunk:
            if bugtask.conjoined_master is not None:
                Store.of(bugtask).remove(bugtask)

Rules
-----

    * Correct the line of code.
            if bugtask.conjoined_master is not None:
                Store.of(bugtask).remove(bugtask.conjoined_master)
    * Revise the test to verify that the productseries has no bugtasks.


QA
--

    * Create a milestone you intend to delete on /gdp/trunk
    * Target https://bugs.edge.launchpad.net/gdp/trunk/+bug/420173
      to the milestone.
    * Delete the milestone.
    * Verify The trunk bugtask is gone and that you can see the status
      of the project bugtask.


Lint
----

Linting changed files:
  lib/lp/registry/browser/__init__.py
  lib/lp/registry/browser/tests/test_milestone.py


Test
----

    * lib/lp/registry/browser/tests/test_milestone.py
      * Added a test to verify the productseries has 0 bugtasks. There was
        1 before I made the code change to make the test pass :(


Implementation
--------------

    * lib/lp/registry/browser/__init__.py
      * Revised the problem code the delete the conjoined_master (the series
        bugtask).
-- 
https://code.launchpad.net/~sinzui/launchpad/delete-conjoined-bugtask-1/+merge/32796
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/delete-conjoined-bugtask-1 into lp:launchpad/devel.

=== modified file 'lib/lp/registry/browser/__init__.py'
--- lib/lp/registry/browser/__init__.py	2010-08-09 22:30:04 +0000
+++ lib/lp/registry/browser/__init__.py	2010-08-16 18:50:59 +0000
@@ -218,7 +218,7 @@
         self._unsubscribe_structure(milestone)
         for bugtask in self._getBugtasks(milestone):
             if bugtask.conjoined_master is not None:
-                Store.of(bugtask).remove(bugtask)
+                Store.of(bugtask).remove(bugtask.conjoined_master)
             else:
                 bugtask.milestone = None
         for spec in self._getSpecifications(milestone):

=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py	2010-08-10 02:09:47 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py	2010-08-16 18:50:59 +0000
@@ -102,6 +102,7 @@
         view = create_initialized_view(milestone, '+delete', form=form)
         self.assertEqual([], view.errors)
         self.assertEqual(0, len(product.all_milestones))
+        self.assertEqual(0, product.development_focus.all_bugtasks.count())
 
 
 def test_suite():