Skip to content

Base GPO expiration on original request date#7389

Merged
matthinz merged 2 commits intomainfrom
matthinz/8063-gpo-expiry
Nov 25, 2022
Merged

Base GPO expiration on original request date#7389
matthinz merged 2 commits intomainfrom
matthinz/8063-gpo-expiry

Conversation

@matthinz
Copy link
Contributor

🎫 Ticket

LG-8063

🛠 Summary of changes

The expiration date printed on GPO letters was being calculated as 30 days after the GPO upload job was run rather than 30 days after the original request was made.

This PR updates the GPO exporter to base expiration dates on the created_at field of the GpoConfirmation record.

Rather than basing on the date we FTP the .csv file out, base the expiry on the time of the original request's creation.

For LG-8063

changelog: Bug Fixes, GPO, Address issue where expiration date on GPO letters was not printed correctly.
@matthinz matthinz requested a review from a team November 23, 2022 21:21
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matthinz matthinz merged commit 9657a83 into main Nov 25, 2022
@matthinz matthinz deleted the matthinz/8063-gpo-expiry branch November 25, 2022 18:53
mdiarra3 pushed a commit that referenced this pull request Nov 28, 2022
* Base GPO expiration on original request date

Rather than basing on the date we FTP the .csv file out, base the expiry on the time of the original request's creation.

For LG-8063

changelog: Bug Fixes, GPO, Address issue where expiration date on GPO letters was not printed correctly.

* Switch to Time.zone.parse
matthinz added a commit that referenced this pull request Nov 28, 2022
Address a flaky test introduced in #7389.

We run tests in CI with a random Timezone each time. This means sometimes the local day and UTC day are different, which was making the GPO exporter test fail.

This change updates `GpoConfirmationExporter` to convert dates to UTC before outputting them. This gives us consistency on tests but should not matter in prod, where the Timezone is already set to UTC.

For LG-8233

[skip changelog]
@jmdembe jmdembe mentioned this pull request Nov 29, 2022
matthinz added a commit that referenced this pull request Nov 29, 2022
* Fix flaky GPO exporter test

Address a flaky test introduced in #7389.

We run tests in CI with a random Timezone each time. This means sometimes the local day and UTC day are different, which was making the GPO exporter test fail.

This change updates `GpoConfirmationExporter` to convert dates to UTC before outputting them. This gives us consistency on tests but should not matter in prod, where the Timezone is already set to UTC.

For LG-8233

[skip changelog]

* Update app/services/gpo_confirmation_exporter.rb

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants