Review upgrade step 1008-2000

.travis.yml

@@ -6,7 +6,7 @@ cache:
   - eggs
   - parts/node
 addons:
-  firefox: latest-esr
+  firefox: 45.8.0esr
 env:
 - PLONE_VERSION=4.3 TEST_EGGS=collective.cover[test]
 - PLONE_VERSION=4.3 TEST_EGGS=plone.app.contenttypes

CHANGES.rst

@@ -6,7 +6,9 @@ There's a frood who really knows where his towel is.
 2.1b2 (unreleased)
 ^^^^^^^^^^^^^^^^^^
 
-- Nothing changed yet.
+- Fix upgrade process between versions 1.0 and 2.0;
+  check documentation on migration from 1.x to 2.x (closes `#198`_).
+  [rodfersou, hvelarde]
 
 
 2.1b1 (2017-02-16)
@@ -194,3 +196,4 @@ There's a frood who really knows where his towel is.
 .. _`#169`: https://github.com/collective/collective.nitf/issues/169
 .. _`#175`: https://github.com/collective/collective.nitf/issues/175
 .. _`#178`: https://github.com/collective/collective.nitf/issues/178
+.. _`#198`: https://github.com/collective/collective.nitf/issues/198

README.rst

@@ -140,3 +140,27 @@ The following command rebuilds static files and exit (insted of keep watching th
 .. code-block:: bash
 
     $ bin/npm_build
+
+Migration from 1.x to 2.x
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+You have to be aware of the following changes when migrating from version 1.x to 2.x:
+
+* Package is no longer compatible with Plone 4.2
+* Package no longer depends on Grok
+* Package no longer depends on `collective.z3cform.widgets <http://pypi.python.org/pypi/collective.z3cform.widgets>`_;
+  you should uninstall that dependency manually if there is no other package depending on it on your site
+* Package no longer depends on `plone.app.referenceablebehavior <http://pypi.python.org/pypi/plone.app.referenceablebehavior>`_;
+  the ``IReferenceable`` behavior included there is no longer assigned by default
+* The character counter is no longer available
+* We use `Cycle2 <http://jquery.malsup.com/cycle2/>`_ instead of `Galleria <https://galleria.io/>`_ as the framework for the slideshow view;
+  package now depends on `collective.js.cycle2 <https://pypi.python.org/pypi/collective.js.cycle2>`_
+* The following views are available for a News Article: ``view``, ``slideshow_view`` and ``text_only_view``
+* View templates were completely refactored and support for semantic markup was added;
+  the default view displays a bigger image
+* The following behaviors are assigned by default to the News Article content type: ``plone.app.relationfield.behavior.IRelatedItems`` and ``collective.nitf.behaviors.interfaces.ISection``
+* A new permission ``collective.nitf: Setup`` is available to access the control panel configlet and is assigned by default to ``Manager`` and ``Site Administrator`` roles
+* Static resources are now named ``nitf.css`` and ``nitf.js`` (easier to debug at the browser)
+
+An upgrade step is available to remove old resources, rename the views, and reindex all News Articles to reflect changes.
+The upgrade step will not remove the ``plone.app.referenceablebehavior.referenceable.IReferenceable`` behavior if applied.

buildout.cfg

@@ -31,7 +31,7 @@ multiprocessing = True
 pre-commit-hook = True
 return-status-codes = True
 flake8 = True
-flake8-ignore = E501,P001,T000
+flake8-ignore = E501,P001,S001,T000
 
 [instance]
 zope-conf-additional += publisher-profile-file ${buildout:directory}/var/instance/profile.dat

src/collective/nitf/tests/test_upgrades.py

@@ -2,6 +2,7 @@
 from collective.nitf.testing import INTEGRATION_TESTING
 from collective.nitf.testing import IS_PLONE_5
 from plone import api
+from plone.dexterity.interfaces import IDexterityFTI
 from plone.registry import field
 from plone.registry.interfaces import IRegistry
 from plone.registry.record import Record
@@ -16,6 +17,9 @@ class UpgradeTestCaseBase(unittest.TestCase):
 
     def setUp(self, from_version, to_version):
         self.portal = self.layer['portal']
+        self.request = self.layer['request']
+        self.request.set('test', True)  # avoid transaction commits on tests
+
         self.setup = self.portal['portal_setup']
         self.profile_id = u'collective.nitf:default'
         self.from_version = from_version
@@ -30,10 +34,9 @@ def get_upgrade_step(self, title):
 
     def execute_upgrade_step(self, step):
         """Execute an upgrade step."""
-        request = self.layer['request']
-        request.form['profile_id'] = self.profile_id
-        request.form['upgrades'] = [step['id']]
-        self.setup.manage_doUpgrades(request=request)
+        self.request.form['profile_id'] = self.profile_id
+        self.request.form['upgrades'] = [step['id']]
+        self.setup.manage_doUpgrades(request=self.request)
 
     @property
     def total_steps(self):
@@ -83,7 +86,7 @@ def setUp(self):
     def test_upgrade_to_2000_registrations(self):
         version = self.setup.getLastVersionForProfile(self.profile_id)[0]
         self.assertTrue(version >= self.to_version)
-        self.assertEqual(self.total_steps, 4)
+        self.assertEqual(self.total_steps, 8)
 
     @unittest.skipIf(IS_PLONE_5, 'Test not supported under Plone 5')
     def test_character_counter_css_resources(self):
@@ -239,6 +242,49 @@ def test_update_configlet(self):
         new_permissions = ('collective.nitf: Setup',)
         self.assertEqual(configlet.getPermissions(), new_permissions)
 
+    def test_update_behaviors(self):
+        # check if the upgrade step is registered
+        title = u'Update behaviors'
+        step = self.get_upgrade_step(title)
+        self.assertIsNotNone(step)
+
+        # simulate state on previous version
+        from collective.nitf.upgrades.v2000 import BEHAVIORS_TO_ADD
+        REFERENCEABLE = 'plone.app.referenceablebehavior.referenceable.IReferenceable'
+        fti = getUtility(IDexterityFTI, name='collective.nitf.content')
+        fti.behaviors = tuple(
+            set(fti.behaviors) - BEHAVIORS_TO_ADD | set([REFERENCEABLE]))
+        for b in BEHAVIORS_TO_ADD:
+            self.assertNotIn(b, fti.behaviors)
+        self.assertIn(REFERENCEABLE, fti.behaviors)
+
+        # run the upgrade step to validate the update
+        self.execute_upgrade_step(step)
+        self.assertIn('plone.app.relationfield.behavior.IRelatedItems', fti.behaviors)
+        self.assertIn('collective.nitf.behaviors.interfaces.ISection', fti.behaviors)
+        self.assertIn(REFERENCEABLE, fti.behaviors)  # should not be removed
+
+    def test_reindex_news_articles(self):
+        # check if the upgrade step is registered
+        title = u'Reindex news articles'
+        step = self.get_upgrade_step(title)
+        self.assertIsNotNone(step)
+
+        with api.env.adopt_roles(['Manager']):
+            for i in xrange(0, 10):
+                api.content.create(self.portal, 'collective.nitf.content', str(i))
+
+        # break the catalog by deleting an object without notifying
+        self.portal._delObject('0', suppress_events=True)
+        self.assertNotIn('0', self.portal)
+        results = api.content.find(portal_type='collective.nitf.content')
+        self.assertEqual(len(results), 10)  # catalog unaffected
+
+        # run the upgrade step to validate the update
+        self.execute_upgrade_step(step)
+        results = api.content.find(portal_type='collective.nitf.content')
+        self.assertEqual(len(results), 10)  # no failure and catalog unaffected
+
 
 class to2001TestCase(UpgradeTestCaseBase):
 

src/collective/nitf/upgrades/v2000/__init__.py

@@ -2,55 +2,52 @@
 from collective.nitf.config import PROJECTNAME
 from collective.nitf.logger import logger
 from plone import api
+from plone.dexterity.interfaces import IDexterityFTI
+from zope.component import getUtility
 
-import logging
+import transaction
 
 
-def apply_profile(context, logger=None):
+def get_valid_objects(brains):
+    """Generate a list of objects associated with valid brains."""
+    for b in brains:
+        try:
+            obj = b.getObject()
+        except KeyError:
+            obj = None
+
+        if obj is None:  # warn on broken entries in the catalog
+            logger.warn(
+                u'Invalid reference in the catalog: {0}'.format(b.getPath()))
+            continue
+        yield obj
+
+
+def apply_profile(setup_tool):
     """Apply upgrade profile; this includes:
 
     - remove character counter resources from CSS and JS registries
     - remove character counter control panel records from registry
     - rename galleria view in News Article content type
-    - cook CSS and JS resources
     """
-    if logger is None:
-        # Called as upgrade step: define our own logger
-        logger = logging.getLogger(PROJECTNAME)
-
     profile = 'profile-collective.nitf.upgrades.v2000:default'
-    setup = api.portal.get_tool('portal_setup')
-    setup.runAllImportStepsFromProfile(profile)
+    setup_tool.runAllImportStepsFromProfile(profile)
 
-    portal_css = api.portal.get_tool('portal_css')
-    portal_css.cookResources()
-    logger.info(u'CSS resources cooked')
-    portal_js = api.portal.get_tool('portal_javascripts')
-    portal_js.cookResources()
-    logger.info(u'JS resources cooked')
-
-
-def update_layouts(context, logger=None):
-    if logger is None:
-        # Called as upgrade step: define our own logger
-        logger = logging.getLogger(PROJECTNAME)
 
+def update_layouts(setup_tool):
     # update existing objects
-    logger.info(u'Updating layout of News Articles')
-    catalog = api.portal.get_tool('portal_catalog')
-    results = catalog(portal_type='collective.nitf.content')
-    logger.info(u'{0} News Articles found'.format(len(results)))
+    logger.info(u'Updating layout of news articles')
+    results = api.content.find(portal_type='collective.nitf.content')
+    logger.info(u'Found {0} news articles'.format(len(results)))
     i = 0
-    for item in results:
-        obj = item.getObject()
+    for obj in get_valid_objects(results):
         if obj.getLayout() == 'nitf_galleria':
             obj.setLayout('slideshow_view')
             i += 1
-        logger.info(u'{0} News Articles updated'.format(i))
+        logger.info(u'{0} news articles updated'.format(i))
 
 
-def install_new_dependencies(context, logger=None):
-    logger = logging.getLogger(PROJECTNAME)
+def install_new_dependencies(setup_tool):
     dependencies = ('collective.js.cycle2',)
     qi = api.portal.get_tool('portal_quickinstaller')
     for p in dependencies:
@@ -64,3 +61,39 @@ def update_configlet(setup_tool):
     profile = 'profile-{0}:default'.format(PROJECTNAME)
     setup_tool.runImportStepFromProfile(profile, 'controlpanel')
     logger.info('Control panel configlet updated.')
+
+
+BEHAVIORS_TO_ADD = frozenset([
+    'plone.app.relationfield.behavior.IRelatedItems',
+    'collective.nitf.behaviors.interfaces.ISection',
+])
+
+
+def update_behaviors(setup_tool):
+    """Update News Article behaviors."""
+    fti = getUtility(IDexterityFTI, name='collective.nitf.content')
+    fti.behaviors = tuple(set(fti.behaviors) | BEHAVIORS_TO_ADD)
+    logger.info('News Article behaviors updated.')
+
+
+def reindex_news_articles(setup_tool):
+    """Reindex news articles to fix interfaces."""
+    test = 'test' in setup_tool.REQUEST  # used to ignore transactions on tests
+    logger.info(
+        u'Reindexing the catalog. '
+        u'This process could take a long time on large sites. Be patient.'
+    )
+    catalog = api.portal.get_tool('portal_catalog')
+    results = api.content.find(portal_type='collective.nitf.content')
+    logger.info(u'Found {0} news articles'.format(len(results)))
+    n = 0
+    for obj in get_valid_objects(results):
+        catalog.catalog_object(obj, idxs=['object_provides'], update_metadata=False)
+        n += 1
+        if n % 1000 == 0 and not test:
+            transaction.commit()
+            logger.info('{0} items processed.'.format(n))
+
+    if not test:
+        transaction.commit()
+    logger.info('Done.')

src/collective/nitf/upgrades/v2000/configure.zcml

@@ -41,6 +41,30 @@
         handler=".update_configlet"
         />
 
+    <genericsetup:upgradeStep
+        title="Update behaviors"
+        description="Update News Article behaviors."
+        handler=".update_behaviors"
+        />
+
+    <genericsetup:upgradeStep
+        title="Reindex news articles"
+        description="Reindex news articles to fix interfaces."
+        handler=".reindex_news_articles"
+        />
+
+    <genericsetup:upgradeStep
+        title="Cook CSS resources"
+        description="There were changes in the CSS files, so we need to cook the resources."
+        handler="..cook_css_resources"
+        />
+
+    <genericsetup:upgradeStep
+        title="Cook JS resources"
+        description="There were changes in the JS files, so we need to cook the resources."
+        handler="..cook_javascript_resources"
+        />
+
   </genericsetup:upgradeSteps>
 
 </configure>