launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

Данило Шеган has proposed merging lp:~danilo/launchpad/remove-primary-duplicate-reason into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~danilo/launchpad/remove-primary-duplicate-reason/+merge/57839

= Remove symmetric handling of duplicates =

Atm, the code assumes that bug duplication is a symmetric process as far as email notifications go.  Since that's not the case (I double-checked that), we need to stop treating it as such.

== Implementation details ==

Remove the bug join condition that fetches the primary bug for a duplicate.  The condition list was Or()d together.

The test has been updated to confirm this is the case.

Lint fixes in a separate commit, getting pushed after review :)

== Tests ==

bin/test -cvvt PersonSubscriptionInfo.*duplicate

== Demo and Q/A ==

  1. Turn on 'malone.advanced-structural-subscriptions.enabled' feature flag
  2. Mark a bug https://bugs.launchpad.dev/firefox/+bug/6 as duplicate of https://bugs.launchpad.dev/firefox/+bug/5
  3. Subscribe to bug 5
  4. Make sure the https://bugs.launchpad.dev/firefox/+bug/6/+subscriptions doesn't say how bug 5 is a duplicate of bug 6

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/model/personsubscriptioninfo.py
  lib/lp/bugs/model/tests/test_personsubscriptioninfo.py

./lib/lp/bugs/model/personsubscriptioninfo.py
     119: E501 line too long (80 characters)
     257: E301 expected 1 blank line, found 0
     267: E301 expected 1 blank line, found 0
     274: E301 expected 1 blank line, found 0
     305: E202 whitespace before ')'
     313: E202 whitespace before ')'
     119: Line exceeds 78 characters.
./lib/lp/bugs/model/tests/test_personsubscriptioninfo.py
      12: 'searchbuilder' imported but unused
      29: 'anonymous_logged_in' imported but unused
       8: 'Store' imported but unused
      16: 'BugTaskStatus' imported but unused
      10: 'ProxyFactory' imported but unused
       9: 'Unauthorized' imported but unused
      16: 'BugTaskImportance' imported but unused
      29: 'login_person' imported but unused
      26: 'BugSubscriptionFilter' imported but unused
      13: 'IStore' imported but unused
      37: E302 expected 2 blank lines, found 1
-- 
https://code.launchpad.net/~danilo/launchpad/remove-primary-duplicate-reason/+merge/57839
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/remove-primary-duplicate-reason into lp:launchpad.

=== modified file 'lib/lp/bugs/model/personsubscriptioninfo.py'
--- lib/lp/bugs/model/personsubscriptioninfo.py	2011-04-13 10:01:27 +0000
+++ lib/lp/bugs/model/personsubscriptioninfo.py	2011-04-15 10:20:58 +0000
@@ -180,8 +180,6 @@
         # membership) in a single query.
         store = Store.of(person)
         bug_id_options = [Bug.id == bug.id, Bug.duplicateofID == bug.id]
-        if bug.duplicateof is not None:
-            bug_id_options.append(Bug.id == bug.duplicateof.id)
         info = store.find(
             (BugSubscription, Bug, Person),
             BugSubscription.bug == Bug.id,

=== modified file 'lib/lp/bugs/model/tests/test_personsubscriptioninfo.py'
--- lib/lp/bugs/model/tests/test_personsubscriptioninfo.py	2011-04-13 10:01:27 +0000
+++ lib/lp/bugs/model/tests/test_personsubscriptioninfo.py	2011-04-15 10:20:58 +0000
@@ -312,20 +312,18 @@
 
     def test_duplicate_direct_reverse(self):
         # Subscribed directly to the primary bug, and a duplicate bug changes.
-        duplicate = self.factory.makeBug()
+        primary = self.factory.makeBug()
         with person_logged_in(self.subscriber):
-            self.bug.markAsDuplicate(duplicate)
-            duplicate.subscribe(self.subscriber, self.subscriber)
+            self.bug.markAsDuplicate(primary)
+            primary.subscribe(self.subscriber, self.subscriber)
         # Load a `PersonSubscriptionInfo`s for subscriber and a bug.
         self.subscriptions.reload()
 
-        self.assertCollectionsAreEmpty(except_='from_duplicate')
+        # This means no subscriptions on the duplicate bug.
+        self.assertCollectionsAreEmpty()
         self.failIf(self.subscriptions.muted)
         self.assertCollectionContents(
-            self.subscriptions.from_duplicate, personal=1)
-        self.assertRealSubscriptionInfoMatches(
-            self.subscriptions.from_duplicate.personal[0],
-            duplicate, self.subscriber, False, [], [])
+            self.subscriptions.from_duplicate, personal=0)
 
     def test_duplicate_multiple(self):
         # Subscribed directly to more than one duplicate bug.