Switch Settings.dig to dot notation

[stevenjcumming]

Summary

• Most settings are accessed via dot notation (Settings.parent_key.child_key)
• Using .dig can obscure the use of those keys by making them difficult to find
• The dot notation makes them easier to find
• If dot notation isn't possible, dig is acceptable

Related issue(s)

• n/a

Testing done

• n/a

Acceptance criteria

• Settings.dig is only used where dot notation isn't possible or difficult

[rmtolmach]

Wow, I just came across this today in #18887 so I asked the PR author to change it up 🎉

[jerekshoe]

Maybe this is a silly question, but isn’t the use of dig in most of these to avoid errors if any of the intermediate values is nil?

[shaunburdick]

Maybe this is a silly question, but isn’t the use of dig in most of these to avoid errors if any of the intermediate values is nil?

I agree with @jerekshoe on this. If we want to mandate/restrict the use of .dig() perhaps put out a message and/or a ticket for each team so we can validate if the .dig is needed or not.

[jerekshoe]

Maybe this is a silly question, but isn’t the use of dig in most of these to avoid errors if any of the intermediate values is nil?

I agree with @jerekshoe on this. If we want to mandate/restrict the use of .dig() perhaps put out a message and/or a ticket for each team so we can validate if the .dig is needed or not.

After thinking about this more I’m assuming it should be safe to use dot notation for these because they are updating usages of dig on the Settings hash, so if any of the intermediate values are nil there are probably bigger issues. I wasn’t thinking about this being scoped to the Settings hash when I asked my initial question

[stevenjcumming]

@jerekshoe @shaunburdick My opinion is the default in the context of Settings should be dot notation, and if that's not possible dig is fine. If a parent (intermediate) key might be nil there's two ways to go about it.

1. safe navigation with &
2. dig

I think either are acceptable alternatives. Based on what I saw in the helm charts those values can't be nil and the settings that don't exist like modules_appeals_api.enabled_unsafe_mode return nil anyways

[shaunburdick]

LGTM based on @stevenjcumming commment! 👍

[rmtolmach]

Overall I like this! I found a few issues with some of the settings though.

[rmtolmach]

@department-of-veterans-affairs/lighthouse-banana-peels This setting doesn't actually exist anywhere... It's not in settings.yml or in any of the values.yaml in the manifest repo.

[stevenjcumming]

@rmtolmach I've add

pension_ipf_vanotify_status_callback and nod_vanotify_status_callback bearer_tokens to settings.yml