Templates use lower filter on values where it should not

[tim-s-ccs]

Describe the bug Within some of the templates the lower filter is used where it should not be. The affect components are:

• Exit this page
• Select

In the "Exist this page" component, the lower filter is being applied to the attributes tag:

{{ attribute }}="{{ value | lower }}"{% endfor %}

As the user passes in the attributes we should not mutate what they are otherwise there could be unexpected consequences. I realises this was required to pass a test but perhaps the test needs to be updated (can be extended as part of my work in https://github.com/LandRegistry/govuk-frontend-jinja/pull/81)

In the "Select" component, the lower filter is being applied to the value:

<option {%- if item.value is not undefined %} value="{{ item.value | lower }}"{% endif %}

Again, as the user passes in the values we should not mutate them are otherwise there could be unexpected consequences (which is how I discovered this bug).

Expected behavior

• The "Exit this page" component should not lower the attributes passed by the user
• The "Subject" component should not lower the values passed by the user

Additional context I'm happy to help and contribute to this project so if you want to make changes I can

[masuk-kazi98]

Any updates on this? We also require the removal of 'lower' from the options tag as it affects how we process the user selections for certain data sets

[samuelhwilliams]

Is there any chance of a fix for/discussion around this @matthew-shaw? This definitely doesn't feel like a decision this template should be making?

[tim-s-ccs]

If this a is a problem for you, you can use the fork I made for our org which does not have this issues https://github.com/Crown-Commercial-Service/ccs-govuk-frontend-jinja

The python package is also on PyPI under ccs-govuk-frontend-jinja

[samuelhwilliams]

Thanks for sharing - will take a look at that and may end up switching over, but obviously it'd be great (presumably for both of us!) if we could get this one updated and not split the userbase.

Tiny one just from glancing at your repo, the first line of the readme says it hasn't been updated beyond GOV.UK Frontend v5.1.0. That's clearly not true based on commits and the rest of the readme, so might just be a tiny one to fix up 👍

[matthew-shaw]

Apologies all for leaving this one so long, simply slipped my mind I'm afraid. I'll try and prioritise this ASAP. I'd rather not split the packages and users between versions due to a bug, so always welcome contributions to help maintain this package when I'm occupied with my day job 👍🏻

[tim-s-ccs]

The issue is a combination of how this project is tested (using govuk-frontend-diff) and because the boolean values in python are capitalised (True and False).

For these particular templates govuk-frontend-diff converts the true params from the JSON fixture into True when passing it into the Jinja template. That's obviously fine but it means when the comparison is done that the HTML is not the same and so the test fails.

The way I addressed this in my Fork was for some areas where there is a slight difference between the HTML in the fixture and the HTML generated by Jinja was to transform the result before comparison: https://github.com/Crown-Commercial-Service/ccs-govuk-frontend-jinja/blob/main/tests/test_components.py#L89

It's not a perfect solution I guess but it works and I've not had any problems with it. So I guess the "fix" really needs to be in govuk-frontend-diff or to look at https://github.com/LandRegistry/govuk-frontend-jinja/issues/76.

[matthew-shaw]

See above commit that hopefully resolves this for boolean type string values only. Will merge in #101 if acceptable.

[samuelhwilliams]

Looks good to me - thanks for sorting that 🙇

[matthew-shaw]

3.8.0 released: https://pypi.org/project/govuk-frontend-jinja/3.8.0/