Merge lp:~jtv/launchpad/bug-427263 into lp:launchpad/db-devel

[Jeroen T. Vermeulen]

Make the "edit" icons for multi-line editor widgets show up in Konqueror as well.

[Jeroen T. Vermeulen (jtv)]

= Bug 427263 =

In Konqueror, the little "edit" buttons for bug descriptions and other multi-line edit fields do not show up. The reason, as Michael Nelson found, was that this browser does not render <a> tags with no text in them.

This branch works around that by inserting a zero-width non-joiner (as well as the sprite) in the anchor tag for the button. There are no visible changes in other browsers that I can see; as the name says, it's a zero-width character.

Tests:
{{{
./bin/test -t lazr-js-widgets.txt
}}}

Unsurprisingly, there's not a whole lot of test impact. This is purely a visual thing.

Demo and Q/A: Log in. Visit anything with an editable description that would normally use the Ajax inline editor—such a bug or a blueprint that you have edit rights on. (On a blueprint the "edit" icon will appear for the Whiteboard). Visit the same page in Konqueror, while logged into the same account. Without this fix, the edit icon is not visible. With this fix, it is. The inline editor doesn't work in konqueror, so it'll still take you to the pre-ajax edit form. But at least you've got a way to reach it!

Jeroen

[Michael Nelson (michael.nelson)]

Thanks Jeroen. Sorry it took so long to do (hotel internet :/ ).

So I'm approving your changes `bzr diff -r 9878..9880`, which is exactly what I see locally doing `bzr diff -r ancestor:../devel`, not sure why the MP has other changes displayed.

Thanks!

review: Approve (code)

[Jeroen T. Vermeulen (jtv)]

> Thanks Jeroen. Sorry it took so long to do (hotel internet :/ ).
>
> So I'm approving your changes `bzr diff -r 9878..9880`, which is exactly what
> I see locally doing `bzr diff -r ancestor:../devel`, not sure why the MP has
> other changes displayed.

Thanks for the review—feels better to finish the sprint without WIP branches. The diff was just messed up from branching off devel and then proposing for merge in db-devel (which is the default).

Jeroen

[Jeroen T. Vermeulen (jtv)]

Stumbled across a better way to fix this at the last moment. Uses CSS only. Abandoning this MP.

=== modified file 'lib/canonical/launchpad/doc/lazr-js-widgets.txt'
--- lib/canonical/launchpad/doc/lazr-js-widgets.txt	2009-07-14 13:16:50 +0000
+++ lib/canonical/launchpad/doc/lazr-js-widgets.txt	2009-11-13 22:40:28 +0000
@@ -39,7 +39,7 @@
         important</span>
         <a href="http://bugs.launchpad.dev/.../+edit"
            class="yui-editable_text-trigger sprite edit"
-        ></a>
+        >...</a>
     </h1>
     <script>
     ...
 
=== added file 'lib/canonical/launchpad/emailtemplates/person-merged.txt'
--- lib/canonical/launchpad/emailtemplates/person-merged.txt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/emailtemplates/person-merged.txt	2009-11-13 22:40:28 +0000
@@ -0,0 +1,15 @@
+
+The Launchpad account named '%(dupename)s' was merged into the account
+named '%(person)s'. You can confirm the merged email addresses to
+associate them with your account. Merged email addresses are unsubscribed
+from all Launchpad mailing lists during the merge. All team memberships
+for the merged account were also transferred to your account, and
+those team may have mailing lists.
+
+You can review and update your email and subscription settings at:
+
+    https://launchpad.net/~%(person)s/+editemails
+
+Thank you,
+
+The Launchpad team
 
=== modified file 'lib/canonical/widgets/lazrjs.py'
--- lib/canonical/widgets/lazrjs.py	2009-10-28 08:03:36 +0000
+++ lib/canonical/widgets/lazrjs.py	2009-11-13 22:40:28 +0000
@@ -55,7 +55,7 @@
     # Template for the trigger button.
     TRIGGER_TEMPLATE = dedent(u"""\
         <a href="%(edit_url)s" class="yui-editable_text-trigger sprite edit"
-        ></a>
+        >&zwnj;</a>
         """)
 
     # Template for the activation script.
 
