launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+448706@xxxxxxxxxxxxxxxxxx>]

Simone Pelosi has proposed merging ~pelpsi/launchpad:team-subscription-bug into launchpad:master.

Commit message:
Flushing db before txn.commit() and txn.abort()


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/448706
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:team-subscription-bug into launchpad:master.

diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
index 1557a46..fb51750 100644
--- a/lib/lp/services/webapp/publication.py
+++ b/lib/lp/services/webapp/publication.py
@@ -50,6 +50,7 @@ from lp.services.database.interfaces import (
     IStoreSelector,
 )
 from lp.services.database.policy import LaunchpadDatabasePolicy
+from lp.services.database.sqlbase import flush_database_updates
 from lp.services.features.flags import NullFeatureController
 from lp.services.oauth.interfaces import IOAuthSignedRequest
 from lp.services.statsd.interfaces.statsd_client import IStatsdClient
@@ -504,6 +505,7 @@ class LaunchpadBrowserPublication(
 
         # Abort the transaction on a read-only request.
         # NOTHING AFTER THIS SHOULD CAUSE A RETRY.
+        flush_database_updates()
         if request.method in ["GET", "HEAD"]:
             self.finishReadOnlyRequest(txn)
         elif txn.isDoomed():
diff --git a/lib/lp/services/webapp/tests/test_publication.py b/lib/lp/services/webapp/tests/test_publication.py
index a9c246a..cdc1570 100644
--- a/lib/lp/services/webapp/tests/test_publication.py
+++ b/lib/lp/services/webapp/tests/test_publication.py
@@ -14,6 +14,7 @@ from zope.component import getUtility
 from zope.interface import directlyProvides
 from zope.publisher.interfaces import NotFound, Retry
 from zope.publisher.publish import publish
+from zope.security.interfaces import Unauthorized
 from zope.security.management import thread_local as zope_security_thread_local
 
 import lp.services.webapp.adapter as dbadapter
@@ -38,7 +39,13 @@ from lp.services.webapp.servers import (
     WebServicePublication,
 )
 from lp.services.webapp.vhosts import allvhosts
-from lp.testing import ANONYMOUS, TestCase, TestCaseWithFactory, login
+from lp.testing import (
+    ANONYMOUS,
+    BrowserTestCase,
+    TestCase,
+    TestCaseWithFactory,
+    login,
+)
 from lp.testing.fakemethod import FakeMethod
 from lp.testing.layers import DatabaseFunctionalLayer
 
@@ -454,3 +461,26 @@ class TestPublisherStats(StatsMixin, TestCaseWithFactory):
         browser = self.getUserBrowser()
         # This shouldn't raise ValueError
         self.assertRaises(NotFound, browser.open, redirect_url)
+
+
+class TestSubscriptions(BrowserTestCase, TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def test_subscriptions(self):
+        owner = self.factory.makePerson()
+        canonicals = []
+        canonicals.append(self.factory.makePerson())
+
+        self.factory.makeTeam(name="canonical", members=canonicals)
+        ppa = self.factory.makeArchive(owner=owner, private=True)
+
+        self.assertRaises(
+            Unauthorized, self.getViewBrowser, ppa, user=canonicals[0]
+        )
+
+        browser = self.getViewBrowser(
+            ppa, view_name="+subscriptions", user=owner
+        )
+        browser.getControl(name="field.subscriber").value = "canonical"
+        browser.getControl("Add").click()
+        browser = self.getViewBrowser(ppa, user=canonicals[0])