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

Migrate confirm remove bill from bill run page #837

Merged
merged 18 commits into from
Mar 20, 2024

Conversation

Cruikshanks
Copy link
Member

@Cruikshanks Cruikshanks commented Mar 19, 2024

https://eaflood.atlassian.net/browse/WATER-4416

We'd recently spotted that the new billing views we'd developed had dropped a 'Remove licence' button. So, we used that opportunity to Migrate confirm remove licence from bill run page and added the button back in.

But then our conscientious QA team started testing it and found it didn't do what it claimed to do. That was to flag the removed licence for the next supplementary bill run. We display the same message on the confirm remove bill from bill run so now doubtful they tested that as well with the same result.

We've fixed the remove licence issue. But whilst we fix the legacy code for remove bill run this is a great opportunity to also migrate the confirm remove bill from bill run page. Not only is it one less thing in the legacy code but we can also update the view to be consistent with others we've done like

Screenshot 2024-03-19 at 22 40 16

https://eaflood.atlassian.net/browse/WATER-4416

We'd recently spotted that the new billing views we'd developed had dropped a 'Remove licence' button. So, we used that opportunity to [Migrate confirm remove licence from bill run page](#828) and [added the button back in](#824).

But then our conscientious QA team started testing it and found it didn't do what it claimed to do. That was flag the removed licence for the next supplementary bill run. We display the same message on the confirm remove bill from bill run so now doubtful they tested that as well with the same result.

We've [fixed the remove licence issue](DEFRA/water-abstraction-service#2464). But whilst we fix the legacy code for remove bill run this is a great opportunity to also migrate the confirm remove bill from bill run page.
@Cruikshanks Cruikshanks added the enhancement New feature or request label Mar 19, 2024
@Cruikshanks Cruikshanks self-assigned this Mar 19, 2024
@Cruikshanks
Copy link
Member Author

I'm not going to close the issues SonarCloud has found this time. It is correct on the duplication. We need to remove it but we intend to do that in a separate PR (this is related to a fix we need to get shipped!)

On the function length, we will need to tell SonarCloud to accept it. But it would be better to do that with our refactored reusable fetch services and presenters rather than in this specific case.

@Cruikshanks Cruikshanks marked this pull request as ready for review March 20, 2024 09:51
@Cruikshanks Cruikshanks merged commit 4d19ddd into main Mar 20, 2024
5 of 6 checks passed
@Cruikshanks Cruikshanks deleted the migrate-remove-bill-from-bill-run-page branch March 20, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant