Code review comment for lp:~henninge/launchpad/bug-314839

=== modified file 'lib/canonical/launchpad/database/potemplate.py'
--- lib/canonical/launchpad/database/potemplate.py 2008-10-10 09:17:54 +0000
+++ lib/canonical/launchpad/database/potemplate.py 2009-01-09 14:51:27 +0000
@@ -734,16 +734,18 @@
     implements(IPOTemplateSubset)

     def __init__(self, sourcepackagename=None, from_sourcepackagename=None,
- distroseries=None, productseries=None):
+ distroseries=None, productseries=None, iscurrent=None):
         """Create a new `POTemplateSubset` object.

         The set of POTemplate depends on the arguments you pass to this
         constructor. The sourcepackagename, from_sourcepackagename,
         distroseries and productseries are just filters for that set.
+ In addition, iscurrent sets the filter for the iscurrent flag.
         """
         self.sourcepackagename = sourcepackagename
         self.distroseries = distroseries
         self.productseries = productseries
+ self.iscurrent = iscurrent
         self.clausetables = []
         self.orderby = ['id']

@@ -773,6 +775,11 @@
             self.orderby.append('DistroSeries.name')
             self.clausetables.append('DistroSeries')

+ # Add the filter for the iscurrent flag if requested.
+ if iscurrent is not None:
+ self.query += " AND POTemplate.iscurrent=%s" % (
+ sqlvalues(iscurrent))
+
         # Finally, we sort the query by its path in all cases.
         self.orderby.append('POTemplate.path')

@@ -932,15 +939,16 @@
         return POTemplate.select(orderBy=['-date_last_updated'])

     def getSubset(self, distroseries=None, sourcepackagename=None,
- productseries=None):
+ productseries=None, iscurrent=None):
         """See `IPOTemplateSet`."""
         return POTemplateSubset(
             distroseries=distroseries,
             sourcepackagename=sourcepackagename,
- productseries=productseries)
+ productseries=productseries,
+ iscurrent=iscurrent)

     def getSubsetFromImporterSourcePackageName(self, distroseries,
- sourcepackagename):
+ sourcepackagename, iscurrent=None):
         """See `IPOTemplateSet`."""
         if distroseries is None or sourcepackagename is None:
             raise AssertionError(
@@ -948,7 +956,8 @@

         return POTemplateSubset(
             distroseries=distroseries,
- sourcepackagename=sourcepackagename)
+ sourcepackagename=sourcepackagename,
+ iscurrent=iscurrent)

     def getPOTemplateByPathAndOrigin(self, path, productseries=None,
         distroseries=None, sourcepackagename=None):

=== modified file 'lib/canonical/launchpad/database/translationimportqueue.py'
--- lib/canonical/launchpad/database/translationimportqueue.py 2009-01-03 19:32:48 +0000
+++ lib/canonical/launchpad/database/translationimportqueue.py 2009-01-09 14:55:51 +0000
@@ -259,7 +259,8 @@
         potemplate_subset = potemplateset.getSubset(
             distroseries=self.distroseries,
             sourcepackagename=self.sourcepackagename,
- productseries=self.productseries)
+ productseries=self.productseries,
+ iscurrent=True)
         potemplate = potemplate_subset.getPOTemplateByTranslationDomain(
             translation_domain)

@@ -270,7 +271,7 @@
             # it in a different source package as a second try. To do it, we
             # need to get a subset of all packages in current distro series.
             potemplate_subset = potemplateset.getSubset(
- distroseries=self.distroseries)
+ distroseries=self.distroseries, iscurrent=True)
             potemplate = potemplate_subset.getPOTemplateByTranslationDomain(
                 translation_domain)

=== modified file 'lib/canonical/launchpad/doc/potemplate.txt'
--- lib/canonical/launchpad/doc/potemplate.txt 2008-09-09 08:30:39 +0000
+++ lib/canonical/launchpad/doc/potemplate.txt 2009-01-09 16:06:55 +0000
@@ -112,13 +112,59 @@
 This method gives us the IPOTemplate that belongs to this subset and its
 translation domain is the given one.