=== modified file 'lib/lp/archiveuploader/poppyinterface.py'
--- lib/lp/archiveuploader/poppyinterface.py	2009-06-24 23:33:29 +0000
+++ lib/lp/archiveuploader/poppyinterface.py	2009-11-13 22:40:28 +0000
@@ -9,8 +9,6 @@
 import stat
 import time
 
-from canonical.launchpad.scripts import execute_zcml_for_scripts
-from canonical.lp import initZopeless
 from contrib.glock import GlobalLock
 
 class PoppyInterfaceFailure(Exception):
@@ -22,8 +20,6 @@
 
     def __init__(self, targetpath, logger, allow_user, cmd=None,
                  targetstart=0, perms=None):
-        execute_zcml_for_scripts()
-        self.tm = initZopeless(dbuser='ro')
         self.targetpath = targetpath
         self.logger = logging.getLogger("%s.PoppyInterface" % logger.name)
         self.cmd = cmd
 
=== renamed file 'lib/lp/registry/doc/xx-coc-views.txt' => 'lib/lp/registry/browser/tests/coc-views.txt'
=== renamed file 'lib/lp/registry/doc/person-edit-views.txt' => 'lib/lp/registry/browser/tests/person-edit-views.txt'
=== renamed file 'lib/lp/registry/doc/person-karma-views.txt' => 'lib/lp/registry/browser/tests/person-karma-views.txt'
=== renamed file 'lib/lp/registry/doc/team-hierarchy-views.txt' => 'lib/lp/registry/browser/tests/team-hierarchy-views.txt'
=== renamed file 'lib/lp/registry/doc/teammembership-views.txt' => 'lib/lp/registry/browser/tests/teammembership-views.txt'
=== modified file 'lib/lp/registry/doc/person-merge.txt'
--- lib/lp/registry/doc/person-merge.txt	2009-08-13 19:03:36 +0000
+++ lib/lp/registry/doc/person-merge.txt	2009-11-13 22:40:28 +0000
@@ -199,8 +199,32 @@
     >>> results.get_one()[0] is None
     True
 
-
-== Person decoration ==
+An email is sent to the user informing him that he should review his email
+and mailing list subscription settings.
+
+    >>> from canonical.launchpad.interfaces.personnotification import (
+    ...     IPersonNotificationSet)
+
+    >>> notification_set = getUtility(IPersonNotificationSet)
+    >>> notifications = notification_set.getNotificationsToSend()
+    >>> notifications.count()
+    1
+    >>> notification = notifications[0]
+    >>> print notification.person.name
+    name12
+    >>> print notification.subject
+    Launchpad accounts merged
+    >>> print notification.body
+    The Launchpad account named 'marilize-merged' was merged into the account
+    named 'name12'. ...
+
+    You can review and update your email and subscription settings at:
+
+        https://launchpad.net/name12/+editemails ...
+
+
+Person decoration
+-----------------
 
 Several tables "extend" the Person table by having additional information
 that is UNIQUEly keyed to Person.id. We have a utility function that merges
 
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2009-10-16 09:25:45 +0000
+++ lib/lp/registry/model/person.py	2009-11-13 22:40:28 +0000
@@ -2861,21 +2861,9 @@
                           name=name, new_name=new_name, product=product))
 
     def _mergeMailingListSubscriptions(self, cur, from_id, to_id):
-        # Update MailingListSubscription. Note that no remaining records
-        # will have email_address set, as we assert earlier that the
-        # from_person has no email addresses.
-        # Update records that don't conflict.
-        cur.execute('''
-            UPDATE MailingListSubscription
-            SET person=%(to_id)d
-            WHERE person=%(from_id)d
-                AND mailing_list NOT IN (
-                    SELECT mailing_list
-                    FROM MailingListSubscription
-                    WHERE person=%(to_id)d
-                    )
-            ''' % vars())
-        # Then trash the remainders.
+        # Update MailingListSubscription. Note that since all the from_id
+        # email addresses are set to NEW, all the subscriptions must be
+        # removed because the user must confirm them.
         cur.execute('''
             DELETE FROM MailingListSubscription WHERE person=%(from_id)d
             ''' % vars())
