feat: add data use restriction column to HCA Data Explorer (#4136) #4294
Conversation
jpaten
requested review from
frano-m
and removed request for
MillenniumFalconMechanic
app/common/types.ts
Outdated
There was a problem hiding this comment.
@jpaten I think we should revert this change and update the type in site-config/hca-dcp/cc-ma-dev/index/common/constants.ts to:
export const COLUMN: PickSome<
Record<keyof typeof HCA_DCP_CATEGORY_KEY, ColumnConfig>,
"ACCESSIBLE" | "DATA_USE_RESTRICTION"
> = {}
There was a problem hiding this comment.
Change to:
export const COLUMN: PickSome<
Record<keyof typeof HCA_DCP_CATEGORY_KEY, ColumnConfig>,
"ACCESSIBLE" | "DATA_USE_RESTRICTION"
> = {}
| // Clone columns. | ||
| const cloneColumns = [...cloneList.columns]; | ||
| // Add data use restriction column. | ||
| cloneColumns.splice(2, 0, COLUMN.DATA_USE_RESTRICTION); |
There was a problem hiding this comment.
The shape returned for files's project is different to projects's project response. e.g.
dataUseRestriction: [null]
- I believe you will need to add to AggregatedProjectResponse
dataUseRestriction: (null | string)[]. - On line 19, remove the reference
COLUMN.DATA_USE_RESTRICTIONbecause this isprojectsresponse related. I think for now let's just insert the new config like this:
cloneColumns.splice(2, 0, {
componentConfig: {
component: C.BasicCell,
viewBuilder: V.buildAggregatedDataUseRestriction,
} as ComponentConfig<typeof C.BasicCell>,
header: HCA_DCP_CATEGORY_LABEL.DATA_USE_RESTRICTION,
id: HCA_DCP_CATEGORY_KEY.DATA_USE_RESTRICTION,
width: { max: "1fr", min: "146px" },
});
The viewBuilder V.buildDataUseRestrictions should probably look something like this:
export const buildAggregatedDataUseRestriction = (
filesResponse: FilesResponse
): React.ComponentProps<typeof C.BasicCell> => {
return {
value: processEntityArrayValue(
filesResponse.projects,
"dataUseRestriction"
),
};
};
Confirm on the files tab that you can see in the Data Use Restriction column, data such as "NRES" or "Unspecified".
| return getMAProjectsEntityConfig(entity); | ||
| } | ||
| if (entity.route === "files") { | ||
| return getMAFilesEntityConfig(entity); | ||
| } | ||
| return getMAProjectsEntityConfig(entity); |
There was a problem hiding this comment.
I believe the default returned value, will always be projects related entity config. This means on the samples tab the column config is now projects related.
I think the default return value on line 43 should be just entity.
There was a problem hiding this comment.
Thanks! Not sure how I missed that one
app/apis/azul/common/utils.ts
Outdated
There was a problem hiding this comment.
Let's leave line 144 as is:
return processNullElements(values, label) ?? [label];
However, I think the ?? [label] is probably no longer required?
There was a problem hiding this comment.
Yeah that confused me - it was like this before, and I don't think it ever was required?
| */ | ||
| export const buildAggregatedDataUseRestriction = ( | ||
| filesResponse: FilesResponse | ||
| ): React.ComponentProps<typeof C.BasicCell> => { |
There was a problem hiding this comment.
Sorry @jpaten - I might have given you incorrect suggestion for the buildAggregatedDataUseRestriction. I wonder if we should make this an NTagCell and return all possible values from the filesResponse?
If so, update:
- lines 192 and 194 with
NTagCellin the comments. - line 198 with
C.NTagCell - the return values to be an array of values.
| component: C.BasicCell, | ||
| viewBuilder: V.buildAggregatedDataUseRestriction, | ||
| } as ComponentConfig<typeof C.BasicCell>, |
There was a problem hiding this comment.
@jpaten, if we should be displaying all possible dataUseRestriction values on the filesResponse, let's change the component on lines 27 and 29 to C.NTagCell.
There was a problem hiding this comment.
Sorry @jpaten I may have given you a bit of misdirection - can you confirm with @MillenniumFalconMechanic or @NoopDog as to whether we should show all possible dataUseRestriction values on the filesResponse? If so, update as per my comments. Thank you again!
|
Hi @frano-m! I've updated the Data Use Restriction cell on Files to use an
|
Resolved! |
jpaten
force-pushed
the
jonah/4136-data-use-restriction
branch
from
800945a to
ca69bc7
Compare
NoopDog
changed the title
NoopDog
changed the title
NoopDog
changed the title
Ticket
Closes #4136 .
Reviewers
@MillenniumFalconMechanic .
Changes
Notes
Note that the projects.dataUseRestriction endpoint contains only null values on the dev backend, so in order to see any values for this column it is necessary to use the prod site