- >>> productseries = ProductSeries.get(5)
+=== Filtering for iscurrent ===
+
+IPOTemplate specifies an "iscurrent" flag that indicates if this template
+should be available for translations or not.
+
+ >>> fooproduct=factory.makeProduct(name="fooproduct")
+ >>> productseries = factory.makeProductSeries(
+ ... product=fooproduct, name="fooseries")
+ >>> current_template = factory.makePOTemplate(
+ ... productseries=productseries, name="current-template",
+ ... translation_domain="foodomain")
+ >>> disabled_template = factory.makePOTemplate(
+ ... productseries=productseries, name="disabled-template",
+ ... translation_domain="foodomain")
+ >>> from zope.security.proxy import removeSecurityProxy
+ >>> removeSecurityProxy(disabled_template).iscurrent = False
+
+The subset can be configured to filter templates on the iscurrent flag. This
+filter is effective for all get* methods but it will only be demonstrated
+here. The filter is applied during creation by specifying the filter value
+as an extra parameter. The default value is "None" which disables the filter
+and both current and disabled templates are returned.
+
+For getPOTemplateByTranslationDomain, this case is an error because only one
+template per translation domain is allowed.
+
     >>> potemplatesubset = potemplate_set.getSubset(
     ... productseries=productseries)
     >>> potemplate = potemplatesubset.getPOTemplateByTranslationDomain(
- ... 'NetApplet')
- >>> print potemplate.title
- Template "netapplet" in NetApplet trunk
+ ... 'foodomain')
+ WARNING:canonical.launchpad.database.potemplate:Found...
+ multiple templates with translation domain 'foodomain'...
+ There should be only one.
+ >>> potemplate is None
+ True
+
+Setting the filter to "True" will only return current (enabled) templates.
+
+ >>> potemplatesubset = potemplate_set.getSubset(
+ ... productseries=productseries, iscurrent=True)
+ >>> potemplate = potemplatesubset.getPOTemplateByTranslationDomain(
+ ... 'foodomain')
+ >>> print potemplate.title
+ Template "current-template" in Fooproduct fooseries
+
+Setting the filter to "False" will only return disabled templates.
+
+ >>> potemplatesubset = potemplate_set.getSubset(
+ ... productseries=productseries, iscurrent=False)
+ >>> potemplate = potemplatesubset.getPOTemplateByTranslationDomain(
+ ... 'foodomain')
+ >>> print potemplate.title
+ Template "disabled-template" in Fooproduct fooseries

 == getClosestPOTemplate ==

=== modified file 'lib/canonical/launchpad/interfaces/potemplate.py'
--- lib/canonical/launchpad/interfaces/potemplate.py 2008-10-07 06:46:36 +0000
+++ lib/canonical/launchpad/interfaces/potemplate.py 2009-01-09 14:54:45 +0000
@@ -471,6 +471,13 @@
             'The `IProductSeries` associated with this subset.'),
         schema=IProductSeries)

+ iscurrent = Bool(
+ title=_("Filter for iscurrent flag."),
+ description=_(
+ "The filter for the iscurrent flag that this subset should "
+ "apply. The filter is disabled if it is None"),
+ required=False)
+
     title = TextLine(
         title=_('The translation file title.'), required=True, readonly=True)

@@ -541,12 +548,12 @@
         """Return an iterator over all POTemplate sorted by modification."""

     def getSubset(distroseries=None, sourcepackagename=None,
- productseries=None):
+ productseries=None, iscurrent=None):
         """Return a POTemplateSubset object depending on the given arguments.
         """

     def getSubsetFromImporterSourcePackageName(
- distroseries, sourcepackagename):
+ distroseries, sourcepackagename, iscurrent=None):
         """Return a POTemplateSubset based on the origin sourcepackagename.
         """