@@ -3472,6 +3460,17 @@
         # flush its caches.
         store.invalidate()
 
+        # Inform the user of the merge changes.
+        if not to_person.isTeam():
+            mail_text = get_email_template('person-merged.txt')
+            mail_text = mail_text % {
+                'dupename': from_person.name,
+                'person': to_person.name,
+                }
+            subject = 'Launchpad accounts merged'
+            getUtility(IPersonNotificationSet).addNotification(
+                to_person, subject, mail_text)
+
     def getValidPersons(self, persons):
         """See `IPersonSet.`"""
         person_ids = [person.id for person in persons]
 
=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py	2009-09-29 19:00:28 +0000
+++ lib/lp/registry/tests/test_personset.py	2009-11-13 22:40:28 +0000
@@ -7,46 +7,56 @@
 
 from unittest import TestLoader
 
+import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
 from lp.registry.model.person import PersonSet
+from lp.registry.interfaces.mailinglistsubscription import (
+    MailingListAutoSubscribePolicy)
 from lp.registry.interfaces.person import (
     PersonCreationRationale, IPersonSet)
-from lp.testing import TestCaseWithFactory
+from lp.testing import TestCaseWithFactory, login_person, logout
+
+from canonical.database.sqlbase import cursor
 from canonical.launchpad.testing.databasehelpers import (
     remove_all_sample_data_branches)
-from canonical.testing import LaunchpadFunctionalLayer
+from canonical.testing import DatabaseFunctionalLayer
 
 
 class TestPersonSetBranchCounts(TestCaseWithFactory):
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
         TestCaseWithFactory.setUp(self)
         remove_all_sample_data_branches()
+        self.person_set = getUtility(IPersonSet)
 
     def test_no_branches(self):
         """Initially there should be no branches."""
-        self.assertEqual(0, PersonSet().getPeopleWithBranches().count())
+        self.assertEqual(0, self.person_set.getPeopleWithBranches().count())
 
     def test_five_branches(self):
         branches = [self.factory.makeAnyBranch() for x in range(5)]
         # Each branch has a different product, so any individual product
         # will return one branch.
