Merge lp:~sinzui/launchpad/project-set-owner-0 into lp:launchpad

[Curtis Hovey]

This is my branch to remove the cascading reassignment when a project
owner is reassigned and avoid oopses with translation queue entries.

  reviewer jtv

    lp:~sinzui/launchpad/project-set-owner-0
    Diff size: 430
    Launchpad bug:
          https://bugs.launchpad.net/bugs/635438
    Test command: ./bin/test -vv \
          -t product-edit-people-view.txt -t registry.*doc/product.txt
          -t test_translationimportqueue
    Pre-implementation: danilos, jtv
    Target release: 10.10

Remove the cascading reassignment when a project owner reassigned
-----------------------------------------------------------------

OOPS-1714L464 show that IntegrityError: duplicate key value violates unique
constraint "translationimportqueueentry__entry_per_importer__unq" was cause
when the user tried to set the project owner to the same team as the project
driver (elementaryart

The product-edit-people-view.txt test indicates that the
translationimportqueue.importer is reassigned. After investigation we
discovered that reassigning a project owner causes many subordinate
object to also be reassigned. The rules for releases, series, and entries
are pointless, because Launchpad does not use the owner field for permissions.
The project owner or driver is always used. Launchpad does not need to do
this work.

Rules
-----

    * Remove the reassignment rules from Product._setOwner()
    * Add a subscriber that reassigns import entries only when there
      is no conflict.

QA
--

We can verify this bug is fixed on edge by setting /nautilus-elementary owner
to ~elementaryart. I will ask the project owner when we think the fix is
ready.

Lint
----

Linting changed files:
  lib/lp/registry/browser/tests/product-edit-people-view.txt
  lib/lp/registry/doc/product.txt
  lib/lp/registry/model/product.py
  lib/lp/services/database/tests/test_prejoin.py
  lib/lp/translations/configure.zcml
  lib/lp/translations/subscribers.py
  lib/lp/translations/tests/test_translationimportqueue.py

Test
----

    * lib/lp/registry/browser/tests/product-edit-people-view.txt
      * Remove the tests that verify reassignment...the view never did that.
    * lib/lp/registry/doc/product.txt
      * Removed the tests for reassignment because the rules were moved
        out of Product.
    * lib/lp/translations/tests/test_translationimportqueue.py
      * Added a test to verify when an entry's importer is reassigned, and
        when it is not.

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

    * lib/lp/registry/model/product.py
      * Replaced the mutator with a simple object.
    * lib/lp/services/database/tests/test_prejoin.py
      * Updated the test to use the real object attr.
    * lib/lp/translations/configure.zcml
      * Registered a subscriber to handle product changes.
    * lib/lp/translations/subscribers.py
      * Added a subscriber function that updated the entry importer attr
        if the product owner changes and there is no conflict.

[Brad Crittenden (bac)]

Hi Curtis,

Interesting branch. So, all along, the effort of changing series and
releases was for nothing? I think we discussed it a *long* time ago
and decided it was unnecessary but harmless. Glad you simplified
things by getting rid of it.

I've got a couple of questions on the tests, shown in-line below.

> === added file 'lib/lp/translations/subscribers.py'
> --- lib/lp/translations/subscribers.py 1970-01-01 00:00:00 +0000
> +++ lib/lp/translations/subscribers.py 2010-09-24 14:17:10 +0000

> +def product_modified(product, event):
> + """Update translations objects when the product changes."""
> + old_owner = event.object_before_modification.owner
> + if old_owner != product.owner:
> + import_queue = getUtility(ITranslationImportQueue)
> + # The change in ownership may cause permission issues for
> + # entries in the translation import queue *if* the old owner has
> + # less permission now. This issue can be avoided by updating the
> + # entries's importer to the new owner. This can only be done if the
> + # new owner does not already have an entry in the queue.
> + # See bug 635438 for more details.
> + old_owner_entries = []
> + new_owner_entries = []
> + for entry in import_queue.getAllEntries(target=product):
> + if entry.importer == old_owner:
> + old_owner_entries.append(entry)
> + elif entry.importer == product.owner:
> + new_owner_entries.append(entry)
> + else:
> + # There is not permission issue.

Please reword this comment because I'm not sure what you're saying.

> + pass
> + if len(old_owner_entries) > 0 and len(new_owner_entries) == 0:
> + # The new owner does not have any conflicting entries in the
> + # queue. Change the old owner's entries to the new owner to
> + # ensure there is not permissions issues during processing.

s/there is not permissions/there are no permission/ ???

> + for entry in old_owner_entries:
> + removeSecurityProxy(entry).importer = product.owner

> === modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
> --- lib/lp/translations/tests/test_translationimportqueue.py 2010-08-20 20:31:18 +0000
> +++ lib/lp/translations/tests/test_translationimportqueue.py 2010-09-24 14:17:10 +0000
> @@ -1,7 +1,7 @@
> -# Copyright 2009 Canonical Ltd. This software is licensed under the
> +# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
> # GNU Affero General Public License version 3 (see the file LICENSE).
>
> -# pylint: disable-msg=C0102
> +from __future__ import with_statement
>
> __metaclass__ = type
>
> @@ -11,9 +11,15 @@
> from zope.component import getUtility
>
> from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
> -from canonical.testing import LaunchpadZopelessLayer
> +from canonical.testing import (
> + LaunchpadFunctionalLayer,
> + LaunchpadZopelessLayer,
> + )
> from lp.services.worlddata.interfaces.language import ILanguageSet
> -from lp.testing import TestCaseWithFactory
> +from lp.t...

review: Needs Fixing (code)

[Curtis Hovey (sinzui)]

On Fri, 2010-09-24 at 15:21 +0000, Brad Crittenden wrote:
...
> > === added file 'lib/lp/translations/subscribers.py'
> > --- lib/lp/translations/subscribers.py 1970-01-01 00:00:00 +0000
> > +++ lib/lp/translations/subscribers.py 2010-09-24 14:17:10 +0000
>
> > +def product_modified(product, event):
> > + """Update translations objects when the product changes."""
> > + old_owner = event.object_before_modification.owner
> > + if old_owner != product.owner:
> > + import_queue = getUtility(ITranslationImportQueue)
> > + # The change in ownership may cause permission issues for
> > + # entries in the translation import queue *if* the old owner has
> > + # less permission now. This issue can be avoided by updating the
> > + # entries's importer to the new owner. This can only be done if the
> > + # new owner does not already have an entry in the queue.
> > + # See bug 635438 for more details.
> > + old_owner_entries = []
> > + new_owner_entries = []
> > + for entry in import_queue.getAllEntries(target=product):
> > + if entry.importer == old_owner:
> > + old_owner_entries.append(entry)
> > + elif entry.importer == product.owner:
> > + new_owner_entries.append(entry)
> > + else:
> > + # There is not permission issue.
>
> Please reword this comment because I'm not sure what you're saying.

# The entry is not affected by the ownership change.

> > + pass
> > + if len(old_owner_entries) > 0 and len(new_owner_entries) == 0:
> > + # The new owner does not have any conflicting entries in the
> > + # queue. Change the old owner's entries to the new owner to
> > + # ensure there is not permissions issues during processing.
>
> s/there is not permissions/there are no permission/ ???

corrected.

> > === modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
> > --- lib/lp/translations/tests/test_translationimportqueue.py 2010-08-20 20:31:18 +0000
> > +++ lib/lp/translations/tests/test_translationimportqueue.py 2010-09-24 14:17:10 +0000
...
> > +class TestProductOwnerEntryImporter(TestCaseWithFactory):
> > + """Test entries update when owners change."""
> > +
> > + layer = LaunchpadFunctionalLayer
> > +
> > + def setUp(self):
> > + super(TestProductOwnerEntryImporter, self).setUp()
> > + self.product = self.factory.makeProduct()
> > + self.old_owner = self.product.owner
> > + self.new_owner = self.factory.makePerson()
> > + with person_logged_in(self.old_owner):
> > + self.product.driver = self.new_owner
> > + self.import_queue = getUtility(ITranslationImportQueue)
> > +
> > + def test_product_change_owner_changes_entry_importer(self):
> > + # Changing the Product owner also updates the importer of the entry.
> > + with person_logged_in(self.old_owner):
> > + entry = self.import_queue.addOrUpdateEntry(
> > + u'po/sr.po', 'foo', True, self.old_owner,
> > + productseries=self.product.series[0])
> > + ...

[Brad Crittenden (bac)]

typo: # This is being initialize. <-- initialized

I think this:

+ if self.owner is None:
+ # This is being initialize.
+ self._owner = new_owner
+ return
+ if self.owner != new_owner:
+ ...

would be more readable as:

+ if self.owner is None:
+ # This is being initialize.
+ self._owner = new_owner
+ elif self.owner != new_owner:
+ ...

review: Approve (code)

[Curtis Hovey (sinzui)]

On Fri, 2010-09-24 at 20:17 +0000, Brad Crittenden wrote:
> Review: Approve code
> typo: # This is being initialize. <-- initialized
>
> I think this:
>
> + if self.owner is None:
> + # This is being initialize.
> + self._owner = new_owner
> + return
> + if self.owner != new_owner:
> + ...
>
> would be more readable as:
>
> + if self.owner is None:
> + # This is being initialize.
> + self._owner = new_owner
> + elif self.owner != new_owner:
> + ...

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2010-09-24 18:58:27 +0000
+++ lib/lp/registry/model/product.py 2010-09-24 20:21:30 +0000
@@ -749,15 +749,17 @@
         changed.
         """
         if self.owner is None:
- # This is being initialize.
+ # This is being initialized.
             self._owner = new_owner
- return
- if self.owner != new_owner:
+ elif self.owner != new_owner:
             old_product = Snapshot(self, providing=providedBy(self))
             self._owner = new_owner
             notify(ObjectModifiedEvent(
                 self, object_before_modification=old_product,
                 edited_fields=['_owner']))
+ else:
+ # The new owner is the same as the current owner.
+ pass

     owner = property(_getOwner, _setOwner)

--
__Curtis C. Hovey_________
http://launchpad.net/

=== modified file 'lib/lp/registry/browser/tests/product-edit-people-view.txt'
--- lib/lp/registry/browser/tests/product-edit-people-view.txt	2010-06-23 18:41:02 +0000
+++ lib/lp/registry/browser/tests/product-edit-people-view.txt	2010-09-25 02:36:04 +0000
@@ -1,15 +1,11 @@
 ProductEditPeopleView
 =====================
 
-Artifact reassignment
----------------------
-
-When a product is re-assigned to another person, objects related to that
-product (product series, product releases and translations in the import
-queue) owned by the same owner/maintainer are also re-assigned to the new
-owner/maintainer.
-
-Firefox is owned by Sample Person (name12)
+Reassignment to an IPerson
+--------------------------
+
+A product can be reassigned to another person. Firefox is owned by Sample
+Person (name12).
 
     >>> from zope.component import getUtility
     >>> from canonical.launchpad.interfaces import IProductSet
@@ -20,31 +16,6 @@
     >>> print sample_person.name
     name12
 
-The series and releases are owned by Sample Person
-
-    >>> [series.name for series in firefox.series]
-    [u'1.0', u'trunk']
-    >>> [series.owner.name for series in firefox.series]
-    [u'name12', u'name12']
-    >>> [release.version for release in firefox.releases]
-    [u'0.9', u'0.9.1', u'0.9.2', u'1.0.0']
-    >>> [release.owner.name for release in firefox.releases]
-    [u'name16', u'name16', u'name16', u'name12']
-
-Add a TranslationImportQueueEntry owned by Sample Person.
-
-    >>> from lp.translations.interfaces.translationimportqueue import (
-    ...     ITranslationImportQueue)
-
-    >>> firefox_productseries = firefox.series[0]
-    >>> translationimportqueue = getUtility(ITranslationImportQueue)
-    >>> login_person(sample_person)
-    >>> entry = translationimportqueue.addOrUpdateEntry(
-    ...     'po/sr.po', 'foo', True, sample_person,
-    ...     productseries=firefox_productseries)
-    >>> print entry.importer.name
-    name12
-
 No Privileges Person is taking over the project, but he cannot access the
 view because he is not yet an owner/maintainer or admin.
 
@@ -71,23 +42,6 @@
     >>> firefox.owner.name
     u'no-priv'
 
-The dependencies are also correctly re-assigned.
-
-    >>> [series.name for series in firefox.series]
-    [u'1.0', u'trunk']
-    >>> [series.owner.name for series in firefox.series]
-    [u'no-priv', u'no-priv']
-    >>> [release.version for release in firefox.releases]
-    [u'0.9', u'0.9.1', u'0.9.2', u'1.0.0']
-    >>> [release.owner.name for release in firefox.releases]
-    [u'name16', u'name16', u'name16', u'no-priv']
-
-    >>> firefox_productseries = firefox.series[0]
-    >>> translationimportqueue = getUtility(ITranslationImportQueue)
-    >>> entry = translationimportqueue.getAllEntries(
-    ...     target=firefox_productseries)
-    >>> print entry[0].importer.name
-    no-priv
 
 Assigning to Registry Administrators
 ------------------------------------

=== modified file 'lib/lp/registry/doc/product.txt'
--- lib/lp/registry/doc/product.txt	2010-09-15 13:25:01 +0000
+++ lib/lp/registry/doc/product.txt	2010-09-25 02:36:04 +0000
@@ -723,68 +723,17 @@
 Changing ownership
 ==================
 
-If the owner of a project changes, all series and productreleases
-owned by the old owner are transfered to the new owner.
+A product owner can be changed by the current owner.
 
     >>> print firefox.owner.name
     name12
 
-    >>> for series in firefox.series:
-    ...     print series.owner.name, series.name
-    name12 1.0
-    name12 trunk
-
-    >>> for release in firefox.releases:
-    ...     print release.owner.name, release.title
-    name16 Mozilla Firefox 0.9 "One Tree Hill"
-    name16 Mozilla Firefox 0.9.1 "One Tree Hill (v2)"
-    name16 Mozilla Firefox 0.9.2 "One (secure) Tree Hill"
-    name12 Mozilla Firefox 1.0.0 "First Stable Release"
-
-    >>> from lp.translations.interfaces.translationimportqueue import (
-    ...     ITranslationImportQueue)
-    >>> import_queue = getUtility(ITranslationImportQueue)
-    >>> entry = import_queue.addOrUpdateEntry(
-    ...     u'po/sr.po', 'foo', True, firefox.owner,
-    ...      productseries=firefox.series[0])
-    >>> foobar = getUtility(IPersonSet).getByEmail("[e-mail]")
-    >>> entry = import_queue.addOrUpdateEntry(
-    ...     u'po/sr.po', 'foo', True, foobar,
-    ...      productseries=firefox.series[1])
-    >>> for entry in import_queue.getAllEntries(target=firefox):
-    ...     print entry.importer.name
-    name12
-    name16
-
-The owner of firefox can be changed.
-
-    >>> login("[e-mail]")
     >>> mark = getUtility(IPersonSet).getByEmail('[e-mail]')
     >>> print mark.name
     mark
 
+    >>> login_person(firefox.owner)
     >>> firefox.owner = mark
 
-Now that the owner for firefox has changed the series and product
-releases previously owned by name12 are now owned by mark.  Those not
-owned by name12 are unchanged.
-
     >>> print firefox.owner.name
     mark
-
-    >>> for series in firefox.series:
-    ...     print series.owner.name, series.name
-    mark 1.0
-    mark trunk
-
-    >>> for release in firefox.releases:
-    ...     print release.owner.name, release.title
-    name16 Mozilla Firefox 0.9 "One Tree Hill"
-    name16 Mozilla Firefox 0.9.1 "One Tree Hill (v2)"
-    name16 Mozilla Firefox 0.9.2 "One (secure) Tree Hill"
-    mark   Mozilla Firefox 1.0.0 "First Stable Release"
-
-    >>> for entry in import_queue.getAllEntries(target=firefox):
-    ...     print entry.importer.name
-    mark
-    name16

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2010-09-14 15:04:06 +0000
+++ lib/lp/registry/model/product.py	2010-09-25 02:36:04 +0000
@@ -17,6 +17,8 @@
 import operator
 
 from lazr.delegates import delegates
+from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.lifecycle.snapshot import Snapshot
 from lazr.restful.error import expose
 import pytz
 from sqlobject import (
@@ -38,7 +40,11 @@
     Unicode,
     )
 from zope.component import getUtility
-from zope.interface import implements
+from zope.event import notify
+from zope.interface import (
+    implements,
+    providedBy,
+    )
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.database.constants import UTC_NOW
@@ -49,9 +55,6 @@
     SQLBase,
     sqlvalues,
     )
-from canonical.launchpad.components.decoratedresultset import (
-    DecoratedResultSet,
-    )
 from canonical.launchpad.interfaces.launchpad import (
     IHasIcon,
     IHasLogo,
@@ -164,9 +167,6 @@
     IHasCustomLanguageCodes,
     )
 from lp.translations.interfaces.translationgroup import TranslationPermission
-from lp.translations.interfaces.translationimportqueue import (
-    ITranslationImportQueue,
-    )
 from lp.translations.model.customlanguagecode import (
     CustomLanguageCode,
     HasCustomLanguageCodesMixin,
@@ -745,22 +745,21 @@
     def _setOwner(self, new_owner):
         """Set the owner.
 
-        Change the owner and change the ownership of related artifacts.
+        Send an IObjectModifiedEvent to notify subscribers that the owner
+        changed.
         """
-        old_owner = self._owner
-        self._owner = new_owner
-        if old_owner is not None:
-            import_queue = getUtility(ITranslationImportQueue)
-            for entry in import_queue.getAllEntries(target=self):
-                if entry.importer == old_owner:
-                    removeSecurityProxy(entry).importer = new_owner
-            for series in self.series:
-                if series.owner == old_owner:
-                    series.owner = new_owner
-            for release in self.releases:
-                if release.owner == old_owner:
-                    release.owner = new_owner
-            Store.of(self).flush()
+        if self.owner is None:
+            # This is being initialized.
+            self._owner = new_owner
+        elif self.owner != new_owner:
+            old_product = Snapshot(self, providing=providedBy(self))
+            self._owner = new_owner
+            notify(ObjectModifiedEvent(
+                self, object_before_modification=old_product,
+                edited_fields=['_owner']))
+        else:
+            # The new owner is the same as the current owner.
+            pass
 
     owner = property(_getOwner, _setOwner)
 

=== modified file 'lib/lp/registry/stories/webservice/xx-project-registry.txt'
--- lib/lp/registry/stories/webservice/xx-project-registry.txt	2010-09-14 18:28:53 +0000
+++ lib/lp/registry/stories/webservice/xx-project-registry.txt	2010-09-25 02:36:04 +0000
@@ -428,16 +428,6 @@
     >>> test_project['owner_link']
     u'http://.../~mark'
 
-    >>> test_series = webservice.get('/test-project/test-series').jsonBody()
-    >>> test_series['owner_link']
-    u'http://.../~mark'
-
-    >>> release_path = '/test-project/test-series/test-milestone'
-    >>> test_project_release = webservice.get(release_path).jsonBody()
-    >>> test_project_release['owner_link']
-    u'http://.../~mark'
-
-
 Read-only attributes, like registrant, cannot be modified via the
 webservice.patch() method.
 

=== modified file 'lib/lp/translations/configure.zcml'
--- lib/lp/translations/configure.zcml	2010-08-19 02:11:43 +0000
+++ lib/lp/translations/configure.zcml	2010-09-25 02:36:04 +0000
@@ -255,6 +255,11 @@
         factory="lp.translations.browser.translations.TranslationsVHostBreadcrumb"
         permission="zope.Public"/>
 
+    <!-- Subscribers to registry objects -->
+
+    <subscriber
+        for="canonical.launchpad.interfaces.IProduct zope.lifecycleevent.interfaces.IObjectModifiedEvent"
+        handler="lp.translations.subscribers.product_modified"/>
 
     <!-- DistroSeriesLanguage and Dummy -->
 

=== added file 'lib/lp/translations/subscribers.py'
--- lib/lp/translations/subscribers.py	1970-01-01 00:00:00 +0000
+++ lib/lp/translations/subscribers.py	2010-09-25 02:36:04 +0000
@@ -0,0 +1,44 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+__all__ = [
+    'product_modified',
+    ]
+
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from lp.translations.interfaces.translationimportqueue import (
+    ITranslationImportQueue,
+    )
+
+
+def product_modified(product, event):
+    """Update translations objects when the product changes."""
+    old_owner = event.object_before_modification.owner
+    if old_owner != product.owner:
+        import_queue = getUtility(ITranslationImportQueue)
+        # The change in ownership may cause permission issues for
+        # entries in the translation import queue *if* the old owner has
+        # less permission now. This issue can be avoided by updating the
+        # entries's importer to the new owner. This can only be done if the
+        # new owner does not already have an entry in the queue.
+        # See bug 635438 for more details.
+        old_owner_entries = []
+        new_owner_entries = []
+        for entry in import_queue.getAllEntries(target=product):
+            if entry.importer == old_owner:
+                old_owner_entries.append(entry)
+            elif entry.importer == product.owner:
+                new_owner_entries.append(entry)
+            else:
+                # The entry is not affected by the ownership change.
+                pass
+        if len(old_owner_entries) > 0 and len(new_owner_entries) == 0:
+            # The new owner does not have any conflicting entries in the
+            # queue. Change the old owner's entries to the new owner to
+            # ensure there is no permissions issues during processing.
+            for entry in old_owner_entries:
+                removeSecurityProxy(entry).importer = product.owner

=== modified file 'lib/lp/translations/tests/test_translationimportqueue.py'
--- lib/lp/translations/tests/test_translationimportqueue.py	2010-08-20 20:31:18 +0000
+++ lib/lp/translations/tests/test_translationimportqueue.py	2010-09-25 02:36:04 +0000
@@ -1,7 +1,7 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=C0102
+from __future__ import with_statement
 
 __metaclass__ = type
 
@@ -11,9 +11,15 @@
 from zope.component import getUtility
 
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
-from canonical.testing import LaunchpadZopelessLayer
+from canonical.testing import (
+    LaunchpadFunctionalLayer,
+    LaunchpadZopelessLayer,
+    )
 from lp.services.worlddata.interfaces.language import ILanguageSet
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
 from lp.testing.factory import LaunchpadObjectFactory
 from lp.translations.interfaces.translationimportqueue import (
     ITranslationImportQueue,
@@ -291,6 +297,45 @@
         self.assertEquals(catalan_valencia, pofile.language)
 
 
+class TestProductOwnerEntryImporter(TestCaseWithFactory):
+    """Test entries update when owners change."""
+
+    layer = LaunchpadFunctionalLayer
+
+    def setUp(self):
+        super(TestProductOwnerEntryImporter, self).setUp()
+        self.product = self.factory.makeProduct()
+        self.old_owner = self.product.owner
+        self.new_owner = self.factory.makePerson()
+        with person_logged_in(self.old_owner):
+            self.product.driver = self.new_owner
+        self.import_queue = getUtility(ITranslationImportQueue)
+
+    def test_product_change_owner_changes_entry_importer(self):
+        # Changing the Product owner also updates the importer of the entry.
+        with person_logged_in(self.old_owner):
+            entry = self.import_queue.addOrUpdateEntry(
+                u'po/sr.po', 'foo', True, self.old_owner,
+                productseries=self.product.series[0])
+            self.product.owner = self.new_owner
+        self.assertEqual(self.new_owner, entry.importer)
+
+    def test_product_change_owner_preserves_entry_importer(self):
+        # When the new owner already has an entry in the product's import
+        # queue, the entry importer is not updated because that would
+        # cause an non-unique key for the entry.
+        with person_logged_in(self.new_owner):
+            new_entry = self.import_queue.addOrUpdateEntry(
+                u'po/sr.po', 'foo', True, self.new_owner,
+                productseries=self.product.series[0])
+        with person_logged_in(self.old_owner):
+            old_entry = self.import_queue.addOrUpdateEntry(
+                u'po/sr.po', 'foo', True, self.old_owner,
+                productseries=self.product.series[0])
+            self.product.owner = self.new_owner
+        self.assertEqual(self.old_owner, old_entry.importer)
+
+
 def test_suite():
     """Add only specific test cases and leave out the base case."""
     suite = unittest.TestSuite()
@@ -300,5 +345,5 @@
         unittest.makeSuite(TestCanSetStatusPOTemplateWithQueuedUser))
     suite.addTest(unittest.makeSuite(TestCanSetStatusPOFileWithQueuedUser))
     suite.addTest(unittest.makeSuite(TestGetGuessedPOFile))
+    suite.addTest(unittest.makeSuite(TestProductOwnerEntryImporter))
     return suite
-