TP2000-1639 new year envelope publishing failure (#1375)

* List by default, add flags for async and sync publishing

* Fix failing tests

* Correct function name. Call correct queryset filter.

* Set crontab to every two hours to publish to crown dependencies

* Correctly reference workbasket's envelope

publishing/management/commands/publish_to_api.py

@@ -8,28 +8,40 @@
 
 
 class Command(BaseCommand):
-    help = "Upload unpublished envelopes to the Tariff API."
+    help = (
+        "Manage envelope uploads to Tariff API. Without arguments, this "
+        "management command lists unpublished envelopes."
+    )
 
     def add_arguments(self, parser):
         parser.add_argument(
-            "-l",
-            "--list",
-            dest="list",
+            "--publish-async",
             action="store_true",
-            help="List unpublished envelopes.",
+            help=(
+                "Asynchronously run (via a Celery task) the function to "
+                "upload unpublished envelopes to the tariffs-api service."
+            ),
+        )
+        parser.add_argument(
+            "--publish-now",
+            action="store_true",
+            help=(
+                "Immediately run (within the current terminal's process) the "
+                "function to upload unpublished envelopes to the tariffs-api "
+                "service."
+            ),
         )
 
     def get_incomplete_envelopes(self):
-        incomplete = CrownDependenciesEnvelope.objects.unpublished()
-        if not incomplete:
-            return None
-        return incomplete
+        return CrownDependenciesEnvelope.objects.unpublished()
 
     def get_unpublished_envelopes(self):
-        unpublished = PackagedWorkBasket.objects.get_unpublished_to_api()
-        if not unpublished:
-            return None
-        return unpublished
+        return PackagedWorkBasket.objects.get_unpublished_to_api()
+
+    def print_envelope_details(self, position, envelope):
+        self.stdout.write(
+            f"position={position}, pk={envelope.pk}, envelope_id={envelope.envelope_id}",
+        )
 
     def list_unpublished_envelopes(self):
         incomplete = self.get_incomplete_envelopes()
@@ -39,22 +51,33 @@ def list_unpublished_envelopes(self):
                 f"{incomplete.count()} envelope(s) not completed publishing:",
             )
             for i, crowndependencies in enumerate(incomplete, start=1):
-                self.stdout.write(
-                    f"{i}: {crowndependencies.packagedworkbaskets.last().envelope}",
+                self.print_envelope_details(
+                    position=i,
+                    envelope=crowndependencies.packagedworkbaskets.last().envelope,
                 )
         if unpublished:
             self.stdout.write(
                 f"{unpublished.count()} envelope(s) ready to be published in the following order:",
             )
             for i, packaged_work_basket in enumerate(unpublished, start=1):
-                self.stdout.write(f"{i}: {packaged_work_basket.envelope}")
-
-    def handle(self, *args, **options):
-        if options["list"]:
-            self.list_unpublished_envelopes()
-            return
+                self.print_envelope_details(
+                    position=i,
+                    envelope=packaged_work_basket.envelope,
+                )
 
+    def publish(self, now: bool):
         if self.get_unpublished_envelopes() or self.get_incomplete_envelopes():
-            publish_to_api.apply()
+            if now:
+                self.stdout.write(f"Calling `publish_to_api()` now.")
+                publish_to_api()
+            else:
+                self.stdout.write(f"Calling `publish_to_api()` asynchronously.")
+                publish_to_api.apply()
         else:
             sys.exit("No unpublished envelopes")
+
+    def handle(self, *args, **options):
+        if options["publish_async"] or options["publish_now"]:
+            self.publish(now=options["publish_now"])
+        else:
+            self.list_unpublished_envelopes()

publishing/models/packaged_workbasket.py

@@ -243,12 +243,9 @@ def get_unpublished_to_api(self) -> "PackagedWorkBasketQuerySet":
         ).order_by("envelope__envelope_id")
         return unpublished
 
