Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Holiday Stops Amend #1225

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Fix Holiday Stops Amend #1225

merged 3 commits into from
Oct 19, 2023

Conversation

andrewHEguardian
Copy link
Contributor

@andrewHEguardian andrewHEguardian commented Oct 16, 2023

What does this change?

Fix amending an existing holiday stop - this was not checking the correct value and therefore calling the api with a POST instead of a PATCH request.

Fix a secondary bug when amending a non-confirmed holiday stop. The wrong dates were being set when this button was clicked, which made the holiday stop to amend appear as an existing suspension. (See image below.) This PR removes the second Amend button.

How to test

With a newspaper delivery product, click Manage suspensions, click "Amend", pick dates, click "Confirm", see loading message "Amending...", do not see error.

How can we measure success?

Have we considered potential risks?

Images

Before
image
Bug after pressing "Amend" on review page, the existing suspension should not appear as that is what is being amended
image

After
image

Accessibility

@andrewHEguardian andrewHEguardian changed the title wip: Fix Holiday Stops Amend Fix Holiday Stops Amend Oct 17, 2023
Copy link
Contributor

@michaelbjacobson michaelbjacobson left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@andrewHEguardian andrewHEguardian merged commit 958ea74 into main Oct 19, 2023
8 checks passed
@andrewHEguardian andrewHEguardian deleted the ahe/holiday-stop-amend branch October 19, 2023 10:27
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @andrewHEguardian 13 minutes and 56 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants