[Launchpad-reviewers] [Merge] ~pappacena/launchpad:stormify-announcement into launchpad:master

2020-12-15 Thread noreply
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

2020-12-15 Thread Thiago F. Pappacena
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

2020-12-15 Thread Colin Watson
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

2020-12-11 Thread Ioana Lasc
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

2020-12-09 Thread Thiago F. Pappacena
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