Merge pull request #565 from petrjasek/fix-publish-packaged

fix(publish): avoid sending individual package items

CHANGELOG.md

@@ -4,7 +4,7 @@
 
 ### Added
 
-- Add `USE_TAKES` (default: `True`) config to be able to turn off takes packages.
+- Add `NO_TAKES` (default: `False`) config to be able to turn off takes packages.
 
 ## [1.1] 2016-08-29
 

apps/publish/content/common.py

@@ -137,7 +137,7 @@ def update(self, id, updates, original):
                 if updates.get('associations'):
                     self._refresh_associated_items(updated)  # updates got lost with update
 
-                if self.published_state != CONTENT_STATE.KILLED and app.config.get('USE_TAKES', True):
+                if self.published_state != CONTENT_STATE.KILLED and not app.config.get('NO_TAKES', False):
                     self._process_takes_package(original, updated, updates)
 
                 self._update_archive(original, updates, should_insert_into_versions=auto_publish)
@@ -452,6 +452,9 @@ def _publish_package_items(self, package, updates):
                 raise SuperdeskApiError.badRequestError("Corrected package cannot be empty!")
             items.extend(added_items)
 
+        if not updates.get('groups') and package.get('groups'):  # this saves some typing in tests
+            updates['groups'] = package.get('groups')
+
         if items:
             archive_publish = get_resource_service('archive_publish')
             for guid in items:
@@ -486,6 +489,7 @@ def _publish_package_items(self, package, updates):
 
                 self.package_service.update_field_in_package(updates, package_item[config.ID_FIELD],
                                                              config.VERSION, package_item[config.VERSION])
