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

_unchecked falsey value displays on check your answers page #1215

Closed
murrlipp opened this issue Jan 11, 2022 · 9 comments · Fixed by #1333
Closed

_unchecked falsey value displays on check your answers page #1215

murrlipp opened this issue Jan 11, 2022 · 9 comments · Fixed by #1333
Assignees

Comments

@murrlipp
Copy link

Description of the issue

When viewing a 'check your answers' page or playing back any submitted checkbox values, _unchecked is sometimes displayed in the list of values. This only appears to happen when a set of answers is submitted, and then changed.

Example:
image

Steps to reproduce the issue

  1. Submit multiple answers via a checkbox list
  2. Go back and uncheck one, or several, but not all of the answers
  3. Go to a check your answers screen, or another screen that displays the values to see if _unchecked appears in the list of values

Note that this only happens intermittently so you may need to try several times before _unchecked appears.

Actual vs expected behaviour

_unchecked is being displayed, which is an internal falsey value. I would expect no value to be displayed if the value is falsey.

Environment (where applicable)

  • Operating system: macOS Catalina
  • Browser: Chrome
  • Browser version: 96.0.4664.110
  • GOV.UK Prototype Kit version: 9.4.0
@murrlipp
Copy link
Author

Tagging @edwardhorsford in as he has experienced this too.

@edwardhorsford
Copy link
Contributor

edwardhorsford commented Jan 11, 2022

I've had this a longstanding issue in one part of my prototype - where checkboxes are used in search filters doing GET requests to the page or loading the page with pre-set query strings.

The kit uses _unchecked as a special value internally - but it's meant to be stripped out so it doesn't bleed out in to the interface. I think there's some edge cases where this doesn't happen correctly.

I wasn't able to track down the cause of the issue, and as it was so intermittent, diagnosing it was hard.

My hacky / brute force solution was to write a function to 'clean' my inputs as part of my route.

@trang-erskine
Copy link

@murrlipp are you able to link to the repo so we can look into it further?

@murrlipp
Copy link
Author

murrlipp commented Feb 1, 2022

@trang-erskine the repo is here: https://github.com/CriminalInjuriesCompensationAuthority/apply-for-compensation-prototype

Note that it includes @edwardhorsford's brute force solution mentioned above.

@BenSurgisonGDS
Copy link
Contributor

I have managed to reproduce this bug with the following steps:

@BenSurgisonGDS
Copy link
Contributor

BenSurgisonGDS commented May 20, 2022

This appears to occur because the code that removes the _unchecked entry from the list of values assumes there is only one there. When the bug occurs, there are two so only one is removed. I have made a fix to remove all occurrences of _unchecked from the list. While this fixes the symptom, it doesn't fix the cause as to why there were two _unchecked entries in the first place. I'll be investigating the cause in my next step followed by a fix and some unit tests to hopefully catch any issues here in the future.

@edwardhorsford
Copy link
Contributor

@BenSurgisonGDS nice catch!

@BenSurgisonGDS
Copy link
Contributor

I've fixed this in PR: #1333

@lfdebrux
Copy link
Member

lfdebrux commented Jun 8, 2022

Hi @murrlipp, thanks for raising this issue! We're hoping a fix will go out in the next release, would you be happy for us to credit you in the release notes?

If you'd prefer to reply privately please feel free to message me using the email address on my public profile :)

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 a pull request may close this issue.

5 participants