-    def last_unpublished_envelope_id(self) -> "publishing_models.EnvelopeId":
-        """Join PackagedWorkBasket with Envelope and CrownDependenciesEnvelope
-        model selecting objects Where an Envelope model exists and the
-        published_to_tariffs_api field is not null Or Where a
-        CrownDependenciesEnvelope is not null Then select the max value for ther
-        envelope_id field in the Envelope instance."""
+    def last_published_envelope_id(self) -> "publishing_models.EnvelopeId":
+        """Get the envelope_id of the last Envelope successfully published to
+        the Tariffs API service."""
 
         return (
             self.select_related(
@@ -409,20 +406,29 @@ def next_expected_to_api(self) -> bool:
         (this means the envelope is the first to be published to the API)
         """
 
-        previous_id = PackagedWorkBasket.objects.last_unpublished_envelope_id()
+        previous_id = PackagedWorkBasket.objects.last_published_envelope_id()
         if self.envelope.envelope_id[2:] == settings.HMRC_PACKAGING_SEED_ENVELOPE_ID:
-            year = int(self.envelope.envelope_id[:2])
-            last_envelope = publishing_models.Envelope.objects.for_year(
-                year=year - 1,
-            ).last()
-            # uses None if first envelope (no previous ones)
-            expected_previous_id = last_envelope.envelope_id if last_envelope else None
-        else:
-            expected_previous_id = str(
-                int(self.envelope.envelope_id) - 1,
+            # NOTE:
+            # Code in this conditional block, and therefore this function,
+            # wrongly assumes a new year has passed since the last envelope was
+            # successfully published to tariffs-api.
+            # See Jira ticket TP2000-1646 for details of the issue.
+
+            current_envelope_year = int(self.envelope.envelope_id[:2])
+            last_envelope_last_year = (
+                publishing_models.Envelope.objects.last_envelope_for_year(
+                    year=current_envelope_year - 1,
+                )
+            )
+            expected_previous_id = (
+                last_envelope_last_year.envelope_id if last_envelope_last_year else None
             )
+        else:
+            expected_previous_id = str(int(self.envelope.envelope_id) - 1)
+
         if previous_id and previous_id != expected_previous_id:
             return False
+
         return True
 
     # processing_state transition management.

publishing/tests/test_publishing_commands.py

@@ -20,11 +20,11 @@ def test_publish_to_api_lists_unpublished_envelopes(
     packaged_work_baskets = PackagedWorkBasket.objects.get_unpublished_to_api()
 
     out = StringIO()
-    call_command("publish_to_api", "--list", stdout=out)
+    call_command("publish_to_api", stdout=out)
     output = out.getvalue()
 
     for packaged_work_basket in packaged_work_baskets:
-        assert str(packaged_work_basket.envelope) in output
+        assert f"envelope_id={packaged_work_basket.envelope.envelope_id}" in output
 
 
 def test_publish_to_api_lists_no_envelopes(
@@ -34,7 +34,7 @@ def test_publish_to_api_lists_no_envelopes(
     settings.ENABLE_PACKAGING_NOTIFICATIONS = False
 
     out = StringIO()
-    call_command("publish_to_api", "--list", stdout=out)
+    call_command("publish_to_api", stdout=out)
     output = out.getvalue()
 
     assert not output
@@ -46,7 +46,7 @@ def test_publish_to_api_exits_no_unpublished_envelopes():
     assert CrownDependenciesEnvelope.objects.unpublished().count() == 0
 
     with pytest.raises(SystemExit):
-        call_command("publish_to_api")
+        call_command("publish_to_api", "--publish-async")
 
 
 def test_publish_to_api_publishes_envelopes(successful_envelope_factory, settings):
@@ -57,6 +57,6 @@ def test_publish_to_api_publishes_envelopes(successful_envelope_factory, setting
 
     assert PackagedWorkBasket.objects.get_unpublished_to_api().count() == 1
 
-    call_command("publish_to_api")
+    call_command("publish_to_api", "--publish-async")
 
     assert PackagedWorkBasket.objects.get_unpublished_to_api().count() == 0

settings/common.py

@@ -633,7 +633,7 @@
     CROWN_DEPENDENCIES_API_CRON = (
         crontab(os.environ.get("CROWN_DEPENDENCIES_API_CRON"))
         if os.environ.get("CROWN_DEPENDENCIES_API_CRON")
-        else crontab(minute="0", hour="8-18/2", day_of_week="mon-fri")
+        else crontab(minute="0", hour="*/2")
     )
     CELERY_BEAT_SCHEDULE["crown_dependencies_api_publish"] = {
         "task": "publishing.tasks.publish_to_api",