Add 5 missing Config DEFAULTS entries and fix autosetup initialization (#3395)#3539
Open
Rolling2405 wants to merge 4 commits intoDARIAEngineering:mainfrom
Open
Add 5 missing Config DEFAULTS entries and fix autosetup initialization (#3395)#3539Rolling2405 wants to merge 4 commits intoDARIAEngineering:mainfrom
Rolling2405 wants to merge 4 commits intoDARIAEngineering:mainfrom
Conversation
Missing keys added to Config::DEFAULTS: - procedure_type: nil (list field, same as other list fields) - show_patient_identifier: false (toggle, off by default) - display_practical_support_attachment_url: false (toggle, off by default) - display_practical_support_waiver: false (toggle, off by default) - display_consent_to_survey: false (toggle, off by default) These 5 config_key enum entries (21-25) existed but had no DEFAULTS, causing Config methods to potentially return unexpected values for new fund setups. Also improved autosetup to populate default values from DEFAULTS hash when creating new config entries, so new funds start with correct initial configuration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix false.present? bug: boolean false defaults were being skipped Changed to 'unless default_value.nil?' so false values are processed - Translate boolean defaults: true -> 'Yes', false -> 'No' (DARIA convention) - Fix silent update failures: config.update -> config.update! - Fix N+1 queries: preload existing keys with pluck().to_set instead of running a COUNT query per config key - Add comprehensive autosetup tests: - Boolean false defaults not skipped - Boolean true defaults translated to 'Yes' - Idempotency (running twice = no duplicates) - Existing values not overridden - String defaults set correctly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The codebase consistently uses lowercase 'yes'/'no' for boolean config values. Changed autosetup to use 'yes'/'no' instead of 'Yes'/'No'. Also fixed test referencing non-existent 'shared_reset' config key. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tests for: - ALL 5 new defaults present in DEFAULTS hash (procedure_type, show_patient_identifier, etc.) - Every config_key has a corresponding DEFAULTS entry - Autosetup with nil default creates config without config_value - procedure_type default is specifically nil (no preset options) - Numeric defaults are stored as string representations - Autosetup handles empty string edge cases correctly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
I rule and have completed some work on Case Manager that's ready for review!
This adds missing default configuration entries and fixes the auto-setup method so that new funds get all the expected Config records created automatically with sensible defaults.
This pull request makes the following changes:
Config.autosetupmethod that creates missing config entries from a defaults hashtrue→"yes",false→"no") to avoid Ruby'sfalse.present?gotchaconfig.update!(bang) for fail-fast error reportingpluck(:config_key).to_setIt relates to the following issue #s:
For reviewer:
featureif it contains a feature, fix, or similar. This is anything that contains a user-facing fix in some way, such as frontend changes, alterations to backend behavior, or bug fixes.dependenciesif it contains library upgrades or similar. This is anything that upgrades any dependency, such as a Gemfile update or npm package upgrade.