From cc2a94060e7c0f977beca04e2d9c3a2f56ec2370 Mon Sep 17 00:00:00 2001 From: Paul Pepper <85895113+paulpepper-trade@users.noreply.github.com> Date: Wed, 8 Jan 2025 11:02:13 +0000 Subject: [PATCH] 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 --- .../management/commands/publish_to_api.py | 67 +++++++++++++------ publishing/models/packaged_workbasket.py | 38 ++++++----- publishing/tests/test_publishing_commands.py | 10 +-- settings/common.py | 2 +- 4 files changed, 73 insertions(+), 44 deletions(-) diff --git a/publishing/management/commands/publish_to_api.py b/publishing/management/commands/publish_to_api.py index 9a29a0060..02c166200 100644 --- a/publishing/management/commands/publish_to_api.py +++ b/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() diff --git a/publishing/models/packaged_workbasket.py b/publishing/models/packaged_workbasket.py index 93b240644..cc24edfa9 100644 --- a/publishing/models/packaged_workbasket.py +++ b/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. diff --git a/publishing/tests/test_publishing_commands.py b/publishing/tests/test_publishing_commands.py index 67c18a688..31d06039e 100644 --- a/publishing/tests/test_publishing_commands.py +++ b/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 diff --git a/settings/common.py b/settings/common.py index 041d45eba..614f98346 100644 --- a/settings/common.py +++ b/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",