chore: aselo-webhcat-react-app - fix unit tests [CHI-3624] #3829
Conversation
gpaoloni
changed the title
| (localeTranslations: ConfigState['translations'][keyof ConfigState['translations']]) => | ||
| (key: string, parameters: Record<string, string> = {}) => { | ||
| const lookedUpValue = localeTranslations[key] || key; | ||
| const lookedUpValue = (localeTranslations && localeTranslations[key]) || key; |
There was a problem hiding this comment.
The translations should always be present with the merged configs
There was a problem hiding this comment.
I know they should but if they don't I'm of the idea is better to have a weird default than a blank page. Also, this way makes testing easier, as we need less mocks.
| import { initLogger, getLogger } from './logger'; | ||
| import { changeExpandedStatus } from './store/actions/genericActions'; | ||
|
|
||
| const getHelplineConfig = async ({ configUrl }: { configUrl: string | URL }) => { |
There was a problem hiding this comment.
This logic seemed more concise and readable inline, especially considering all the result type bloat the separate function adds
A comment indicating that the section of code loads the config would be better than moving it out to a function. Internal functions that are only called from one place generally don't help readability - better to have it inline, commented as appropriate - though there are exceptions
gpaoloni
changed the title
Description
This PR
Checklist
Verification steps
AFTER YOU MERGE
You are responsible for ensuring the above steps are completed. If you move a ticket into QA without advising what version to test, the QA team will assume the latest tag has the changes. If it does not, the following confusion is on you! :-P