Sample type color field and color configuration - #2038
Conversation
| sampleTypeName?: string, | ||
| containerPath?: string | ||
| ): Promise<number[]> { | ||
| const rowId = sampleTypeRowId ?? (await getSampleTypeRowId(sampleTypeName)); |
There was a problem hiding this comment.
since both sampleTypeRowId and sampleTypeName are optional params, should we have a quick check that one of them is provided here and error early if not?
| return new Promise((resolve, reject) => { | ||
| selectRows({ | ||
| schemaQuery: SCHEMAS.EXP_TABLES.SAMPLE_SETS, | ||
| columns: 'Name,' + fieldKey, |
There was a problem hiding this comment.
I assume this was already like this before you brought the code over from the premium package, but any idea why there isn't a filter here on the name? instead it queries for all sample types and then does the "filter" in the response. That seems inefficient.
There was a problem hiding this comment.
Maybe this has to do with case sensitivity? There might be case where sampletype name is parsed from URL, and could have a different casing?
There was a problem hiding this comment.
ah yes, that is probably why
| }, [updated, excludedTypes, initialExcludedTypes, saveColor]); | ||
|
|
||
| const onToggleArchive = useCallback(() => { | ||
| saveColor({ ...updated, archived: !updated.archived }); |
There was a problem hiding this comment.
in manual testing, you can select a color > uncheck some sample types to make them excluded > click archive. it makes it seem like that color has the exclusions but they were not saved with the saveColor here. This seems like an edge case as people are unlikely to try to set exclusions while archiving.
Rationale
Adds the components and actions for configuring sample colors.
Related Pull Requests
Changes