Prevent people being subscribed twice to the same bug

[Björn Tillenius]

Person merge code should be updated to prevent people being subscribed twice to the same bug. Once that happens, a constraint should be added to prevent this happening again.

There should be a unique constraint on (person, bug) in the BugSubscription table, so that people can't be subscribed twice. We currently have around 190 people subscribe multiple times to the same bug, and that causes a problem when migrating bug subscriptions to the new ACL table. For example, Martin Pool is subscribed twice to bug 96751.

Something like this should delete the duplicates records, while leaving one of them:

  DELETE FROM
    bugsubscription WHERE id NOT IN (
      SELECT MAX(dup.id) FROM
        bugsubscription AS dupGROUP BY dup.person, dup.bug);

(The equivalent for structural subscriptions is bug 663947.)

[Deryck Hodge (deryck)]

We'll get one of the guys working on the better subscriptions/notifications story to knock this out next week when PQM reopens.

Cheers,
deryck

[Brian Murray (brian-murray)]

Does somebody happen to know how these came about so that we can be aware of what situations might cause a problem after the constraint is added?

[Björn Tillenius (bjornt)]

On Wed, Jul 14, 2010 at 07:52:32AM -0000, Brian Murray wrote:
> Does somebody happen to know how these came about so that we can be
> aware of what situations might cause a problem after the constraint is
> added?

No. But by adding a constraint, the next time it happens we'll get an
OOPS report, so that we will know what to fix.

[Brian Murray (brian-murray)]

These are happening at least in part due to the account merging process.

Error in test testMergeTwoUnvalidatedAccounts (canonical.launchpad.scripts.ftests.test_keyringtrustanalyser.TestMergeClusters)
Traceback (most recent call last):
  File "/usr/lib/python2.6/unittest.py", line 279, in run
    testMethod()
  File "/srv/buildbot/slaves/launchpad/lucid-db-devel/build/lib/canonical/launchpad/scripts/ftests/test_keyringtrustanalyser.py", line 257, in testMergeTwoUnvalidatedAccounts
    '<email address hidden>'])])
  File "/srv/buildbot/slaves/launchpad/lucid-db-devel/build/lib/lp/registry/scripts/keyringtrustanalyser.py", line 180, in mergeClusters
    _mergeOrAddEmails(personset, emailset, cluster, logger)
  File "/srv/buildbot/slaves/launchpad/lucid-db-devel/build/lib/lp/registry/scripts/keyringtrustanalyser.py", line 145, in _mergeOrAddEmails
    personset.merge(otherperson, person)
  File "/srv/buildbot/slaves/launchpad/lucid-db-devel/build/lib/lp/registry/model/person.py", line 3568, in merge
    src_tab, src_col, ref_tab, ref_col
NotImplementedError: bugsubscription.person reference to person.id is in a UNIQUE index but has not been handled

[Launchpad QA Bot (lpqabot)]

Fixed in db-stable r9537 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/db-stable/revision/9537>.

[Robert Collins (lifeless)]

We should fix this; it causes ui problems today, and the person merge code really isn't that ugly.

[Curtis Hovey (sinzui)]

This is really bug 87076, but since this is being worked I restructured the duplicates.