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

CLDC-3344: Sales CSV download ecstat2 child label bug fix #2801

Merged

Conversation

Dinssa
Copy link
Contributor

@Dinssa Dinssa commented Nov 20, 2024

We had a bug where instead of the user friendly label to explain what is meant when ecstat2 = 9 the value itself was returned in CSV downloads, unlike how it should appear (look to ecstat3).
Screenshot 2024-11-20 at 18 29 12

This bug only appears on person 2's working situation question (ecstat2) and only for when the value is 9 as the matching key-value pair in the answer options was missing. But was present on the generic working situation question for all people from person3, person4....onwards.

Result:
Screenshot 2024-11-20 at 18 32 20

Comment on lines 160 to 166
(start_index..6).each do |idx|
if age_under_16?(idx)
self["ecstat#{idx}"] = 9
elsif self["ecstat#{idx}"] == 9
self["ecstat#{idx}"] = nil
end
end
Copy link
Contributor Author

@Dinssa Dinssa Nov 20, 2024

Choose a reason for hiding this comment

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

I've added this so a user can change the age of a person that was first set as under 16 to another age above 16. At the moment should they want to do that they would get an error:
Answer cannot be over 16 as person’s 2 working situation is ‘child under 16‘.

As the working situation question for a child is not one they can access they'd get stuck. Now should they change the age e.g. to 17, the next question will be to choose the working situation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add some tests for this to make sure it doesn't clear it when we don't want it to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, added

Copy link

"9" => {
"value" => "Child under 16",
"depends_on" => [
{ "saledate" => { "operator" => "<", "operand" => Time.zone.local(2024, 4, 1) } },
Copy link
Collaborator

Choose a reason for hiding this comment

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

If form.start_year_2025_or_later? the saledate can't be before 2024?

Maybe it's worth having answer_options and displayed_answer_options as we do on some other questions?

Comment on lines 30 to 36
"6" => { "value" => "Not seeking work" },
"7" => { "value" => "Full-time student" },
"8" => { "value" => "Unable to work due to long term sick or disability" },
"9" => { "value" => "Child under 16" },
"0" => { "value" => "Other" },
"10" => { "value" => "Buyer prefers not to say" },
}.freeze
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are not displaying the key “9” in 2024 or beyond, I added the value but omitted the depends_on conditions found on the generic person working situation page's answer options.

@Dinssa Dinssa added this pull request to the merge queue Dec 9, 2024
Merged via the queue into main with commit 474ed4f Dec 9, 2024
15 checks passed
@Dinssa Dinssa deleted the CLDC-3344-Sales-csv-download-label-bug-for-persion-2-ecstat-child branch December 9, 2024 15:55
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 this pull request may close these issues.

2 participants