Conversation
Reviewer's GuideReplaces the hard-coded imaging modality list with data-driven options fetched from a new /imaging-modalities API endpoint, plumbs modality metadata through App → QueryForm/ResultContainer/ResultCard, and extends tests and fixtures to cover the new behavior and notifications for success/empty/partial/failure cases. Sequence diagram for fetching imaging modalities and rendering chipssequenceDiagram
participant App
participant axios
participant Snackbar as NotificationSystem
participant QueryForm
participant ResultContainer
participant ResultCard
App->>App: useEffect()
App->>App: getAttributes(imaging-modalities, nb:Image, onSuccess)
App->>axios: GET baseAPIURL + imaging-modalities
axios-->>App: AttributeResponse_ImagingModalityOption_
alt nodes_response_status == fail
App->>Snackbar: enqueueSnackbar("Failed to retrieve imaging-modalities options", error)
App-->>App: return
else nodes_response_status != fail
App->>App: record warning notifications for errors[]
App->>App: items = responses[nb:Image]
alt items is empty or undefined
App->>Snackbar: add info notification "No imaging modalities options were available"
else items nonempty
App->>App: filter items with null Label (if removeMissingLabels)
App->>App: build ImagingModalitiesMetadata map
App->>App: setImagingModalityOptions(items)
App->>App: setImagingModalitiesMetadata(map)
end
end
App-->>QueryForm: props.imagingModalityOptions
App-->>ResultContainer: props.imagingModalitiesMetadata
QueryForm->>QueryForm: render CategoricalField "Imaging modality" using imagingModalityOptions
ResultContainer->>ResultCard: pass imagingModalitiesMetadata and imageModals
ResultCard->>ResultCard: filter imageModals with valid metadata (DataType, Abbreviation)
ResultCard->>ResultCard: lookup color via modalitiesDataTypeColorMapping[DataType]
ResultCard->>ResultCard: render modality chips with Abbreviation or Label
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
✅ Deploy Preview for neurobagel-query ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR transitions the imaging modality options from hardcoded constants to a dynamic API-driven approach, fetching modality metadata from a new /imaging-modalities endpoint. The changes enable more flexible and maintainable imaging modality management by allowing the backend to control available options and their metadata.
Key changes:
- Introduced new types (
ImagingModalityOption,ImagingModalitiesMetadata, genericAttributeResponse<T>) to support imaging modality metadata with DataType and Abbreviation fields - Replaced the hardcoded
modalitiesconstant with amodalitiesDataTypeColorMappingthat maps BIDS data types to colors, enabling API-driven modality metadata while preserving color associations - Enhanced the shared
getAttributeshelper function to be generic and support configurable label filtering, allowing it to handle imaging modalities alongside diagnoses, assessments, and pipelines
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/types.ts |
Added ImagingModalityOption extending AttributeOption with optional Abbreviation and DataType fields; added ImagingModalitiesMetadata map type and generic AttributeResponse<T> |
src/utils/constants.ts |
Replaced the hardcoded modalities object with modalitiesDataTypeColorMapping that maps BIDS data types to colors |
src/App.tsx |
Generalized getAttributes function to support typed responses and configurable label filtering; added imaging modality fetch with metadata map construction keyed by both prefixed and full IRI formats |
src/components/QueryForm.tsx |
Updated to consume imagingModalityOptions from API instead of hardcoded modalities constant |
src/components/ResultContainer.tsx |
Updated modality label lookup to use API-driven imagingModalitiesMetadata map |
src/components/ResultCard.tsx |
Refactored modality chip rendering to use API metadata for labels/abbreviations, filter out incomplete metadata, and apply colors based on DataType mapping |
cypress/fixtures/mocked-responses.ts |
Added comprehensive mock responses for imaging modalities including success, empty, partial failure, and complete failure scenarios |
cypress/e2e/ResultsTSV.cy.ts |
Added imaging modality API intercepts and wait conditions |
cypress/e2e/Auth.cy.ts |
Added imaging modality API intercepts to authentication flow tests |
cypress/e2e/APIRequests.cy.ts |
Added comprehensive test coverage for imaging modality API responses including empty, partial failure, and failure cases with notification assertions |
cypress/component/ResultContainer.cy.tsx |
Updated all test cases to provide imagingModalitiesMetadata prop |
cypress/component/ResultCard.cy.tsx |
Updated test fixtures with imaging modality metadata and verified correct color rendering based on DataType |
cypress/component/QueryForm.cy.tsx |
Updated to provide imagingModalityOptions prop in all test cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
surchs
left a comment
There was a problem hiding this comment.
Thanks @rmanaem. It seems you have inconsistent type requirements between AttributeOption (requires Label) and getAttributes (optional Label / can be null).
Could you take a look at that before merging please. also left a couple of other comments.
Sidenote: preview does not work in this case because it's testing against fAPI that does not return the expected imaging data. This is a bigger problem but would be good to come up with some ideas for how to address this (i.e. preview build against staging rather than production)
I'm approving with the above changes
🧑🍳
|
🚀 PR was released in |
Changes proposed in this pull request:
ImagingModalityOption,ImagingModalitiesMetadata) and a genericAttributeResponseAppto pull the imaging-modalities endpoint with DataType/Abbreviation casing.modalitiesstructureconstantsResultContainerandResultCardto consume API-driven modality metadata for labels/abbreviations/colors and to skip modalities missing DataType or AbbreviationAPIRequestse2e suiteResultCardandResultContainercomponent testsChecklist
This section is for the PR reviewer
[ENH],[FIX],[REF],[TST],[CI],[MNT],[INF],[MODEL],[DOC]) (see our Contributing Guidelines for more info)skip-release(to be applied by maintainers only)Closes #XXXXquery-tool-resultsfiles in the neurobagel_examples repo have been regeneratedFor new features:
For bug fixes:
Summary by Sourcery
Fetch imaging modality options and metadata from a new API endpoint instead of hardcoded constants, and propagate this data through the query form and results views.
New Features:
Enhancements:
Tests: