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 bill run type in view licence bills tab #1159

Merged
merged 11 commits into from
Jul 3, 2024

Conversation

Cruikshanks
Copy link
Member

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

Part of the work to replace the legacy view licence page

While working on Switch to GOV.UK table component for licence bills we spotted that a Nunjucks filter was being used to title-case the bill run type. How the bill run type is displayed is not as simple as just title-casing what the DB returns. In fact, because there is some logic involved and we have had to use it in a number of places, we've added it to the base presenter as a reusable formatter.

This change corrects the view licence bills presenter to use the existing bill run type formatter.

@Cruikshanks Cruikshanks added the bug Something isn't working label Jul 2, 2024
@Cruikshanks Cruikshanks self-assigned this Jul 2, 2024
https://eaflood.atlassian.net/browse/WATER-4316

> Part of the work to replace the legacy view licence page

Whilst working on [Switch to GOV.UK table component for licence bills](#1158) we spotted that a Nunjucks filter was being used to title-case the bill run type. How the bill run type is displayed is not as simple as just title-casing what the DB returns. In fact, because there is some logic involved and we have had to use it in a number of places we've added it to the base presenter as a reusable formatter.

This change corrects the view licence bills presenter to use the existing bill run type formatter.
We also need the scheme and whether the bill run was flagged as summer or not in order to correctly determine the bill run type.
We started with the aim of just updating the presenter to use the existing bill run type formatter. But we then spotted that the unit tests didn't follow our standard pattern of setting up test conditions in the `beforeEach()` (it was doing it in the `it`)

It was also testing things that are using existing formatters that have their own tests which means there is no need to repeat them here.

Finally, we felt we could simplify things if we renamed things to just be as they are (which meant some tweaks to the view as well). We also dropped `legacyId` from the presenter result because nothing in the view is using it.
@Cruikshanks Cruikshanks force-pushed the fix-bill-type-in-view-licence branch from 4712c8d to 24d632d Compare July 2, 2024 23:00
@Cruikshanks Cruikshanks marked this pull request as ready for review July 3, 2024 06:42
@Cruikshanks Cruikshanks merged commit 47aa8a5 into main Jul 3, 2024
6 checks passed
@Cruikshanks Cruikshanks deleted the fix-bill-type-in-view-licence branch July 3, 2024 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant