Enhancement: Make empty defaults in Config into optional props #136
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
`startTime` and `endTime` have a default/unused/unset/empty value of `null`. The Multiple Section Start/End Time setting for these mixins are optional. Currently the UI has a checkbox that when clicked reveals the forms to collect Single and/or Multiple Section info. This commit changes the constructed Optional Types to include whether or not the the forms are used/viewable. The UI form's viewability (or user disinterest in the optional config) is represented by the property being Optional (either absent or set as undefined). This stands in for the unchecked boolean nature of the checkbox. Once viewable (or user has expressed interest) then the form's default state is shown. The functionality added here is a backwards compatible way to make the `startTime` and `endTime` optional properties in the stored Token ATM Configuration. The goal is reduce the stored JSON length since undefined props are stripped when stringified. This is a useful change for possibly adding `startTime` and `endTime` configuration to the baseline Token Option Mixin. Which means they would be included as a choice in all Token Options. Adding a required `startTime: null` & `endTime: null` to every Token Option would needlessly increase the length/size of the Token ATM Configuration. The backwards compatible change adds: - acceptance of undefined/absent `startTime` or `endTime` in the Token ATM Configuration, - converts the optionals into their default `null` property for use at runtime, and - changes `null`s into `undefined` when writing them back to the Token ATM Configuration. This will strip `null`s from existing Token Option configurations when they are next saved. The stripping of `null`s will break the Token ATM Configuration's compatiblity with earlier versions of Token ATM. So while this change is backwards compatible, it is not downgrade compatible.
This was referenced Jan 11, 2025
|
Implementing automated tests showed some errors with this implementation. Converting back to draft pull request. |
Properly implements partial `startTime` and `endTime`s and provides tests to ensure backwards compatibility. Error occured because `io-ts` type's `.validate()` performs the transformation. And the `chain` function was calling the Partial's `validate` and then the original `validate`. This implementation instead checks that the type is a Partial one, handles the undefined case, and uses the original's `validate` to keep the data backwards compatible. This implementation also replaces `null` `startTime` and `endTime`s with `undefined`` which are stripped when stringified by JSON.
|
Implementation now passes further automated tests. Pull request reopened for review, and Description and Testing Note have been updated. |
|
Testing Results
|
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.
Description
startTimeandendTimehave a default/unused/unset/empty value ofnull.The Multiple Section Start/End Time setting for the changed mixins are optional. Currently the UI has a checkbox that when clicked reveals the forms to collect Single and/or Multiple Section info.
This pull request changes the constructed Optional Types to include whether or not the the forms are used/viewable.
The UI form's viewability (or user disinterest in the optional config) is represented by the property being Optional (either absent or set as undefined). This stands in for the unchecked boolean nature of the checkbox.
Once viewable (or user has expressed interest) then the form's default state is shown.
The functionality added here is a backwards compatible way to make the
startTimeandendTimeoptional properties in the stored Token ATM Configuration.The goal is to reduce the stored JSON length since undefined props are stripped when stringified. This is a useful change for possibly adding
startTimeandendTimeconfiguration to the baseline Token Option Mixin. Which means they would be included as a choice in all Token Options.Adding a required
startTime: null&endTime: nullto every Token Option would needlessly increase the length/size of the Token ATM Configuration.The backwards compatible change adds:
startTimeorendTimein the Token ATM Configuration,nullproperty for use at runtime, andnulls intoundefinedwhen writing them back to the Token ATM Configuration. This will stripnulls from existing Token Option configurations when they are next saved.The stripping of
nulls will break the Token ATM Configuration's compatibility with earlier versions of Token ATM. So while this change is backwards compatible, previous version are not forward compatible with this change.In addition to implementing optional props for
startTime&endTimethis pull request also includes, 2 enhancements:EarnBySurveyToken Option tests and automatically increase the test space of valid tests. The raw data generators create valid raw data equivalents of valid data examples. An optional boolean flag can be provided to generate valid but not equal raw data. This combination allows gradually increasing the token option data values that are tested for proper encoding/decoding, using a minimal number of valid examples to tests the breadth of possible data situations, and provides a baseline for ensuring backwards compatibility with existing Token ATM Configurations and runtimes.PlaceholderToken Option following theEarnBySurveytests as a template and using raw data generators for Placeholder Token Option Data.Testing Note
Test if the enhancement works as described. Perform all tests for both Optional Multiple Section Start Times and Optional Multiple Section End Times. Currently these 2 mixins are only used by the Placeholder Token Option.
Some Specific Baseline Tests:
Applicable property can have null value in Token ATM ConfigurationIncluded in automated testsproperty -> nullpairs from the saved Token ATM Configuration (Automated tests show this should be the case, but also should also be verified on Canvas)