-        self.assertEqual(5, PersonSet().getPeopleWithBranches().count())
-        self.assertEqual(1, PersonSet().getPeopleWithBranches(
+        self.assertEqual(5, self.person_set.getPeopleWithBranches().count())
+        self.assertEqual(1, self.person_set.getPeopleWithBranches(
                 branches[0].product).count())
 
 
 class TestPersonSetEnsurePerson(TestCaseWithFactory):
 
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
     email_address = '[e-mail]'
     displayname = 'Testing ensurePerson'
     rationale = PersonCreationRationale.SOURCEPACKAGEUPLOAD
 
+    def setUp(self):
+        TestCaseWithFactory.setUp(self)
+        self.person_set = getUtility(IPersonSet)
+
     def test_ensurePerson_returns_existing_person(self):
         # IPerson.ensurePerson returns existing person and does not
         # override its details.
@@ -54,7 +64,7 @@
         testing_person = self.factory.makePerson(
             email=self.email_address, displayname=testing_displayname)
 
-        ensured_person = getUtility(IPersonSet).ensurePerson(
+        ensured_person = self.person_set.ensurePerson(
             self.email_address, self.displayname, self.rationale)
         self.assertEquals(testing_person.id, ensured_person.id)
         self.assertIsNot(
@@ -67,7 +77,7 @@
     def test_ensurePerson_hides_new_person_email(self):
         # IPersonSet.ensurePerson creates new person with
         # 'hide_email_addresses' set.
-        ensured_person = getUtility(IPersonSet).ensurePerson(
+        ensured_person = self.person_set.ensurePerson(
             self.email_address, self.displayname, self.rationale)
         self.assertTrue(ensured_person.hide_email_addresses)
 
@@ -78,7 +88,7 @@
             self.displayname, email=self.email_address)
         self.assertIs(None, test_account.preferredemail.person)
 
-        ensured_person = getUtility(IPersonSet).ensurePerson(
+        ensured_person = self.person_set.ensurePerson(
             self.email_address, self.displayname, self.rationale)
         self.assertEquals(test_account.id, ensured_person.account.id)
         self.assertTrue(ensured_person.hide_email_addresses)
@@ -98,7 +108,7 @@
         self.assertIs(None, testing_account.preferredemail.person)
         self.assertIs(None, testing_person.preferredemail)
 
-        ensured_person = getUtility(IPersonSet).ensurePerson(
+        ensured_person = self.person_set.ensurePerson(
             self.email_address, self.displayname, self.rationale)
 
         # The existing Person was retrieved and the Account
@@ -108,5 +118,36 @@
                           ensured_person.preferredemail.id)
 
 
+class TestPersonSetMerge(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        TestCaseWithFactory.setUp(self)
+        # Use the unsecured PersonSet so that private methods can be tested.
+        self.person_set = PersonSet()
+        self.from_person = self.factory.makePerson()
+        self.to_person = self.factory.makePerson()
+        self.cur = cursor()
+
+    def test__mergeMailingListSubscriptions_no_subscriptions(self):
+        self.person_set._mergeMailingListSubscriptions(
+            self.cur, self.from_person.id, self.to_person.id)
+        self.assertEqual(0, self.cur.rowcount)
+
+    def test__mergeMailingListSubscriptions_with_subscriptions(self):
+        self.from_person.mailing_list_auto_subscribe_policy = (
+            MailingListAutoSubscribePolicy.ALWAYS)
+        self.team, self.mailing_list = self.factory.makeTeamAndMailingList(
+            'test-mailinglist', 'team-owner')
+        login_person(self.team.teamowner)
+        self.team.addMember(self.from_person, reviewer=self.team.teamowner)
+        logout()
+        transaction.commit()
+        self.person_set._mergeMailingListSubscriptions(
+            self.cur, self.from_person.id, self.to_person.id)
+        self.assertEqual(1, self.cur.rowcount)
+
+
 def test_suite():
     return TestLoader().loadTestsFromName(__name__)
 
=== modified file 'lib/lp/testing/menu.py'
--- lib/lp/testing/menu.py	2009-09-10 20:35:24 +0000
+++ lib/lp/testing/menu.py	2009-11-13 22:40:28 +0000
@@ -13,7 +13,6 @@
 
 def check_menu_links(menu):
     context = menu.context
-    is_sane_menu = True
     for link in menu.iterlinks():
         if '?' in link.target:
             view_name, _args = link.target.split('?')
@@ -24,6 +23,5 @@
         try:
             view = getMultiAdapter((context, request), name=view_name)
         except:
-            is_sane_menu = False
-            print 'Bad link %s: %s' % (link.name, url)
-    print is_sane_menu
+            return 'Bad link %s: %s' % (link.name, url)
+    return True
 
=== modified file 'lib/lp/translations/scripts/message_sharing_migration.py'
--- lib/lp/translations/scripts/message_sharing_migration.py	2009-10-30 11:18:59 +0000
+++ lib/lp/translations/scripts/message_sharing_migration.py	2009-11-13 22:40:28 +0000
@@ -7,7 +7,6 @@
     ]
 
 
-import gc
 import os
 
 from zope.component import getUtility
@@ -291,15 +290,6 @@
         if self.txn is None:
             return
 
-        if self.commit_count % 100 == 0 or not intermediate:
-            freed = gc.collect()
-            objcount = len(gc.get_objects())
-            garbage = len(gc.garbage)
-            memsize = open("/proc/%s/statm" % os.getpid()).read().split()[5]
-            log.debug(
-                "Freed: %d.  Object count: %d.  Garbage: %d.  Memory size: %s"
-                % (freed, objcount, garbage, memsize))
-
         self.commit_count += 1
 
         if intermediate and self.commit_count % 10 != 0: