-
Notifications
You must be signed in to change notification settings - Fork 3
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
Support Grand calculated summary as a value source #216
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
katie-gardner
changed the base branch from
main
to
grand-calculated-summary-repeating-sections
September 26, 2023 10:14
2 tasks
Base automatically changed from
grand-calculated-summary-repeating-sections
to
main
October 4, 2023 16:05
MebinAbraham
reviewed
Oct 5, 2023
petechd
reviewed
Oct 9, 2023
petechd
reviewed
Oct 9, 2023
MebinAbraham
approved these changes
Oct 9, 2023
petechd
approved these changes
Oct 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the context of this PR?
This PR adds support for using Grand Calculated Summaries as value sources. Similar to calculated summaries this is done with a value source of the form
grand_calculated_summary
has been added as a valid value source, and validation has been added to ensure that any such value source is a valid grand calculated summary id. It corrects an existing issue in which a calculated summary value source would be marked as valid if there are no calculated summaries in the schema.Some minor refactoring was done alongside supporting the GCS value source as multiple places were using the same approach to get answer ids relevant to a value sources.
Due to the need for checking this is valid within a repeating section this PR sits on top of the PR for
grand-calculated-summary-repeating-sections
It will only be merged in after the other PR.The corresponding runner PR makes use of this
Note
Made the PR for merge into main so that the test suite would run and pass and have now pointed it at GCS-repeating-sections for ease of review, will point it back to main before merge
How to review
Check that the updated schemas cover grand calculated summary value sources in repeats/outside repeats/ piping/ logic and that the updates to the schema json are correct. Ensure only valid grand calculated summary ids can be used in value sources.
Checklist