‐Resolves #4500 autocomplete minimal…

Resolves #4500

This documents a revision of #4712 now closed.

What has been done to verify that this works as intended?

Tested with new SelectOneResetTest.

Why is this the best possible solution? Were any other approaches considered?

See 'Design notes' below.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

More logical behaviour from user perspective. No apparent regression risks.

Do we need any specific form for testing your changes? If so, please attach one.

selectOneReset.xlsx

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Perhaps form design advice could specify the benefits and limitations of the improved behaviour?

Before submitting this PR, please make sure you have:

• [x ] run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
• [x ] verified that any code or assets from external sources are properly credited in comments and/or in the about file.
• [x ] verified that any new UI elements use theme colors. UI Components Style guidelines

Design notes

A lot of stuff here, as both the issue and the proposed resolution have turned out to be quite complex - though the resolution has been progressively simplified in revisions of this PR.

The issue

A shout out to @aurdipas for a form that demonstrates clearly the undesired behaviour of selects with fast external itemsets.

• Most obviously, answers for cascades using fast external itemsets don't reset (or at best not immediately) when preceding members are updated, in constrast to their behaviour with internal itemsets.

• This behaviour is most glaringly apparent with minimal appearances, especially in field lists - plain select widgets do reset when they can’t match an existing value in their current choices.

Desirable if less obvious behaviours also implied by the form are that

• Members should reset even when unrelated fields are interpolated in the cascade.
• Members hidden by an interpolated select when non-relevant should be reset once unhidden.

And all this should work inside groups and especially field lists; the hierarchy views should be reset immediately upon each change.

Not only may the current behaviour confuse the user, it permits saving of incoherent data.

So a resolution to this issue must focus on resetting the answers for a complete cascade, rather than just ensuring that widgets reset when rendered.

The improvement

Preparatory changes

Unchanged as of this revision

In SelectOneWidget, clearFollowingItemsetWidgets sort of resets succeding cascade members, but hitherto there’s been no equivalent in SelectOneMinimalWidget.

So to start with I recycled clearFollowingItemsetWidgets:

• Pulled it up to ItemsWidget
• Called it from SelectOneMinimalWidget by analogy with SelectOneWidget

Widget sequences (one per view)

Updated in this revision

SelectOneWidgetUtils now has checkFastExternalCascade, called from a stub clearFollowingItemsetWidgets and itself calling the private doCascadeCheck.

This last method extends the algorithm from the current clearFollowingItemsetWidgets by

• detecting fast external cascade members from their query strings
• quitting at a non-question or on encountering a new cascade
• skipping interpolated questions - including unrelated selects as in the issue form

Like the code it derives from, it has the helpful side-effect of resetting questions that are about to be hidden by an interpolated select.

Field lists

Updated in this revision

The new checkFastExternalCascade aborts silently when the cascade is in a field list as in the issue form; in the current FormEntryActivity, updateFieldListQuestions resets only the next cascade member (possibly as a side effect?).

In this improvement FormEntryActivity calls checkFastExternalCascadeInFieldList, also in SelectOneWidgetUtils and also (as of this revision) calling doCascadeCheck; of which the side-effect in a field list is that hidden questions are reset when unhidden.

Testing

Updated for this revision

The issue appears resolved as outlined above when changes are tested manually against the issue form (when slightly modified with no cascade splitting between groups or nested field list).

For formal testing there are a number of use cases to consider, representing combinations of:

• 2 types of itemset: internal (as a baseline and to validate the testing code) and fast external
• 4 appearances: plain, minimal, minimal autocomplete and (not used in practice?) plain autocomplete
• 2 contexts: widget sequences and field lists

Test form

The form derives from selectOneExternal.zip from #4258 - thanks @mmarciniak90. It covers

• four cascade members - seems the maximum in practice
• interpolated fields - actually relevance selects before last members as in the issue form
• use cases broken up into
  • sections covering each combination of itemset and appearance
  • each section split into blocks for each context

The workbook includes these supporting sheets:

• The variants sheet defines section numbers in terms of the two itemset types and the two essentially different appearances.
• The asserts sheet records results of testing both before and after the update.
• The master sheet defines two functionally identical context blocks:
  • A - widget sequence (one per view)
  • B - field-list
• It includes two alternative type and four appearance columns, and prefills defaults ready for testing of reset behaviour.
• These are propagated in the survey sheet into sections testing the various combinations of appearances and itemsets.
• For this draft PR the form implements just the four core variants:
  • 0 and 1 - internal itemsets with plain and minimal appearances
  • 2 and 3 - fast external itemsets

Test class

SelectOneResetTest borrows extensively from FieldListUpdateTest. In each block it makes changes and asserts expected outcomes as set out in the workbook asserts sheet.

• The SectionVariant enum combines an itemset based on the type of its section with an appearance from the appearance.

• The Block enum provides the appropriate strings for each SectionVariant.

The single test method iterates through the variants, thus visiting all sections of the prefilled form and running logically equivalent tests on the context blocks comprising each section.

Tests are identified in the code as detailed in the workbook asserts sheet. The test code makes allowance for the failure of standard selects to reset on hide/unhide (asserts of type (A|B)1(e|h)).

The code makes use of new convenience methods in FormEntryPage that wrap useful formulations found in FieldListUpdateTest.

SelectOneResetTest can be validated by running against the current codebase and checking that it fails where expected. Its progress can be monitored by filtering the log on 'SelectOne' which also picks up the traces from SelectOneWidgetUtils.