[Launchpad-reviewers] [Merge] ~pappacena/launchpad:stormify-announcement into launchpad:master
The proposal to merge ~pappacena/launchpad:stormify-announcement into launchpad:master has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/395106 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:stormify-announcement. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:stormify-announcement into launchpad:master
The proposal to merge ~pappacena/launchpad:stormify-announcement into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/395106 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:stormify-announcement. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:stormify-announcement into launchpad:master
Review: Approve Diff comments: > diff --git a/lib/lp/registry/model/announcement.py > b/lib/lp/registry/model/announcement.py > index 45b5740..a2c9ac9 100644 > --- a/lib/lp/registry/model/announcement.py > +++ b/lib/lp/registry/model/announcement.py > @@ -151,44 +191,43 @@ class HasAnnouncements: > > def getAnnouncements(self, limit=5, published_only=True): > """See IHasAnnouncements.""" > +from lp.registry.model.product import Product > > # Create the SQL query. > -query = '1=1 ' > +using = [Announcement] > +query = [] > # Filter for published news items if necessary. > if published_only: > -query += """ AND > -Announcement.date_announced <= timezone('UTC'::text, now()) > AND > -Announcement.active IS TRUE > -""" > +query += [ > +Announcement.date_announced <= UTC_NOW, > +Announcement.active == True] > if IProduct.providedBy(self): > if self.projectgroup is None: > -query += """ AND > -Announcement.product = %s""" % sqlvalues(self.id) > +query.append(Announcement.product == self.id) > else: > -query += """ AND > -(Announcement.product = %s OR Announcement.project = %s) > -""" % sqlvalues(self.id, self.projectgroup) > +query.append(Or( > +Announcement.product == self.id, > +Announcement.projectgroup == self.projectgroup)) > elif IProjectGroup.providedBy(self): > -query += """ AND ( > -Announcement.project = %s > -OR Announcement.product IN ( > -SELECT id FROM Product > -WHERE Product.project = %s AND Product.active)) > -""" % sqlvalues(self.id, self.id) > +child_products = Select( > +Product.id, > +And(Product.projectgroup == self, Product.active == True)) > +query.append(Or( > +Announcement.projectgroup == self, > +Announcement.product_id.is_in(child_products))) > elif IDistribution.providedBy(self): > -query += (' AND Announcement.distribution = %s' > -% sqlvalues(self.id)) > +query.append(Announcement.distribution == self) > elif IAnnouncementSet.providedBy(self): > # Just filter out inactive projects, mostly to exclude spam. > -query += """ AND ( > -Announcement.product IS NULL > -OR EXISTS ( > -SELECT 1 FROM Product WHERE > -Product.id = Announcement.product AND Product.active)) > -""" > +using.append( > +LeftJoin(Product, Product.id == Announcement.product_id)) > +query.append(Or( > +Announcement.product == None, > +Product.active == True)) I think this transformation is correct here, but make sure it has good tests. > else: > raise AssertionError('Unsupported announcement target') > -return Announcement.select(query, limit=limit) > +store = IStore(Announcement) > +return store.using(*using).find(Announcement, *query)[:limit] > > > class MakesAnnouncements(HasAnnouncements): -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/395106 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:stormify-announcement. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:stormify-announcement into launchpad:master
Review: Approve looks good -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/395106 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:stormify-announcement. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:stormify-announcement into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:stormify-announcement into launchpad:master. Commit message: Converting Announcements to Storm Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/395106 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:stormify-announcement into launchpad:master. diff --git a/lib/lp/registry/model/announcement.py b/lib/lp/registry/model/announcement.py index 45b5740..a2c9ac9 100644 --- a/lib/lp/registry/model/announcement.py +++ b/lib/lp/registry/model/announcement.py @@ -11,12 +11,21 @@ __all__ = [ 'MakesAnnouncements', ] -from sqlobject import ( -BoolCol, -ForeignKey, -SQLObjectNotFound, -StringCol, +import pytz +import six +from storm.expr import ( +And, +LeftJoin, +Or, +Select, ) +from storm.properties import ( +Bool, +DateTime, +Int, +Unicode, +) +from storm.references import Reference from zope.interface import implementer from lp.registry.interfaces.announcement import ( @@ -28,40 +37,72 @@ from lp.registry.interfaces.person import validate_public_person from lp.registry.interfaces.product import IProduct from lp.registry.interfaces.projectgroup import IProjectGroup from lp.services.database.constants import UTC_NOW -from lp.services.database.datetimecol import UtcDateTimeCol -from lp.services.database.sqlbase import ( -SQLBase, -sqlvalues, +from lp.services.database.interfaces import ( +IMasterStore, +IStore, ) +from lp.services.database.stormbase import StormBase from lp.services.utils import utc_now @implementer(IAnnouncement) -class Announcement(SQLBase): +class Announcement(StormBase): """A news item. These allow us to generate lists of recent news for project groups, products and distributions. """ -_defaultOrder = ['-date_announced', '-date_created'] - -date_created = UtcDateTimeCol( -dbName='date_created', notNull=True, default=UTC_NOW) -date_announced = UtcDateTimeCol(default=None) -date_last_modified = UtcDateTimeCol( -dbName='date_updated', default=None) -registrant = ForeignKey( -dbName='registrant', foreignKey='Person', -storm_validator=validate_public_person, notNull=True) -product = ForeignKey(dbName='product', foreignKey='Product') -projectgroup = ForeignKey(dbName='project', foreignKey='ProjectGroup') -distribution = ForeignKey( -dbName='distribution', foreignKey='Distribution') -title = StringCol(notNull=True) -summary = StringCol(default=None) -url = StringCol(default=None) -active = BoolCol(notNull=True, default=True) +__storm_table__ = 'Announcement' + +__storm_order__ = ('-date_announced', '-date_created') + +id = Int(primary=True) + +date_created = DateTime(allow_none=False, default=UTC_NOW, tzinfo=pytz.UTC) +date_announced = DateTime(allow_none=True, default=None, tzinfo=pytz.UTC) +date_last_modified = DateTime( +name='date_updated', allow_none=True, default=None, tzinfo=pytz.UTC) + +registrant_id = Int( +name="registrant", allow_none=False, validator=validate_public_person) +registrant = Reference(registrant_id, "Person.id") + +product_id = Int(name="product", allow_none=True, default=None) +product = Reference(product_id, "Product.id") + +projectgroup_id = Int(name="project", allow_none=True, default=None) +projectgroup = Reference(projectgroup_id, "ProjectGroup.id") + +distribution_id = Int(name="distribution", allow_none=True, default=None) +distribution = Reference(distribution_id, "Distribution.id") + +title = Unicode(allow_none=False) +summary = Unicode(allow_none=True, default=None) +url = Unicode(allow_none=True, default=None) +active = Bool(allow_none=False, default=True) + +def __init__(self, registrant, title, summary=None, url=None, + active=True, date_created=UTC_NOW, date_announced=None, + date_last_modified=None, product=None, projectgroup=None, + distribution=None): +self.registrant = registrant +self.title = title +self.summary = summary +self.url = url +self.active = active +self.date_created = date_created +self.date_announced = date_announced +self.date_last_modified = date_last_modified +self.product = product +self.projectgroup = projectgroup +self.distribution = distribution + +def destroySelf(self): +IMasterStore(self).remove(self) def modify(self, title, summary, url): +title = six.text_type(title) if title is not None else None +summary = six.text_type(summary) if summary is not None else None +url = six.text_type(url) if url is not None else None if