launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08653
[Merge] lp:~sinzui/launchpad/uncommercial-projects into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/uncommercial-projects into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1008658 in Launchpad itself: "Lp sends commercial expiration emails to project that transitioned to open"
https://bugs.launchpad.net/launchpad/+bug/1008658
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/uncommercial-projects/+merge/109404
Pre-implementation: jcsackett
Testing on QA staging revealed that Launchpad does not know how to
handle project that *had* a commercial subscription, but allowed it
to expire. The project is open source and it does not have any
commercial features enabled. Projects like /swift will get an
expiration notice every day.
--------------------------------------------------------------------
RULES
* Add a delete method to CommercialSubscription that ensures you
cannot delete an active subscription.
* At the end of the expiration job's cleanup, it deletes the
commercial subscription if the project is open source.
* The project will be ignored by subsequent runs of commercial jobs
because they only search for projects with commercial subscriptions.
* This also ensures users do not see the expired commercial subscription
message on the project page.
ADDENDUM
* The jobs runner was not setup to run the jobs when they are created.
* The configuration overlaps with the daily_product_jobs script.
Update the schema so that the job runner and the cronscript share
information.
QA
* Prepare and run a clean up script that deletes the expired commercial
subscriptions of noncommercial projects.
* This is a separate task that will also be run on production.
* Ask a WebOps to run ./cronscripts/daily_product_jobs.py -vvv
* Verify that the script reports that 14 or more jobs were created.
* Verify that a ThirtyDay... job were created for numerink.
* Verify that an Expired... job were created for sausalito.
* Verify that sausalito is deactivated.
* Verify swift did not get a job created.
LINT
cronscripts/daily_product_jobs.py
database/schema/security.cfg
lib/lp/registry/errors.py
lib/lp/registry/interfaces/commercialsubscription.py
lib/lp/registry/model/commercialsubscription.py
lib/lp/registry/model/productjob.py
lib/lp/registry/tests/test_commercial_subscription.py
lib/lp/registry/tests/test_productjob.py
lib/lp/services/config/schema-lazr.conf
TEST
./bin/test -vvc lp.registry.tests.test_productjob
./bin/test -vvc lp.registry.tests.test_commercial_subscription
IMPLEMENTATION
Added the three commercial subscription jobs to run in the job source groups.
Updated the cronscript to share the config section.
cronscripts/daily_product_jobs.py
lib/lp/services/config/schema-lazr.conf
Updated Commercial subscription to support delete when the subscription is
expired.
lib/lp/registry/errors.py
lib/lp/registry/interfaces/commercialsubscription.py
lib/lp/registry/model/commercialsubscription.py
lib/lp/registry/tests/test_commercial_subscription.py
Updated the CommercialExpiredJob to delete the subscription if the project
is not proprietary. This is a case were a non-proprietary project is no
longer needs commercial features. I also removed some duplication in the tests.
database/schema/security.cfg
lib/lp/registry/model/productjob.py
lib/lp/registry/tests/test_productjob.py
--
https://code.launchpad.net/~sinzui/launchpad/uncommercial-projects/+merge/109404
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/uncommercial-projects into lp:launchpad.
=== modified file 'cronscripts/daily_product_jobs.py'
--- cronscripts/daily_product_jobs.py 2012-05-31 19:58:01 +0000
+++ cronscripts/daily_product_jobs.py 2012-06-08 17:33:26 +0000
@@ -21,7 +21,7 @@
def __init__(self):
name = 'daily_product_jobs'
- dbuser = config.product_job.dbuser
+ dbuser = config.ICommercialExpiredJob.dbuser
LaunchpadCronScript.__init__(self, name, dbuser)
def main(self):
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2012-06-05 02:03:44 +0000
+++ database/schema/security.cfg 2012-06-08 17:33:26 +0000
@@ -2367,7 +2367,7 @@
[product-job]
groups=script
public.account = SELECT
-public.commercialsubscription = SELECT
+public.commercialsubscription = SELECT, UPDATE, DELETE
public.emailaddress = SELECT
public.job = SELECT, INSERT, UPDATE
public.person = SELECT
=== modified file 'lib/lp/registry/errors.py'
--- lib/lp/registry/errors.py 2012-02-14 02:22:59 +0000
+++ lib/lp/registry/errors.py 2012-06-08 17:33:26 +0000
@@ -5,6 +5,7 @@
__all__ = [
'DistroSeriesDifferenceError',
'NotADerivedSeriesError',
+ 'CannotDeleteCommercialSubscription',
'CannotTransitionToCountryMirror',
'CommercialSubscribersOnly',
'CountryMirrorAlreadySet',
@@ -185,3 +186,7 @@
@error_status(httplib.BAD_REQUEST)
class PPACreationError(Exception):
"""Raised when there is an issue creating a new PPA."""
+
+
+class CannotDeleteCommercialSubscription(Exception):
+ """Raised when a commercial subscription cannot be deleted."""
=== modified file 'lib/lp/registry/interfaces/commercialsubscription.py'
--- lib/lp/registry/interfaces/commercialsubscription.py 2012-05-24 20:25:54 +0000
+++ lib/lp/registry/interfaces/commercialsubscription.py 2012-06-08 17:33:26 +0000
@@ -102,3 +102,9 @@
title=_('Active'),
readonly=True,
description=_("Whether this subscription is active.")))
+
+ def delete():
+ """Delete the expired Commercial Subscription.
+
+ :raises: CannotDeleteCommercialSubscription when is_active is True.
+ """
=== modified file 'lib/lp/registry/model/commercialsubscription.py'
--- lib/lp/registry/model/commercialsubscription.py 2011-12-30 06:14:56 +0000
+++ lib/lp/registry/model/commercialsubscription.py 2012-06-08 17:33:26 +0000
@@ -15,6 +15,7 @@
)
from zope.interface import implements
+from lp.registry.errors import CannotDeleteCommercialSubscription
from lp.registry.interfaces.commercialsubscription import (
ICommercialSubscription,
)
@@ -44,5 +45,13 @@
@property
def is_active(self):
+ """See `ICommercialSubscription`"""
now = datetime.datetime.now(pytz.timezone('UTC'))
return self.date_starts < now < self.date_expires
+
+ def delete(self):
+ """See `ICommercialSubscription`"""
+ if self.is_active:
+ raise CannotDeleteCommercialSubscription(
+ "This CommercialSubscription is still active.")
+ self.destroySelf()
=== modified file 'lib/lp/registry/model/productjob.py'
--- lib/lp/registry/model/productjob.py 2012-05-31 19:14:36 +0000
+++ lib/lp/registry/model/productjob.py 2012-06-08 17:33:26 +0000
@@ -470,6 +470,7 @@
for series in self.product.series:
if series.branch.private:
removeSecurityProxy(series).branch = None
+ self.product.commercial_subscription.delete()
def run(self):
"""See `ProductNotificationJob`."""
=== added file 'lib/lp/registry/tests/test_commercial_subscription.py'
--- lib/lp/registry/tests/test_commercial_subscription.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/tests/test_commercial_subscription.py 2012-06-08 17:33:26 +0000
@@ -0,0 +1,41 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+"""Tests for the commercialsubscriptiojn module."""
+
+__metaclass__ = type
+
+from datetime import timedelta
+
+from zope.security.proxy import removeSecurityProxy
+
+from lp.registry.errors import CannotDeleteCommercialSubscription
+from lp.registry.interfaces.product import License
+from lp.services.propertycache import clear_property_cache
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class CommecialSubscriptionTestCase(TestCaseWithFactory):
+ """Test CommercialSubscription."""
+ layer = DatabaseFunctionalLayer
+
+ def test_delete_raises_error_when_active(self):
+ # Active commercial subscriptions cannot be deleted.
+ product = self.factory.makeProduct(
+ licenses=[License.OTHER_PROPRIETARY])
+ cs = product.commercial_subscription
+ self.assertIs(True, cs.is_active)
+ self.assertRaises(
+ CannotDeleteCommercialSubscription, cs.delete)
+
+ def test_delete(self):
+ # Inactive commercial subscriptions can be deleted.
+ product = self.factory.makeProduct(
+ licenses=[License.OTHER_PROPRIETARY])
+ cs = product.commercial_subscription
+ date_expires = cs.date_expires - timedelta(days=31)
+ removeSecurityProxy(cs).date_expires = date_expires
+ self.assertIs(False, cs.is_active)
+ cs.delete()
+ clear_property_cache(product)
+ self.assertIs(None, product.commercial_subscription)
=== modified file 'lib/lp/registry/tests/test_productjob.py'
--- lib/lp/registry/tests/test_productjob.py 2012-06-01 13:38:25 +0000
+++ lib/lp/registry/tests/test_productjob.py 2012-06-08 17:33:26 +0000
@@ -57,12 +57,20 @@
)
from lp.testing.mail_helpers import pop_notifications
from lp.services.log.logger import BufferLogger
+from lp.services.propertycache import clear_property_cache
from lp.services.scripts.tests import run_script
from lp.services.webapp.publisher import canonical_url
class CommercialHelpers:
+ @staticmethod
+ def expire_commercial_subscription(product):
+ expired_date = (
+ product.commercial_subscription.date_expires - timedelta(days=365))
+ removeSecurityProxy(
+ product.commercial_subscription).date_expires = expired_date
+
def make_expiring_product(self, date_expires, job_class=None):
product = self.factory.makeProduct(
licenses=[License.OTHER_PROPRIETARY])
@@ -538,10 +546,7 @@
licenses=[License.OTHER_PROPRIETARY])
commercial_subscription = product.commercial_subscription
if self.EXPIRE_SUBSCRIPTION:
- expired_date = (
- commercial_subscription.date_expires - timedelta(days=365))
- removeSecurityProxy(
- commercial_subscription).date_expires = expired_date
+ self.expire_commercial_subscription(product)
iso_date = commercial_subscription.date_expires.date().isoformat()
job = self.JOB_CLASS.create(product, reviewer)
pop_notifications()
@@ -635,13 +640,16 @@
# When the project is proprietary, the product is deactivated.
product, reviewer = self.make_notification_data(
licenses=[License.OTHER_PROPRIETARY])
+ self.expire_commercial_subscription(product)
job = CommercialExpiredJob.create(product, reviewer)
job._deactivateCommercialFeatures()
+ clear_property_cache(product)
self.assertIs(False, product.active)
+ self.assertIsNot(None, product.commercial_subscription)
def test_deactivateCommercialFeatures_open_source(self):
# When the project is open source, the product's commercial features
- # are deactivated.
+ # are deactivated and the commercial subscription is deleted.
product, reviewer = self.make_notification_data(licenses=[License.MIT])
public_branch = self.factory.makeBranch(
owner=product.owner, product=product)
@@ -654,21 +662,21 @@
public_series.branch = public_branch
private_series = product.newSeries(
product.owner, 'special', 'testing', branch=private_branch)
+ self.expire_commercial_subscription(product)
job = CommercialExpiredJob.create(product, reviewer)
job._deactivateCommercialFeatures()
+ clear_property_cache(product)
self.assertIs(True, product.active)
self.assertIs(False, product.private_bugs)
self.assertEqual(public_branch, public_series.branch)
self.assertIs(None, private_series.branch)
+ self.assertIs(None, product.commercial_subscription)
def test_run_deactivation_performed(self):
# An email is sent and the deactivation steps are performed.
product, reviewer = self.make_notification_data(
licenses=[License.OTHER_PROPRIETARY])
- expired_date = (
- product.commercial_subscription.date_expires - timedelta(days=365))
- removeSecurityProxy(
- product.commercial_subscription).date_expires = expired_date
+ self.expire_commercial_subscription(product)
job = CommercialExpiredJob.create(product, reviewer)
job.run()
self.assertIs(False, product.active)
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2012-06-03 23:11:40 +0000
+++ lib/lp/services/config/schema-lazr.conf 2012-06-08 17:33:26 +0000
@@ -1472,12 +1472,6 @@
dbuser: productreleasefinder
-[product_job]
-# The database user which will be used by this process.
-# datatype: string
-dbuser: product-job
-
-
[profiling]
# When set to True, the user is allowed to request profiles be run for
# either individual pages or for every request sent.
@@ -1755,12 +1749,20 @@
# dbuser, the crontab_group, and the module that the job source class
# can be loaded from.
job_sources:
+ ICommercialExpiredJob,
IMembershipNotificationJobSource,
IPersonMergeJobSource,
IPlainPackageCopyJobSource,
IQuestionEmailJobSource,
IRemoveBugSubscriptionsJobSource,
- IRemoveGranteeSubscriptionsJobSource
+ IRemoveGranteeSubscriptionsJobSource,
+ ISevenDayCommercialExpirationJobSource,
+ IThirtyDayCommercialExpirationJobSource
+
+[ICommercialExpiredJob]
+module: lp.registry.interfaces.productjob
+dbuser: product-job
+crontab_group: MAIN
[IMembershipNotificationJobSource]
# This section is used by cronscripts/process-job-source.py.
@@ -1798,6 +1800,16 @@
dbuser: sharing-jobs
crontab_group: MAIN
+[ISevenDayCommercialExpirationJobSource]
+module: lp.registry.interfaces.productjob
+dbuser: product-job
+crontab_group: MAIN
+
+[IThirtyDayCommercialExpirationJobSource]
+module: lp.registry.interfaces.productjob
+dbuser: product-job
+crontab_group: MAIN
+
[job_runner_queues]
# The names of all queues.
queues: job job_slow branch_write_job branch_write_job_slow
Follow ups
