-
Notifications
You must be signed in to change notification settings - Fork 14
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
Looping 2.5 | Repeating Blocks for List Items #1106
Merged
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
…ector Question handling many add types by enum.
…r are not posted correctly and navigation not tested.
…block go to repeating_blocks if existing.
…ow on list collector summary.
…_blocks shown on list collector summary underneath the relevant list item.
…is upon block GET.
…ress Tracking) (#1100) * Schema for repeating blocks and section summary incomplete + ListCollector Question handling many add types by enum. * Repeating blocks are presented to the user and can be answered however are not posted correctly and navigation not tested. * List Collector now always enters add_block & both add_block and edit_block go to repeating_blocks if existing. * make format * edit_block does not lead to repeating_blocks. All repeating_blocks show on list collector summary. * Blocks in repeating_blocks calculated upfront. Answers from repeating_blocks shown on list collector summary underneath the relevant list item. * Reverting list_collector handler and fixing invalid use of set * Return to summary reused throughout ListActions
# Conflicts: # app/questionnaire/questionnaire_store_updater.py # app/views/handlers/list_add_question.py # app/views/handlers/list_repeating_block.py
…stionnaire store updater.
… the list items list of completed locations
…ched property for repeating block ids.
…ing blocks to evaluate completeness.
…of lists, renamed method for removing list items.
petechd
reviewed
Jun 21, 2023
berroar
approved these changes
Jun 22, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retested functionally - LGTM! 🚀
petechd
reviewed
Jun 22, 2023
petechd
reviewed
Jun 22, 2023
petechd
reviewed
Jun 22, 2023
petechd
reviewed
Jun 22, 2023
petechd
reviewed
Jun 22, 2023
petechd
reviewed
Jun 22, 2023
petechd
reviewed
Jun 23, 2023
petechd
reviewed
Jun 23, 2023
petechd
reviewed
Jun 23, 2023
…adding context to list collector RP schema.
petechd
approved these changes
Jun 26, 2023
Yuyuutsu
approved these changes
Jun 26, 2023
2 tasks
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 implements the feature Looping 2.5.
It is required for future surveys that items added via a list collector can have additional information provided for them as they are added on following pages after addition to the list.
To support this, the list collector has been augmented to support repeating blocks where are a list of blocks shown after every add block, the answers from which are stored in the same way answers from repeating sections would be stored.
Repeating blocks are mandatory and thus will be redirected to from the list collector pages if the suer attempts to proceed from the list collector block without having completed all repeating blocks for the added list items.
Changes
app/data_models/progress_store.py: Misc type hinting
app/jinja_filters.py:
map_summary_item_config
now no longer takes an icon string to pass tomap_list_collector_config
, which also now does not take this parameter. Instead this method takes a bool flag indicating whether list item icons should render. The icon that renders is the green checkmark icon if the a list item has repeating blocks and is complete, else no icon should render.Additional type hinting.
app/questionnaire/questionnaire_schema.py: Added
ListRepeatingBlock
toLIST_COLLECTOR_CHILDREN
.Added a new private class attribute to track repeating blocks by their block id in a dictionary.
Repeating blocks are added as entries to the the
self._blocks_by_id
attribute as well as the new attribute whenself._blocks_by_id
is populated at construction. Parent id maps are populated as a normal block.Redundant type ignore removed from
_update_answer_dependencies_for_value_source
._block_for_answer
returns the repeating block id for an answer id from a repeating block instead of the list collector block which is the case for the other other list collector child blocks.Removal of redundant type ignore.
Added public class method
is_block_in_repeating_blocks
to return true if the given block id corresponds to a repeating block, else false.app/questionnaire/questionnaire_store_updater.py: Public method
remove_list_item_and_answers
has been renamed toremove_list_item
as it now removes list item progress from the progress store. No code was needed to add this, as the list item progress is keyed in the same way as the repeating section progress and is thus removed the same way.Public methods
get_relationship_answers_for_list_name
andget_chronological_section_dependents
are now private as they are not used publicly and prevents the mypy/pylint error of too many public methods.Kwargs enforced on public method
update_section_status
.Added a public method
is_section_complete
which simply calls and returns the result ofProgressStore.is_section_complete
, allowing services which do not have access to the ProgressStore to check section progress.app/views/contexts/list_context.py: Misc type hinting throughout.
Private method
_build_list_items_context
now takessection_id
and a bool flaghas_repeating_blocks
which is used to build the list item context.app/views/contexts/summary/list_collector_block.py: Public method
list_summary_element
passes the repeating_blocks (if any) the private method_get_related_answers
which now expects this. This allows the repeating blocks to have renderabel Blocks generated. This method also supplies the new params toListContext._build_list_items_context
.Public method
get_blocks_for_related_answers
has been made private.app/views/handlers/block_factory.py: New block type
ListRepeatingBlock
has been added.app/views/handlers/list_action.py: New private method
_is_returning_to_section_summary
has been added to remove duplication from other methods.app/views/handlers/list_add_question.py: Added class attribute
_list_item_id
so that computing the next location url after posting, has context of the list item.Public method
get_next_location_url
now navigates to the first repeating block if repeating blocks exist, else goes to the parent location as normal.Public method
handle_post
now assigns the class attribute_list_item_id
once the list item has been added.app/views/handlers/list_collector.py: Added a new cached property
repeating_block_ids
which returns repeating block ids if repeating blocks exist.Added a new cached property
list_name
which returns the list name.Public method
get_next_location_url
now navigates to any incomplete repeating blocks.Public method
get_context
pass section id andhas_repeating_blocks
bool flag to the list context constructor.Private method
_is_list_collector_complete
which makes use of the new parent methodget_first_incomplete_repeating_block_location
to return if the list collector this instance represents, is complete.app/views/handlers/list_remove_question.py: Makes use of newly renamed
QuestionnaireStoreUpdater. remove_list_item_data
.app/views/handlers/list_repeating_block.py: New block handler for Repeating Blocks.
This class extends ListAction as it can make use of the
get_next_location_url
method in the parent.get_next_location_url
is overridden to return to section summary if the url specifies that it should. It will also navigate to the next repeating block by reusing theget_first_incomplete_repeating_block_location_for_list_item
method from the parent. If it is the last incomplete repeating block, it navigates via the parent's overridden method.app/views/handlers/question.py: Public method
get_list_summary_context
now populates the new LisContext constructor parameters.Public method
capture_dependent_sections_for_list
will not add list item progresses as dependants (which they should not be). This is a workaround to allow us to reuse the ProgressStore for tracking list item progresses.New public method
get_first_incomplete_repeating_block_location_for_list_item
added which will check all repeating blocks for a given list item and return the location of the first incomplete one. returning None before iterating over the repeating blocks if the list item is already marked complete in the progress store.New public method
get_first_incomplete_repeating_block_location
added which will return the location of the first incomplete repeating block from a list model specified by section, list name and repeating blocks. This method iterates through all list items in the model and calls the former method on each, returning early if an incomplete location is found.app/views/handlers/relationships/unrelated_question.py: Public method
get_list_summary_context
supplies the new ListContext constructor parameters.schemas/test/en/test_list_collector_repeating_blocks.json: A new test schema which contains only a list collector with repeating blocks and no driving question, section summary or placeholders.
schemas/test/en/test_list_collector_repeating_blocks_section_summary.json: A new test schema which contains a list collector with repeating blocks that does have a driving question, section summary and placeholders in the repeating blocks.
tests/app/conftest.py: Moved the location fixture here to be used by the questionnaire tests as well as contexts tests.
templates/listrepeatingblock.html: A new jinja template which is used to render the repeating block which simply extends the list-action.html template.
templates/partials/summary/collapsible-summary.html: Removed passing icon to
map_summary_item_config
.templates/partials/summary/list-summary.html: Using kwargs when calling
map_list_collector_config
and passing the newrender_icon
bool flag.templates/partials/summary/summary.html: Removed passing icon to
map_summary_item_config
.tests/app/conftest.py: Moved the location fixture to here from the tests/app/questionnaire level to be used by the questionnaire tests as well as contexts tests.
tests/app/questionnaire/conftest.py: Moved the location fixture from here to the tests/app level to be used by the questionnaire tests as well as contexts tests.
tests/app/questionnaire/test_questionnaire_schema.py: Test coverage for getting repeating blocks and answers by id and adding them to the recorded blocks in QuestionnaireSchema.
tests/app/questionnaire/test_questionnaire_store_updater.py: Ammending to call a public, now private method, in existing tests.
Additional formatting.
test_jinja_filters: Amending tests which expected an icon string to now expect the render icon bool.
tests/app/views/contexts/conftest.py: Adding fixtures for an AnswerStore and ListStore with repeating blocks list collector.
test_list_context: Many tests now have a
ProgressStore
fixture passed to them to be used to construct the testedListContext
, additionally they have the location fixture passed in as well to support the new constructor param.Some assertions now also assert that the
"is_complete"
and"repeating_blocks"
bool flags are also present in the built list summaries/contexts.Also added test coverage to check ListContexts build correctly with complete and incomplete list collectors with and without repeating_blocks.
test_section_summary:
Many tests now have a
ProgressStore
fixture passed to them to be used to construct the testedListContext
.Some assertions now also assert that the
"is_complete"
and"repeating_blocks"
bool flags are also present in the built list summaries/contexts.tests/functional/generate_pages.py:
process_block
now calls itself on repeating blocks if they exist.tests/functional/helpers.js: Renamed and modified method
checkPeopleInList
tocheckItemsInList
to be more generic and use with repeating blocks func tests.tests/functional/spec/features/repeating_blocks/list_collector_repeating_blocks.spec.js: Added functional tests to cover the feature scenarios.
tests/integration/integration_test_case.py: Added
getLinksByAttribute
helper method to find links by a given dict of attributes.tests/integration/questionnaire/test_questionnaire_list_collector_repeating_blocks.py: Added integration tests to cover the feature scenarios and supplement coverage.
How to review
Familiarise yourself with the Looping 2.5 card and scenarios linked above. Make use of the test schema
test_list_collector_repeating_blocks_section_summary.json
to manually verify and attempt to break the scenarios and requirements.Try out other test schemas that make use of List Collectors and check that other features work as expected.
All new and existing unit, integration and functional tests should be passing locally as well as on PR.