Use new webapi to store concept sets#2560
Open
khairul-syazwan wants to merge 14 commits into
Open
Conversation
… khairul-syazwan/analyze-2440
khairul-syazwan
requested review from
LSriragavan,
brandantck,
jerome-ng,
maggie-li-yd and
p-hoffmann
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates concept set persistence in the d2e-webapi facade to use OHDSI WebAPI concept set endpoints, while keeping legacy terminology-svc concept sets readable but effectively read-only in the UI.
Changes:
- Added a WebAPI-backed concept set API client and updated the concept set service to merge legacy + WebAPI concept sets using an encoded ID namespace.
- Updated UI concept set/terminology flows to respect
hasWriteAccess(read-only legacy) and to display “Legacy/WebAPI” badges plus a legacy read-only banner. - Added unit tests for the new facade ID encoding/mapping and UI action rendering.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/ui/apps/concept-sets/tsconfig.json | Limits main TS include set (vite config handled via node tsconfig reference). |
| plugins/ui/apps/concept-sets/src/webcomponents/registerWebComponents.ts | Adjusts JSX intrinsic element typing via React module augmentation. |
| plugins/ui/apps/concept-sets/src/types/terminology.ts | Adds source field to concept set types. |
| plugins/ui/apps/concept-sets/src/Terminology/utils/types.ts | Extends concept set types with access flags and source. |
| plugins/ui/apps/concept-sets/src/Terminology/utils/d2eWebapiMappers.ts | Maps access flags and source from facade responses. |
| plugins/ui/apps/concept-sets/src/Terminology/Terminology.tsx | Enforces read-only behavior for legacy sets and surfaces legacy warning. |
| plugins/ui/apps/concept-sets/src/context/state/translation-state.ts | Adds i18n strings for Legacy/WebAPI labels/tooltips and legacy read-only text. |
| plugins/ui/apps/concept-sets/src/ConceptSets/ConceptSetsTable.tsx | Uses hasWriteAccess for actions; adds Legacy/WebAPI chips. |
| plugins/ui/apps/concept-sets/src/ConceptSets/ConceptSetsTable.test.tsx | Adds tests for read-only vs writable actions and chips. |
| plugins/ui/apps/concept-sets/src/ConceptSets/ConceptSets.tsx | Stops passing userName to table (access now based on flags). |
| plugins/ui/apps/concept-sets/src/ConceptSets/ConceptSetDeleteDialog.tsx | Removes unused React default import. |
| plugins/ui/apps/concept-sets/src/axios/request.ts | Tightens AbortSignal typing for request dedup wrapper. |
| plugins/ui/apps/concept-sets/src/axios/public-webapi-proxy.ts | Marks unused pagination params with underscores. |
| plugins/functions/d2e-webapi/src/services/conceptset.service.ts | Switches CRUD to WebAPI, merges lists, encodes WebAPI IDs, legacy becomes read-only. |
| plugins/functions/d2e-webapi/src/services/conceptset.service.test.ts | Adds tests for encoding/mapping/read-only assertions. |
| plugins/functions/d2e-webapi/src/routes/conceptset.ts | Returns 403 for legacy mutation attempts with a structured error. |
| plugins/functions/d2e-webapi/src/errors/ConceptSetErrors.ts | Adds LegacyConceptSetReadOnlyError. |
| plugins/functions/d2e-webapi/src/dto/conceptset.ts | Adds required source to concept set response DTO. |
| plugins/functions/d2e-webapi/src/api/WebApiConceptSetAPI.ts | New fetch-based WebAPI concept set client with SERVICE_ROUTES base URL. |
| }, | ||
| ]} | ||
| isLoading={false} | ||
| userName="owner" |
Comment on lines
+603
to
+610
| setConceptSetSource(conceptSet.source || null); | ||
| setConceptSetShared(conceptSet.shared); | ||
| setIsUserConceptSet(conceptSet.createdBy === userName); | ||
| setIsUserConceptSet(!!conceptSet.hasWriteAccess); | ||
| setErrorMsg( | ||
| conceptSet.hasWriteAccess | ||
| ? "" | ||
| : LEGACY_CONCEPT_SET_READ_ONLY_MESSAGE | ||
| ); |
Comment on lines
+130
to
132
| const usage = await getConceptSetUsage(token, datasetId, resolvedConceptSetId); | ||
|
|
||
| if (usage.inUse) { |
| ); | ||
| const [terminologyConceptSets, webApiConceptSets] = await Promise.all([ | ||
| terminologySvcApi.getConceptSets(datasetId), | ||
| webApiConceptSetApi.getConceptSets().catch(() => []), |
Comment on lines
+67
to
+74
| const getWebApiBaseUrl = () => { | ||
| try { | ||
| const parsed = JSON.parse(Deno.env.get("SERVICE_ROUTES") ?? "{}"); | ||
| return parsed.webapi ?? DEFAULT_WEBAPI_URL; | ||
| } catch { | ||
| return DEFAULT_WEBAPI_URL; | ||
| } | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#2440
Merge Checklist
Please cross check this list if additions / modifications needs to be done on top of your core changes and tick them off. Reviewer can as well glance through and help the developer if something is missed out.
developbranch)