+
                 if package_item.get('associations'):
                     self.package_service.update_field_in_package(
                         updates,

apps/publish/enqueue/enqueue_service.py

@@ -53,6 +53,12 @@ class EnqueueService:
     def _enqueue_item(self, item):
         if item[ITEM_TYPE] == CONTENT_TYPE.COMPOSITE and item.get(PACKAGE_TYPE):
             return self.publish(doc=item, target_media_type=SUBSCRIBER_TYPES.DIGITAL)
+        elif item[ITEM_TYPE] == CONTENT_TYPE.COMPOSITE and app.config.get('NO_TAKES'):
+            queued = self._publish_package_items(item)
+            if not queued:  # this was only published to subscribers with config.packaged on
+                return self.publish(doc=item, target_media_type=SUBSCRIBER_TYPES.DIGITAL)
+            else:
+                return queued
         elif item[ITEM_TYPE] == CONTENT_TYPE.COMPOSITE:
             return self._publish_package_items(item)
         elif item[ITEM_TYPE] not in [CONTENT_TYPE.TEXT, CONTENT_TYPE.PREFORMATTED]:
@@ -295,7 +301,6 @@ def queue_transmission(self, doc, subscribers, subscriber_codes={}):
         :param list subscribers: List of subscriber dict.
         :return : (list, bool) tuple of list of missing formatters and boolean flag. True if queued else False
         """
-
         try:
             queued = False
             no_formatters = []
@@ -311,6 +316,11 @@ def queue_transmission(self, doc, subscribers, subscriber_codes={}):
                             PACKAGE_TYPE not in doc and destination['config'].get('packaged', False)
                         if embed_package_items:
                             doc = self._embed_package_items(doc)
+
+                        if doc.get(PUBLISHED_IN_PACKAGE) and destination['config'].get('packaged', False) and \
+                                app.config.get('NO_TAKES'):
+                            continue
+
                         # Step 2(a)
                         formatter = get_formatter(destination['format'], doc)
 

features/http_push.feature

@@ -1,12 +1,11 @@
-@wip
 Feature: HTTP Push publishing
 
     @auth
     @http_mock_adapter
     Scenario: Publish a text item without takes package
-        Given config
+        Given config update
         """
-        {"USE_TAKES": false}
+        {"NO_TAKES": true}
         """
         Given "products"
         """

features/publish_publicapi.feature

@@ -448,6 +448,10 @@ Feature: Publish content to the public API
   @auth
   @notification
   Scenario: Publish a composite item with embedded items
+    Given config update
+    """
+    {"NO_TAKES": true}
+    """
     Given empty "archive"
     Given "desks"
         """
@@ -479,7 +483,7 @@ Feature: Publish content to the public API
                 "renditions": {}
             },
             {
-                "headline" : "WA:Navy steps in with WA asylum-seeker boat",
+                "headline" : "text item with embedded pic",
                 "guid" : "item1",
                 "state" : "submitted",
                 "type" : "text",
@@ -668,23 +672,21 @@ Feature: Publish content to the public API
         """
     And we publish "compositeitem" with "publish" type and "published" state
     Then we get OK response
-    And we get notifications
-        """
-        [{"event": "item:publish", "extra": {"item": "compositeitem"}}]
-        """
     And we get existing resource
         """
         {"_current_version": 2, "state": "published", "task":{"desk": "#desks._id#", "stage": "#desks.incoming_stage#"}}
         """
+    And we get notifications
+        """
+        [{"event": "item:publish", "extra": {"item": "compositeitem"}}]
+        """
     When we enqueue published
     When we get "/publish_queue"
-    Then we get existing resource
+    Then we get list with 1 items
         """
         {"_items":
         	[
-        		{"item_id" : "compositeitem", "state": "pending", "content_type": "composite"},
-        		{"state": "pending", "content_type": "composite", "published_in_package": "compositeitem"},
-        		{"item_id" : "item2", "state": "pending", "content_type": "picture", "published_in_package": "compositeitem"}
+        		{"item_id" : "compositeitem", "state": "pending", "content_type": "composite"}
         	]
         }
         """
@@ -702,8 +704,8 @@ Feature: Publish content to the public API
     		"associations": {
     			"main": {
                     "body_html": "item content",
-					"headline": "WA:Navy steps in with WA asylum-seeker boat",
-					"type": "composite",
+					"headline": "text item with embedded pic",
+					"type": "text",
                     "associations": {
                         "embedded1": {
                             "type": "picture",

superdesk/factory/default_settings.py

@@ -452,3 +452,6 @@ def env(variable, fallback_value=None):
 
 # This setting is used to overide the desk/stage expiry for items to expire from the spike
 SPIKE_EXPIRY_MINUTES = None
+
+# toggle on/off takes packages creation
+NO_TAKES = False

superdesk/tests/__init__.py

@@ -118,10 +118,12 @@ def setup(context=None, config=None, app_factory=get_app):
     })
 
     app = app_factory(app_config)
+
     logging.getLogger('superdesk').setLevel(logging.WARNING)
     logging.getLogger('elastic').setLevel(logging.WARNING)  # elastic datalayer
     logging.getLogger('elasticsearch').setLevel(logging.WARNING)
     logging.getLogger('urllib3').setLevel(logging.WARNING)
+
     drop_elastic(app)
     drop_mongo(app)
 

superdesk/tests/steps.py

@@ -14,10 +14,10 @@
 import arrow
 from datetime import datetime, timedelta
 
+import superdesk.tests as tests
 from superdesk.io import registered_feeding_services
 from superdesk.io.commands.update_ingest import LAST_ITEM_UPDATE
 import superdesk
-import superdesk.tests as tests
 from behave import given, when, then  # @UnresolvedImport
 from flask import json
 from eve.methods.common import parse
@@ -278,6 +278,15 @@ def get_resource_name(url):
     return basename(parsed_url.path)
 
 
+def format_items(items):
+    output = ['']  # insert empty line
+    for item in items:
+        if item.get('formatted_item'):
+            item['formatted_item'] = json.loads(item['formatted_item'])
+        output.append(json.dumps(item, indent=4, sort_keys=True))
+    return ',\n'.join(output)
+
+
 @given('empty "{resource}"')
 def step_impl_given_empty(context, resource):
     if not is_user_resource(resource):
@@ -344,6 +353,11 @@ def step_impl_given_resource_with_provider(context, provider):
         context.resource = resource
 
 
+@given('config update')
+def given_config_update(context):
+    context.app.config.update(json.loads(context.text))
+
+
 @given('config')
 def step_impl_given_config(context):
     tests.setup(context, json.loads(context.text))
@@ -829,7 +843,8 @@ def step_impl_then_get_list(context, total_count):
     if '+' in total_count:
         assert int_count <= data['_meta']['total'], '%d items is not enough' % data['_meta']['total']
     else:
-        assert int_count == data['_meta']['total'], 'got %d' % (data['_meta']['total'])
+        assert int_count == data['_meta']['total'], 'got %d: %s' % (data['_meta']['total'],
+                                                                    format_items(data['_items']))
     if context.text:
         test_json(context)
 
@@ -1423,6 +1438,7 @@ def step_when_we_reset_notifications(context):
 
 @then('we get notifications')
 def then_we_get_notifications(context):
+    assert hasattr(context.app.notification_client, 'messages'), 'no messages'
     notifications = context.app.notification_client.messages
     notifications_data = [json.loads(notification) for notification in notifications]
     context_data = json.loads(apply_placeholders(context, context.text))