launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02971
[Merge] lp:~wallyworld/launchpad/recipe-date-last-modified into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/recipe-date-last-modified into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #735900 in Launchpad itself: "Inline editing of recipe doesn't update the date_last_modified property"
https://bugs.launchpad.net/launchpad/+bug/735900
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/recipe-date-last-modified/+merge/53573
When using the inline editing widgets on the recipe page, the date_last_modified is not updated. It is only updated when the recipe is edited using the HTML form.
== Implementation ==
The date_last_modified field was only being set in the HTML form action implemented on the view class. Inline editing of properties doesn't use this code. However, in all cases, regardless of the method used to update the recipe, an ObjectModifiedEvent is published. So the code to set the date_last_modified property was removed from the view code and put into a subscription listener.
== Tests ==
There were no existing tests for the date_last_modified updates on recipes. So tests were created. The makeSourcePackageRecipe factory method (and friends) was modified to take an optional date_created parameter so that the date_last_modified property can be set to a user specified value on recipe creation.
lp.code.model.tests.test_sourcepackagerecipe.RecipeDateLastModified
test_initialValue
test_modifiedevent_sets_date_last_updated
lp.code.browser.tests.test_sourcepackagerecipe.TestSourcePackageRecipeEditView
test_edit_recipe_sets_date_last_modified
A test was added for TestSourcePackageRecipeEditView because the recipe update action explicitly publishes an ObjectModifiedEvent.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/code/configure.zcml
lib/lp/code/browser/sourcepackagerecipe.py
lib/lp/code/browser/tests/test_sourcepackagerecipe.py
lib/lp/code/interfaces/sourcepackagerecipe.py
lib/lp/code/model/sourcepackagerecipe.py
lib/lp/code/model/tests/test_sourcepackagerecipe.py
lib/lp/testing/factory.py
./lib/lp/code/browser/tests/test_sourcepackagerecipe.py
482: E501 line too long (80 characters)
1102: E501 line too long (85 characters)
1132: E501 line too long (85 characters)
1167: E501 line too long (85 characters)
232: Line exceeds 78 characters.
482: Line exceeds 78 characters.
1102: Line exceeds 78 characters.
1132: Line exceeds 78 characters.
1154: Line exceeds 78 characters.
1167: Line exceeds 78 characters.
--
https://code.launchpad.net/~wallyworld/launchpad/recipe-date-last-modified/+merge/53573
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/recipe-date-last-modified into lp:launchpad.
=== modified file 'lib/lp/code/browser/sourcepackagerecipe.py'
--- lib/lp/code/browser/sourcepackagerecipe.py 2011-03-02 01:47:19 +0000
+++ lib/lp/code/browser/sourcepackagerecipe.py 2011-03-16 07:17:31 +0000
@@ -49,7 +49,6 @@
SimpleVocabulary,
)
-from canonical.database.constants import UTC_NOW
from canonical.launchpad import _
from canonical.launchpad.browser.launchpad import Hierarchy
from canonical.launchpad.webapp import (
@@ -852,9 +851,6 @@
form_field.__name__ for form_field in self.form_fields]
notify(ObjectModifiedEvent(
self.context, recipe_before_modification, field_names))
- # Only specify that the context was modified if there
- # was in fact a change.
- self.context.date_last_modified = UTC_NOW
self.next_url = canonical_url(self.context)
=== modified file 'lib/lp/code/browser/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2011-03-02 22:38:44 +0000
+++ lib/lp/code/browser/tests/test_sourcepackagerecipe.py 2011-03-16 07:17:31 +0000
@@ -15,14 +15,16 @@
from textwrap import dedent
from mechanize import LinkNotFoundError
-from pytz import utc
+from pytz import UTC
from testtools.matchers import Equals
import transaction
from zope.component import getUtility
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
+from canonical.database.constants import UTC_NOW
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
from canonical.launchpad.testing.pages import (
extract_text,
find_main_content,
@@ -40,8 +42,8 @@
from lp.buildmaster.enums import BuildStatus
from lp.code.browser.sourcepackagerecipe import (
SourcePackageRecipeRequestBuildsView,
- SourcePackageRecipeView,
- )
+ SourcePackageRecipeEditView,
+ SourcePackageRecipeView)
from lp.code.browser.sourcepackagerecipebuild import (
SourcePackageRecipeBuildView,
)
@@ -63,6 +65,7 @@
ANONYMOUS,
BrowserTestCase,
login,
+ login_person,
person_logged_in,
TestCaseWithFactory,
time_counter,
@@ -201,8 +204,6 @@
return extract_text(tags)
-
-
class TestSourcePackageRecipeAddViewInitalValues(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
@@ -775,6 +776,24 @@
self.assertThat(
'PPA 2', MatchesPickerText(content, 'edit-daily_build_archive'))
+ def test_edit_recipe_sets_date_last_modified(self):
+ """Editing a recipe sets the date_last_modified property."""
+ date_created = datetime(2000, 1, 1, 12, tzinfo=UTC)
+ recipe = self.factory.makeSourcePackageRecipe(
+ owner=self.chef, registrant=self.chef,
+ name=u'things', description=u'This is a recipe',
+ distroseries=self.squirrel, date_created=date_created)
+
+ login_person(self.chef)
+ view = SourcePackageRecipeEditView(recipe, LaunchpadTestRequest())
+ view.initialize()
+ view.request_action.success({
+ 'name': u'fings',
+ 'recipe_text': recipe.recipe_text,
+ 'distros': recipe.distroseries})
+ self.assertSqlAttributeEqualsDate(
+ recipe, 'date_last_modified', UTC_NOW)
+
def test_admin_edit(self):
self.factory.makeDistroSeries(
displayname='Mumbly Midget', name='mumbly',
@@ -1041,8 +1060,8 @@
build = removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
build.status = BuildStatus.FULLYBUILT
- build.date_started = datetime(2010, 03, 16, tzinfo=utc)
- build.date_finished = datetime(2010, 03, 16, tzinfo=utc)
+ build.date_started = datetime(2010, 03, 16, tzinfo=UTC)
+ build.date_finished = datetime(2010, 03, 16, tzinfo=UTC)
self.assertTextMatchesExpressionIgnoreWhitespace("""\
Master Chef Recipes cake_recipe
@@ -1073,8 +1092,8 @@
build = removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
build.status = BuildStatus.FULLYBUILT
- build.date_started = datetime(2010, 03, 16, tzinfo=utc)
- build.date_finished = datetime(2010, 03, 16, tzinfo=utc)
+ build.date_started = datetime(2010, 03, 16, tzinfo=UTC)
+ build.date_finished = datetime(2010, 03, 16, tzinfo=UTC)
build.log = self.factory.makeLibraryFileAlias()
self.assertTextMatchesExpressionIgnoreWhitespace("""\
@@ -1089,8 +1108,8 @@
build = removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
build.status = BuildStatus.FULLYBUILT
- build.date_started = datetime(2010, 03, 16, tzinfo=utc)
- build.date_finished = datetime(2010, 03, 16, tzinfo=utc)
+ build.date_started = datetime(2010, 03, 16, tzinfo=UTC)
+ build.date_finished = datetime(2010, 03, 16, tzinfo=UTC)
build.log = self.factory.makeLibraryFileAlias()
package_name = self.factory.getOrMakeSourcePackageName('chocolate')
source_package_release = self.factory.makeSourcePackageRelease(
@@ -1120,8 +1139,8 @@
build = removeSecurityProxy(self.factory.makeSourcePackageRecipeBuild(
recipe=recipe, distroseries=self.squirrel, archive=self.ppa))
build.status = BuildStatus.FULLYBUILT
- build.date_started = datetime(2010, 03, 16, tzinfo=utc)
- build.date_finished = datetime(2010, 03, 16, tzinfo=utc)
+ build.date_started = datetime(2010, 03, 16, tzinfo=UTC)
+ build.date_finished = datetime(2010, 03, 16, tzinfo=UTC)
build.log = self.factory.makeLibraryFileAlias()
package_name = self.factory.getOrMakeSourcePackageName('chocolate')
source_package_release = self.factory.makeSourcePackageRelease(
@@ -1138,8 +1157,8 @@
processor=builder.processor))
binary_build.queueBuild()
binary_build.status = BuildStatus.FULLYBUILT
- binary_build.date_started = datetime(2010, 04, 16, tzinfo=utc)
- binary_build.date_finished = datetime(2010, 04, 16, tzinfo=utc)
+ binary_build.date_started = datetime(2010, 04, 16, tzinfo=UTC)
+ binary_build.date_finished = datetime(2010, 04, 16, tzinfo=UTC)
binary_build.log = self.factory.makeLibraryFileAlias()
self.assertTextMatchesExpressionIgnoreWhitespace("""\
@@ -1197,7 +1216,7 @@
# We create builds in time ascending order (oldest first) since we
# use id as the ordering attribute and lower ids mean created earlier.
date_gen = time_counter(
- datetime(2010, 03, 16, tzinfo=utc), timedelta(days=1))
+ datetime(2010, 03, 16, tzinfo=UTC), timedelta(days=1))
build1 = self.makeBuildJob(recipe, date_gen.next())
build2 = self.makeBuildJob(recipe, date_gen.next())
build3 = self.makeBuildJob(recipe, date_gen.next())
@@ -1516,7 +1535,7 @@
view.context.buildqueue_record.job.start()
clear_property_cache(view)
self.assertTrue(view.estimate)
- removeSecurityProxy(view.context).date_finished = datetime.now(utc)
+ removeSecurityProxy(view.context).date_finished = datetime.now(UTC)
clear_property_cache(view)
self.assertFalse(view.estimate)
@@ -1534,7 +1553,7 @@
self.assertIs(None, view.eta)
queue_entry = self.factory.makeSourcePackageRecipeBuildJob(
recipe_build=build)
- queue_entry._now = lambda: datetime(1970, 1, 1, 0, 0, 0, 0, utc)
+ queue_entry._now = lambda: datetime(1970, 1, 1, 0, 0, 0, 0, UTC)
self.factory.makeBuilder()
clear_property_cache(view)
self.assertIsNot(None, view.eta)
@@ -1576,7 +1595,7 @@
self.makeBinaryBuild(release, 'itanic')
naked_build = removeSecurityProxy(release.source_package_recipe_build)
naked_build.status = BuildStatus.FULLYBUILT
- naked_build.date_finished = datetime(2009, 1, 1, tzinfo=utc)
+ naked_build.date_finished = datetime(2009, 1, 1, tzinfo=UTC)
naked_build.date_started = (
naked_build.date_finished - timedelta(minutes=1))
naked_build.buildqueue_record.destroySelf()
=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml 2011-03-07 07:27:56 +0000
+++ lib/lp/code/configure.zcml 2011-03-16 07:17:31 +0000
@@ -985,6 +985,10 @@
<allow attributes="as_tuple recipe_branch nest_path" />
</class>
+ <subscriber
+ for="lp.code.interfaces.sourcepackagerecipe.ISourcePackageRecipe zope.lifecycleevent.interfaces.IObjectModifiedEvent"
+ handler="lp.code.model.sourcepackagerecipe.recipe_modified"/>
+
<utility component="lp.code.model.sourcepackagerecipebuild.SourcePackageRecipeBuildJob"
name="RECIPEBRANCHBUILD"
provides="lp.buildmaster.interfaces.buildfarmjob.IBuildFarmJob"/>
=== modified file 'lib/lp/code/interfaces/sourcepackagerecipe.py'
--- lib/lp/code/interfaces/sourcepackagerecipe.py 2011-03-11 23:54:32 +0000
+++ lib/lp/code/interfaces/sourcepackagerecipe.py 2011-03-16 07:17:31 +0000
@@ -261,7 +261,7 @@
"""
def new(registrant, owner, distroseries, name,
- builder_recipe, description):
+ builder_recipe, description, date_created):
"""Create an `ISourcePackageRecipe`."""
def exists(owner, name):
=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py 2011-03-15 14:32:17 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py 2011-03-16 07:17:31 +0000
@@ -41,6 +41,7 @@
implements,
)
+from canonical.database.constants import UTC_NOW
from canonical.database.datetimecol import UtcDateTimeCol
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
from canonical.launchpad.interfaces.lpstorm import (
@@ -91,6 +92,15 @@
return buildables
+def recipe_modified(recipe, event):
+ """Update the date_last_modified property when a recipe is modified.
+
+ This method is registered as a subscriber to `IObjectModifiedEvent` events
+ on recipes.
+ """
+ recipe.date_last_modified = UTC_NOW
+
+
class NonPPABuildRequest(Exception):
"""A build was requested to a non-PPA and this is currently
unsupported."""
@@ -176,7 +186,8 @@
@staticmethod
def new(registrant, owner, name, recipe, description,
- distroseries=None, daily_build_archive=None, build_daily=False):
+ distroseries=None, daily_build_archive=None, build_daily=False,
+ date_created=None):
"""See `ISourcePackageRecipeSource.new`."""
store = IMasterStore(SourcePackageRecipe)
sprecipe = SourcePackageRecipe()
@@ -191,6 +202,8 @@
sprecipe.description = description
sprecipe.daily_build_archive = daily_build_archive
sprecipe.build_daily = build_daily
+ if date_created is not None:
+ sprecipe.date_created = date_created
store.add(sprecipe)
return sprecipe
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py 2011-03-15 14:32:17 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py 2011-03-16 07:17:31 +0000
@@ -16,9 +16,12 @@
from storm.locals import Store
import transaction
from zope.component import getUtility
+from zope.event import notify
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
+from lazr.lifecycle.event import ObjectModifiedEvent
+from canonical.database.constants import UTC_NOW
from canonical.launchpad.webapp.authorization import check_permission
from canonical.launchpad.webapp.testing import verifyObject
from canonical.testing.layers import (
@@ -855,6 +858,31 @@
child_branch, "zam", self.merged_branch.bzr_identity, revspec="2")
+class RecipeDateLastModified(TestCaseWithFactory):
+ """Exercises the situations where date_last_modified is updated."""
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ TestCaseWithFactory.setUp(self, 'test@xxxxxxxxxxxxx')
+
+ def test_initialValue(self):
+ """Initially the date_last_modifed is the date_created."""
+ recipe = self.factory.makeSourcePackageRecipe()
+ self.assertEqual(recipe.date_last_modified, recipe.date_created)
+
+ def test_modifiedevent_sets_date_last_updated(self):
+ # We publish an object modified event to check that the last modified
+ # date is set to UTC_NOW.
+ date_created = datetime(2000, 1, 1, 12, tzinfo=UTC)
+ recipe = self.factory.makeSourcePackageRecipe(
+ date_created=date_created)
+ field = ISourcePackageRecipe['name']
+ notify(ObjectModifiedEvent(
+ removeSecurityProxy(recipe), recipe, [field]))
+ self.assertSqlAttributeEqualsDate(
+ recipe, 'date_last_modified', UTC_NOW)
+
+
class TestWebservice(TestCaseWithFactory):
layer = AppServerLayer
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2011-03-15 23:13:37 +0000
+++ lib/lp/testing/factory.py 2011-03-16 07:17:31 +0000
@@ -2073,11 +2073,11 @@
for version in versions:
entry = dedent('''
%s (%s) unstable; urgency=low
-
- * %s.
-
+
+ * %s.
+
-- Foo Bar <foo@xxxxxxxxxxx> Tue, 01 Jan 1970 01:50:41 +0000
-
+
''' % (spn, version, version))
changelog += entry
return self.makeLibraryFileAlias(content=changelog)
@@ -2492,7 +2492,8 @@
distroseries=None, name=None,
description=None, branches=(),
build_daily=False, daily_build_archive=None,
- is_stale=None, recipe=None):
+ is_stale=None, recipe=None,
+ date_created=None):
"""Make a `SourcePackageRecipe`."""
if registrant is None:
registrant = self.makePerson()
@@ -2514,7 +2515,7 @@
assert branches == ()
source_package_recipe = getUtility(ISourcePackageRecipeSource).new(
registrant, owner, name, recipe, description, [distroseries],
- daily_build_archive, build_daily)
+ daily_build_archive, build_daily, date_created)
if is_stale is not None:
removeSecurityProxy(source_package_recipe).is_stale = is_stale
IStore(source_package_recipe).flush()
