feat: add allocation time unit to instrument picker question

[joeshannon]

Description

Motivation and Context

How Has This Been Tested

Fixes

This fixes UserOfficeProject/issue-tracker#1629

Changes

Before:

After:

Depends on

Tests included/Docs Updated?

• I have added tests to cover my changes.
• All relevant doc has been updated

[zacharyjhankin]

I see this is in a try block, but if the try fails then it just errors out. Maybe we'd rather go back to fallBackConfig, if we can't get the call? or still show the instrument picker on screen, just without a allocatedTimeUnit?

[zacharyjhankin]

I am a bit confused why we actually need to store the allocationTimeUnit as a config with the question type. As one question could be used for multiple different calls, so could have multiple different allocationTimeUnits that we need to show to users.

Surely it would make more sense to generate the allocationTimeUnit that is relevant to the user purely in the frontend by calling a graphQL query and fill in the string at that point in time. Then you wouldn't need to edit the config and put the value in there.

What we are risking by putting the allocationTimeUnit in the database here, is that the value in the database is wrong. So we need to verify that it is correct at runtime when the user loads the screen. At that point we wouldn't need it in the database anyway

[joeshannon]

I am a bit confused why we actually need to store the allocationTimeUnit as a config with the question type. As one question could be used for multiple different calls, so could have multiple different allocationTimeUnits that we need to show to users.

Surely it would make more sense to generate the allocationTimeUnit that is relevant to the user purely in the frontend by calling a graphQL query and fill in the string at that point in time. Then you wouldn't need to edit the config and put the value in there.

What we are risking by putting the allocationTimeUnit in the database here, is that the value in the database is wrong. So we need to verify that it is correct at runtime when the user loads the screen. At that point we wouldn't need it in the database anyway

Note that modifications made by transformConfig do not get stored in the database:

  /**
   * Function to transform the config value before sending back to the front end.
   * Ideal for changes in config at run time.
   */
  readonly transformConfig?: (
    config: QuestionDataTypeConfigMapping<T>,
    callId?: number
  ) => Promise<QuestionDataTypeConfigMapping<T>>;

I've used the same approach as is used for the instruments in the picker where the available instruments are dynamically set based on the call.

I agree with your comment in general, although I'm not sure what an alternative solution would look like. The complication arises due to the fact that a question doesn't directly have a relation to a specific call. Once there is an answer associated (i.e. the user presses save) it is possible - this is actually already implemented for InstrumentPickerAnswerRenderer. However it's entirely possible I've missed an alternative approach!