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

Add support in repeating GCS for static CS with repeating answers #1268

Merged
merged 11 commits into from
Dec 15, 2023

Conversation

katie-gardner
Copy link
Contributor

@katie-gardner katie-gardner commented Nov 28, 2023

What is the context of this PR?

This PR adds support for a repeating Grand Calculated Summary to include a static Calculated Summary that has repeating answers. These can be repeating block answers or dynamic answers.

This is paired with the corresponding validator PR which disallows the use of a calculated summary which has repeating answers for the same lists. So if for example, a calculated summary included a sum of dynamic answers for vehicles. In a repeating section for vehicles that calculated summary could not then be included in a repeating grand calculated summary, as it would be unclear whether to resolve the repeating answer id to the specific vehicle instance, or the collection of answers, given it was from another section.

I had to update test_repeating_sections_with_hub_and_spoke.json to stop validator from breaking. It has the same answer id in the add block for both list collectors but this breaks under the new validation (and is a bug anyway) raised another card for a fix to that properly in validator

How to review

Look over the card to check all parts have been covered. Functionally the changes are very small, its just a change to value source resolver to support returning all the items unless in a repeat for the same list. Primarily the changes are test coverage.

How to test functionally

Use the schema test_grand_calculated_summary_inside_repeating_section

  • Ensure that the change links in the grand calculated summaries work both for repeating answers and dynamic answers
  • Make sure that changing an answer which affects the other repeating section correctly updates progress
  • Note the guidance about the validation block, there is a separate card for implementing grand calculated summary dependencies. But make sure the calculated summary dependencies are working as expected.

Checklist

  • New static content marked up for translation
  • Newly defined schema content included in eq-translations repo

@katie-gardner katie-gardner marked this pull request as ready for review November 28, 2023 14:50
Copy link
Contributor

@VirajP1002 VirajP1002 left a comment

Choose a reason for hiding this comment

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

Functionally tested and it works as intended 👍

scripts/run_validator.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@berroar berroar left a comment

Choose a reason for hiding this comment

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

Not sure if this is one of the known issues for submitting incomplete? But when you fill in the GCS inside the repeating question, and the the dependent breakdown question with the validation based on the GCS total. If you go from the Summary and change the What is your monthly expenditure on fuel for your Car? question total, you can revisit the summary without the breakdown question being revalidated (gcs-breakdown-block)?
So then you get back to the section summary with something like:
image

app/questionnaire/value_source_resolver.py Outdated Show resolved Hide resolved
@katie-gardner
Copy link
Contributor Author

Not sure if this is one of the known issues for submitting incomplete? But when you fill in the GCS inside the repeating question, and the the dependent breakdown question with the validation based on the GCS total. If you go from the Summary and change the What is your monthly expenditure on fuel for your Car? question total, you can revisit the summary without the breakdown question being revalidated (gcs-breakdown-block)? So then you get back to the section summary with something like: image

Yeah this will unfortunately be the case until my GCS dependencies card merges, anything that the GCS depends on will work as expected, but anything that depends on the GCS doesn't yet get revisited when the GCS changes, so the functional test here will need minor modification when the other merges, or whichever goes first

Copy link
Contributor

@berroar berroar left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@liamtoozer liamtoozer left a comment

Choose a reason for hiding this comment

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

Functionally tested and works great for me 👍

@katie-gardner katie-gardner merged commit 36736be into main Dec 15, 2023
16 checks passed
@katie-gardner katie-gardner deleted the gcs-with-non-repeating-cs-with-repeating-answers branch December 15, 2023 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants