Conversation
WalkthroughThis PR removes V2 component variants and feature flag-based conditional logic, consolidating the application to use 2025-era helper functions exclusively. Function signatures are simplified by eliminating feature data parameters (featureSNData, featureSCData, featureNewCellClusterData), routes to V2 components are removed, and related Redux reducers and state initialization are cleaned up. Changes
Possibly related PRs
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Summary/GeneSummary.js (1)
32-43: Remove the unusedgene.symbolargument fromfetchDataTypeSummaryLocalcall on line 33.Line 33 passes
this.props.gene.symboltofetchDataTypeSummaryLocal(), but the function defined on line 55 accepts no parameters:fetchDataTypeSummaryLocal = async () => {...}. The argument is silently ignored. Additionally, the internal call tofetchDataTypeSummaryInformation2025()does not accept a gene symbol parameter—only an optionalfetchPolicyargument. Simply remove the unused argument.
🧹 Nitpick comments (5)
src/components/Explorer/DataSelectorContainer.js (1)
6-9: Consider removing the empty mapStateToProps.Since
mapStateToPropsreturns an empty object and provides no state-derived props to the component, you can simplify this by passingnullas the first argument toconnector omittingmapStateToPropsentirely.🔎 Proposed refactor
-const mapStateToProps = (state, props) => -({ - -}); - const mapDispatchToProps = (dispatch, props) => ({ setSelectedConcept(dataType){ dispatch(setDataTypeAndRedirect(dataType, props)); } }); -export default withRouter(connect(mapStateToProps, mapDispatchToProps)(DataSelector)) +export default withRouter(connect(null, mapDispatchToProps)(DataSelector))src/components/DataViz/DataTypeSelectorContainer.js (1)
20-33: Consider simplifying the dispatch pattern and consolidating duplicate cases.
The
dispatch((dispatch) => window.open(...))pattern dispatches a thunk that ignores its parameter. This works but is unconventional—you could callwindow.opendirectly after the dispatch.The
'sc'and'sn'cases have identical navigation logic and could be combined.🔎 Proposed simplification
setDataType(dataType) { dispatch(setDataType(dataType)); if (dataType === 'rt') { - dispatch((dispatch) => window.open("/explorer/regionalviz", "_self")); - } else if (dataType === 'sc') { - dispatch((dispatch) => window.open("/explorer/dataViz", "_self")); - } - else if (dataType === 'sn') { - dispatch((dispatch) => window.open("/explorer/dataViz", "_self")); - } - else if(dataType === "rp"){ - dispatch((dispatch) => window.open("/explorer/regionalpro", "_self")); + window.open("/explorer/regionalviz", "_self"); + } else if (dataType === 'sc' || dataType === 'sn') { + window.open("/explorer/dataViz", "_self"); + } else if (dataType === "rp") { + window.open("/explorer/regionalpro", "_self"); } }src/actions/DataType/dataTypeActions.js (2)
29-29: Unusedpropsparameter.The
propsparameter is declared but never used withinsetDataTypeAndRedirect. Consider removing it if not needed, or document its purpose if retained for API consistency with other action creators.
57-64: Minor style inconsistencies: preferletovervarand ensure consistent semicolons.Line 58 uses
varwhile line 62 useslet(implicitly through reassignment). Also, somebreakstatements are missing trailing semicolons (lines 50, 53, 56, 60, 64) while others have them.🔎 Proposed fix
case "Segmentation Masks & Pathomics Vectors": - var linkVariable = "PAS%20%28Segmentation%20Masks%29"; - window.open('/spatial-viewer/?size=n_20_n&filters%5B0%5D%5Bfield%5D=imagetype&filters%5B0%5D%5Bvalues%5D%5B0%5D=' + linkVariable +'&filters%5B0%5D%5Btype%5D=any', '_self') - break + let linkVariable = "PAS%20%28Segmentation%20Masks%29"; + window.open('/spatial-viewer/?size=n_20_n&filters%5B0%5D%5Bfield%5D=imagetype&filters%5B0%5D%5Bvalues%5D%5B0%5D=' + linkVariable +'&filters%5B0%5D%5Btype%5D=any', '_self'); + break; default: - linkVariable = dataType.replaceAll(" ", "%20") - window.open('/spatial-viewer/?size=n_20_n&filters%5B0%5D%5Bfield%5D=datatype&filters%5B0%5D%5Bvalues%5D%5B0%5D='+ linkVariable +'&filters%5B0%5D%5Btype%5D=any', '_self') - break + const encodedDataType = dataType.replaceAll(" ", "%20"); + window.open('/spatial-viewer/?size=n_20_n&filters%5B0%5D%5Bfield%5D=datatype&filters%5B0%5D%5Bvalues%5D%5B0%5D='+ encodedDataType +'&filters%5B0%5D%5Btype%5D=any', '_self'); + break; }src/components/Summary/GeneSummary.js (1)
55-67: Consider adding thegeneSymbolparameter if needed by the API.If
fetchDataTypeSummaryInformation2025should filter by gene, the function signature should be updated:🔎 Potential fix if gene filtering is needed
- fetchDataTypeSummaryLocal = async () => { - await fetchDataTypeSummaryInformation2025().then( + fetchDataTypeSummaryLocal = async (geneSymbol) => { + await fetchDataTypeSummaryInformation2025(geneSymbol).then(
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
package.jsonsrc/App.jssrc/actions/Concepts/conceptActions.jssrc/actions/DataType/dataTypeActions.jssrc/actions/FeatureSwitch/featureSwitchActions.jssrc/components/ConceptSelect/ConceptSelect.jssrc/components/ConceptSelect/ConceptSelectContainer.jssrc/components/ConceptSelect/ConceptSelectFullWidth.jssrc/components/DataViz/DataTypeSelector.jssrc/components/DataViz/DataTypeSelectorContainer.jssrc/components/DataViz/RNASeqViz.jssrc/components/DataViz/RNASeqVizContainer.jssrc/components/DataViz/RNASeqVizContainerV2.jssrc/components/DataViz/RNASeqVizV2.jssrc/components/DataViz/UMAPContainer.jssrc/components/DataViz/UMAPContainerV2.jssrc/components/DataViz/UMAPViz.jssrc/components/DataViz/UMAPVizV2.jssrc/components/DataViz/featureSwitchReducer.jssrc/components/Explorer/DataSelector.jssrc/components/Explorer/DataSelectorContainer.jssrc/components/Explorer/NephronSchemaCard.jssrc/components/Explorer/NephronSchemaCardContainer.jssrc/components/Explorer/SamplesByDataTypeTable.jssrc/components/ExpressionTables/DiffexByCluster.jssrc/components/ExpressionTables/DiffexByClusterContainer.jssrc/components/ExpressionTables/DiffexByClusterContainerV2.jssrc/components/ExpressionTables/DiffexInfoBar.jssrc/components/Home/SamplesByDataTypeTable.jssrc/components/Home/SamplesByDataTypeTableContainer.jssrc/components/Plots/UMAPPlot.jssrc/components/Plots/UMAPPlot2.jssrc/components/Summary/CellTypeSummary.jssrc/components/Summary/CellTypeSummaryContainer.jssrc/components/Summary/CellTypeSummaryContainerV2.jssrc/components/Summary/GeneSummary.jssrc/components/Summary/GeneSummaryContainer.jssrc/components/Summary/GeneSummaryContainerV2.jssrc/helpers/ApolloClient.jssrc/helpers/Utils.jssrc/initialState.jsonsrc/reducers.js
💤 Files with no reviewable changes (11)
- src/components/DataViz/RNASeqVizV2.js
- src/components/DataViz/featureSwitchReducer.js
- src/components/Plots/UMAPPlot2.js
- src/actions/FeatureSwitch/featureSwitchActions.js
- src/components/DataViz/UMAPContainerV2.js
- src/components/ExpressionTables/DiffexByClusterContainerV2.js
- src/components/DataViz/UMAPVizV2.js
- src/components/Summary/GeneSummaryContainerV2.js
- src/components/Summary/CellTypeSummaryContainerV2.js
- src/helpers/ApolloClient.js
- src/components/DataViz/RNASeqVizContainerV2.js
🧰 Additional context used
🧬 Code graph analysis (17)
src/components/DataViz/UMAPViz.js (1)
src/helpers/ApolloClient.js (2)
fetchPlotlyData2025(169-199)fetchPlotlyData2025(169-199)
src/components/DataViz/RNASeqViz.js (2)
src/helpers/Utils.js (6)
options(64-91)options(97-118)options(123-148)options(153-153)getDataTypeOptions2025(122-150)getDataTypeOptions2025(122-150)src/helpers/ApolloClient.js (4)
fetchGeneExpression2025(249-283)fetchGeneExpression2025(249-283)fetchPlotlyData2025(169-199)fetchPlotlyData2025(169-199)
src/components/ConceptSelect/ConceptSelectContainer.js (2)
src/actions/DataType/dataTypeActions.js (2)
setDataTypeAndRedirect(29-67)setDataTypeAndRedirect(29-67)src/actions/Concepts/conceptActions.js (2)
setSelectedConceptAndRedirect(23-37)setSelectedConceptAndRedirect(23-37)
src/components/ExpressionTables/DiffexByCluster.js (1)
src/helpers/ApolloClient.js (2)
fetchGeneExpression2025(249-283)fetchGeneExpression2025(249-283)
src/components/ExpressionTables/DiffexInfoBar.js (1)
src/helpers/Utils.js (6)
getDataTypeOptions2025(122-150)getDataTypeOptions2025(122-150)options(64-91)options(97-118)options(123-148)options(153-153)
src/components/Explorer/NephronSchemaCardContainer.js (2)
src/actions/EnrollmentCategory/enrollmentCategoryActions.js (2)
setEnrollmentCategory(3-8)setEnrollmentCategory(3-8)src/actions/Concepts/conceptActions.js (2)
setSelectedConceptAndRedirect(23-37)setSelectedConceptAndRedirect(23-37)
src/components/Explorer/DataSelectorContainer.js (1)
src/actions/DataType/dataTypeActions.js (2)
setDataTypeAndRedirect(29-67)setDataTypeAndRedirect(29-67)
src/components/DataViz/DataTypeSelectorContainer.js (4)
src/components/DataViz/UMAPContainer.js (1)
mapDispatchToProps(12-15)src/components/DataViz/RegionalVizContainer.js (1)
mapDispatchToProps(15-26)src/components/Explorer/Explorer.js (1)
mapDispatchToProps(31-48)src/actions/DataType/dataTypeActions.js (2)
setDataType(3-27)setDataType(3-27)
src/components/ConceptSelect/ConceptSelect.js (1)
src/helpers/ApolloClient.js (2)
fetchDataTypesForConcept2025(235-247)fetchDataTypesForConcept2025(235-247)
src/components/Explorer/NephronSchemaCard.js (2)
src/helpers/ApolloClient.js (2)
fetchCellTypeHierarchy2025(85-109)fetchCellTypeHierarchy2025(85-109)src/helpers/googleAnalyticsHelper.js (2)
handleGoogleAnalyticsEvent(3-9)handleGoogleAnalyticsEvent(3-9)
src/components/Home/SamplesByDataTypeTableContainer.js (4)
src/components/Summary/GeneSummaryContainer.js (1)
mapDispatchToProps(13-20)src/components/ConceptSelect/ConceptSelectContainer.js (1)
mapDispatchToProps(15-26)src/components/Explorer/DataSelectorContainer.js (1)
mapDispatchToProps(11-16)src/actions/DataType/dataTypeActions.js (2)
setDataTypeAndRedirect(29-67)setDataTypeAndRedirect(29-67)
src/components/Home/SamplesByDataTypeTable.js (1)
src/helpers/ApolloClient.js (2)
fetchDataTypeSummaryInformation2025(138-167)fetchDataTypeSummaryInformation2025(138-167)
src/components/Summary/GeneSummary.js (1)
src/helpers/Utils.js (2)
getDataTypeOptions2025(122-150)getDataTypeOptions2025(122-150)
src/components/Summary/CellTypeSummary.js (1)
src/helpers/ApolloClient.js (2)
fetchClusterHierarchy2025(111-136)fetchClusterHierarchy2025(111-136)
src/components/Explorer/SamplesByDataTypeTable.js (2)
src/helpers/ApolloClient.js (2)
fetchDataTypeSummaryInformation2025(138-167)fetchDataTypeSummaryInformation2025(138-167)src/helpers/googleAnalyticsHelper.js (2)
handleGoogleAnalyticsEvent(3-9)handleGoogleAnalyticsEvent(3-9)
src/components/DataViz/DataTypeSelector.js (1)
src/helpers/ApolloClient.js (2)
fetchDataTypeSummaryInformation2025(138-167)fetchDataTypeSummaryInformation2025(138-167)
src/components/Summary/GeneSummaryContainer.js (1)
src/actions/DataType/dataTypeActions.js (2)
setDataTypeAndRedirect(29-67)setDataTypeAndRedirect(29-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build (20.19.0)
🔇 Additional comments (32)
package.json (1)
60-61: Minor: DevDependencies reordering appears unrelated to PR objective.Lines 60–61 show a reordering of the
react-scriptsandsassdevDependencies entries with no version changes. This change is purely cosmetic and unrelated to the feature switch removal objective.Consider reverting this change to keep the PR focused on its core goal (removing V2 components and feature flags).
src/components/ConceptSelect/ConceptSelectFullWidth.js (1)
17-17: Props spreading simplifies pass-through logic.The change to spread all props (
{...this.props}) simplifies the component by eliminating explicit prop assignments. This aligns with the removal of feature flag props and maintains flexibility as the prop surface evolves.src/components/Explorer/SamplesByDataTypeTable.js (1)
45-45: Consolidation to 2025 helper function is correct.The migration to
fetchDataTypeSummaryInformation2025removes the conditional logic based on feature flags, simplifying the data fetching path.src/actions/Concepts/conceptActions.js (2)
23-23: Verify all callers updated for simplified function signature.The signature change from
setSelectedConceptAndRedirect(concept, featureNewCellClusterData, props)tosetSelectedConceptAndRedirect(concept, props)removes the feature flag parameter. Confirm all call sites have been updated to use the new 2-parameter signature.
26-35: Route consolidation is complete.The routes have been successfully simplified:
cell_type→/explorer/celltypesummarygene→/explorer/genesummaryRoute definitions are properly configured in
src/App.jsand match the references inconceptActions.js. V2 route variants have been fully removed from the codebase.src/initialState.json (1)
1-10: Feature flags completely removed from codebase.Verification confirms that
featureSNData,featureSCData, andfeatureNewCellClusterDatahave been completely removed—no remaining references found in reducers, selectors, components, or elsewhere.src/components/Explorer/DataSelector.js (1)
24-24: Props correctly simplified to remove feature flags.The removal of
featureSCDataandfeatureSNDataprops aligns with the broader refactor to eliminate feature-flag-based conditional logic. ReviewingSamplesByDataTypeTable.jsconfirms these props are no longer needed.src/components/Home/SamplesByDataTypeTable.js (2)
4-4: API migration to 2025 data source.The migration from
fetchDataTypeSummaryInformationtofetchDataTypeSummaryInformation2025simplifies the data-fetching logic by removing feature-flag conditionals. The relevant code snippet confirms the 2025 API returns the expected data structure.Also applies to: 38-38
54-54: VerifysetDataTypesignature consistency across the codebase.The call to
setDataTypehas been simplified from a 4-argument form to 2 arguments, removingfeatureSNDataandfeatureSCData. Ensure all implementations and callers ofsetDataTypehave been updated to match this new signature.Run the following script to verify consistent usage of the simplified
setDataTypesignature:#!/bin/bash # Description: Verify all setDataType calls use the new 2-argument signature # Find all setDataType implementations echo "=== setDataType implementations ===" ast-grep --pattern $'setDataType($$$) { $$$ }' # Find all setDataType calls with context echo -e "\n=== setDataType call sites ===" rg -nP -C3 'setDataType\s*\(' --type=js --type=jsx -g '!node_modules' -g '!*.test.js' # Check for any remaining 4-argument calls (heuristic: look for setDataType with multiple commas) echo -e "\n=== Potential 4-argument setDataType calls ===" rg -nP 'setDataType\([^)]*,[^)]*,[^)]*,' --type=js --type=jsx -g '!node_modules'src/components/Plots/UMAPPlot.js (1)
20-31: UMAP reference image assets verified and in place.The new v3 reference images exist in the public directory:
public/img/KPMP_SC_reference-UMAP_v3.png(232 KB)public/img/KPMP_SN_reference-UMAP_v3.svg(17.5 MB)No action needed.
src/components/DataViz/UMAPViz.js (1)
5-5: Migration to 2025 API is complete and consistent.The import at line 5 and usage at line 67 correctly use
fetchPlotlyData2025. No remaining references to the oldfetchPlotlyDatafunction exist in the codebase. The response structure withreferenceDatacontaining cluster properties (clusterAbbreviation, clusterName, xValues, yValues, color) is properly handled in thesetData()method. The same function is also consistently used in RNASeqViz.js.src/components/ExpressionTables/DiffexInfoBar.js (1)
3-3: LGTM! Feature flag removed and consolidated to 2025 helper.The conditional logic based on
featureNewCellClusterDatahas been successfully removed, and the component now consistently usesgetDataTypeOptions2025. The change aligns with the PR objective to remove feature switches and consolidate to 2025-era helpers. Error handling is preserved.Also applies to: 18-27
src/components/Explorer/NephronSchemaCard.js (1)
3-3: LGTM! Feature-gated logic removed successfully.The component has been simplified to always use
fetchCellTypeHierarchy2025and thesetSelectedConceptcall no longer branches on feature flags. This consolidation aligns with the PR's goal to remove V2 component variants and feature switches.Also applies to: 25-34
src/components/ExpressionTables/DiffexByCluster.js (1)
5-5: LGTM! Consolidated to 2025 data fetching path.The component now consistently uses
fetchGeneExpression2025for all non-regional data types, and thefeatureNewCellClusterDataprop has been correctly removed from theDiffexInfoBarcomponent. This simplification aligns with the broader refactor to remove feature switches.Also applies to: 56-64, 205-205
src/components/DataViz/RNASeqViz.js (1)
7-11: LGTM! Successfully migrated to 2025-era helpers.All data-fetching helpers have been updated to use the 2025 variants:
getDataTypeOptions2025(line 102)fetchGeneExpression2025(line 122)fetchPlotlyData2025(line 134)The component logic remains unchanged, and the file has been reformatted for consistency. This aligns with the PR's goal to consolidate to 2025-era helpers.
Also applies to: 102-102, 122-131, 134-141
src/components/DataViz/UMAPContainer.js (1)
2-2: UMAPViz component correctly replaces the old UMAP component.The container now wraps
UMAPVizinstead ofUMAP. The consolidated component exists and the old UMAP component has been removed. Redux wiring remains unchanged.src/components/Summary/CellTypeSummaryContainer.js (2)
8-12: LGTM! Clean removal of feature switch state dependencies.The removal of
featureSNData,featureSCData, andfeatureNewCellClusterDatafrommapStateToPropscorrectly aligns with the PR objectives.
17-23: The/explorer/diffexroute is properly configured. The route exists insrc/App.js:82and correctly maps to theDiffexByClusterContainercomponent.src/components/Home/SamplesByDataTypeTableContainer.js (1)
7-18: LGTM! Consistent simplification of state and dispatch props.The removal of
featureSNDataandfeatureSCDatafrommapStateToPropsand the simplifiedsetDataTypesignature correctly align with the updatedsetDataTypeAndRedirectaction that now accepts only(concept, props). This is consistent with similar changes across other container components.src/components/Explorer/DataSelectorContainer.js (1)
11-16: LGTM! Simplified dispatch signature.The updated
setSelectedConceptnow correctly dispatchessetDataTypeAndRedirect(dataType, props)without feature switch parameters, aligning with the consolidated action signature shown in the relevant code snippets.src/components/DataViz/DataTypeSelector.js (3)
6-6: LGTM! Consolidation to 2025 data fetcher.The import change from
fetchDataTypeSummaryInformationtofetchDataTypeSummaryInformation2025correctly consolidates to the newer API variant as part of removing legacy feature switch logic.
60-76: LGTM! Simplified setDataType calls.The removal of
featureSNDataandfeatureSCDataparameters from thesetDataTypecalls (Lines 63 and throughout) correctly simplifies the data type selection flow.
158-164: LGTM! Simplified handleInputChange.Line 161 correctly updates the
setDataTypecall to pass onlyinputValue.value, removing the extra feature switch parameters.src/reducers.js (1)
8-16: No orphaned references to removed reducers detected.The removal of
featureSNData,featureSCData, andfeatureNewCellClusterDatafrom the combined state is clean with no remaining references in the codebase.src/components/Summary/CellTypeSummary.js (1)
48-48: LGTM!The simplified
setDataTypeAndClustercall with two arguments aligns with the updated function signature after feature switch removal.src/components/ConceptSelect/ConceptSelect.js (1)
21-46: LGTM!The
handleSelectmethod is cleanly refactored to use the simplified signatures. The data flow is straightforward:
- Always uses
fetchDataTypesForConcept2025- Calls
setDataTypeandsetSelectedConceptwith single arguments- Error handling is preserved
src/components/ConceptSelect/ConceptSelectContainer.js (1)
15-26: LGTM!The
mapDispatchToPropsis cleanly refactored:
setDataTypenow dispatchessetDataTypeAndRedirectwith simplified argumentssetSelectedConceptcorrectly uses conditional dispatch based onprops.useRedirectionThis aligns well with the simplified action creator signatures defined in the relevant code snippets.
src/components/Explorer/NephronSchemaCardContainer.js (1)
12-18: LGTM!The
mapDispatchToPropsis correctly updated to use the simplifiedsetSelectedConceptAndRedirect(concept, props)signature while preserving the enrollment category reset behavior.src/components/Summary/GeneSummary.js (2)
5-6: LGTM on helper migration.The imports are correctly updated to use the 2025-era helper functions, aligning with the broader refactoring effort.
194-194: LGTM on removingfeatureNewCellClusterDataprop.This aligns with the PR objective to remove feature switch logic. The component now uses a simpler API without feature flags.
src/App.js (2)
19-27: LGTM on V2 component removal.The imports are correctly updated to reference non-V2 containers (
RNASeqVizContainerandGeneSummaryContainer), and the corresponding V2 routes have been removed. This consolidation aligns with the PR objective to remove feature switches.
75-88: Routes look consistent.All routes now reference non-V2 components. The cleanup is complete with no orphaned V2 route definitions.
| fetchDataTypeSummaryInformation = async (geneSymbol) => { | ||
| this.setState({ isDatasetSummaryLoading: true, datasetToggle: 'collapsed' }); | ||
| if ((this.props.dataType === "sc" && this.props.featureSCData) || (this.props.dataType === "sn" && this.props.featureSNData)) { | ||
| return fetchDataTypeSummaryInformation2025(geneSymbol).then( | ||
| (datasetSummary) => { | ||
| if (datasetSummary) { | ||
| datasetSummary = this.formatGeneDataset(datasetSummary) | ||
| this.setSelectedDatasetSummary(this.props.dataType, datasetSummary) | ||
| this.setState({ isDatasetSummaryLoading: false }); | ||
| return datasetSummary | ||
| } | ||
| }, | ||
| (error) => { | ||
| let selectedDataset = { | ||
| participantCount: '-', | ||
| hrtCount: '-', | ||
| akiCount: '-', | ||
| ckdCount: '-' | ||
| } | ||
|
|
||
| console.log('There was a problem fetching the gene summary data: ' + error) | ||
| this.setState({ selectedDataset, isDatasetSummaryLoading: false }); | ||
| return fetchDataTypeSummaryInformation2025(geneSymbol).then( | ||
| (datasetSummary) => { | ||
| if (datasetSummary) { | ||
| datasetSummary = this.formatGeneDataset(datasetSummary) | ||
| this.setSelectedDatasetSummary(this.props.dataType, datasetSummary) | ||
| this.setState({ isDatasetSummaryLoading: false }); | ||
| return datasetSummary | ||
| } | ||
| ); | ||
| } else { | ||
| return fetchDataTypeSummaryInformation(geneSymbol).then( | ||
| (datasetSummary) => { | ||
| if (datasetSummary) { | ||
| datasetSummary = this.formatGeneDataset(datasetSummary) | ||
| this.setSelectedDatasetSummary(this.props.dataType, datasetSummary) | ||
| this.setState({ isDatasetSummaryLoading: false }); | ||
| return datasetSummary | ||
| } | ||
| }, | ||
| (error) => { | ||
| let selectedDataset = { | ||
| participantCount: '-', | ||
| hrtCount: '-', | ||
| akiCount: '-', | ||
| ckdCount: '-' | ||
| } | ||
|
|
||
| console.log('There was a problem fetching the gene summary data: ' + error) | ||
| this.setState({ selectedDataset, isDatasetSummaryLoading: false }); | ||
| }, | ||
| (error) => { | ||
| let selectedDataset = { | ||
| participantCount: '-', | ||
| hrtCount: '-', | ||
| akiCount: '-', | ||
| ckdCount: '-' | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| console.log('There was a problem fetching the gene summary data: ' + error) | ||
| this.setState({ selectedDataset, isDatasetSummaryLoading: false }); | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the fetchDataTypeSummaryInformation2025 function definition and usage
echo "=== Looking for fetchDataTypeSummaryInformation2025 definition ==="
rg -n 'fetchDataTypeSummaryInformation2025' --type js --type ts -B 3 -A 10
echo -e "\n=== Looking for getDataTypeSummaryInformation2025 GraphQL query ==="
rg -n 'getDataTypeSummaryInformation2025' --type js --type ts --type graphql -B 2 -A 5
echo -e "\n=== Checking for older fetch variants (non-2025) ==="
rg -n 'fetchDataTypeSummaryInformation' --type js --type ts | grep -v '2025' | head -20Repository: KPMP/pegasus-web
Length of output: 11743
🏁 Script executed:
#!/bin/bash
echo "=== Full fetchDataTypeSummaryInformation2025 implementation ==="
sed -n '138,170p' src/helpers/ApolloClient.js
echo -e "\n=== Checking if there are multiple versions or overloads ==="
rg -n 'const fetchDataTypeSummaryInformation2025|export const fetchDataTypeSummaryInformation2025' src/helpers/ApolloClient.js -A 3Repository: KPMP/pegasus-web
Length of output: 1537
🏁 Script executed:
#!/bin/bash
echo "=== Check fetchDataTypeSummaryInformation method signature and parameters ==="
sed -n '89,115p' src/components/DataViz/DataTypeSelector.js
echo -e "\n=== Check where geneSymbol comes from ==="
sed -n '35,45p' src/components/DataViz/DataTypeSelector.js
echo -e "\n=== Check old implementation if it exists ==="
git log --oneline -n 5 src/components/DataViz/DataTypeSelector.js 2>/dev/null || echo "Git history not available"Repository: KPMP/pegasus-web
Length of output: 1838
Fix parameter mismatch: geneSymbol is being passed to wrong parameter slot.
The refactored fetchDataTypeSummaryInformation method calls fetchDataTypeSummaryInformation2025(geneSymbol) at line 91, but the actual function signature only accepts fetchPolicy as a parameter: async (fetchPolicy = 'no-cache'). This passes the gene symbol string where a fetch policy strategy is expected, which will cause incorrect behavior. The geneSymbol parameter is unused by the underlying query and should not be passed. Either remove the argument or update the function to accept and use geneSymbol if the query needs gene-specific filtering.
| handleDataTypeClick(dataType) { | ||
| handleGoogleAnalyticsEvent('Explorer', 'Navigation', `data type: ${dataType} and gene: ${this.props.gene}`); | ||
| this.props.setSelectedConcept(dataType, this.props.featureSNData, this.props.featureSCData, this.props); | ||
| this.props.setSelectedConcept(dataType, this.props); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -A 2 'setSelectedConcept\s*\(' --type=js --type=jsx -g '!node_modules'Repository: KPMP/pegasus-web
Length of output: 87
🏁 Script executed:
# Also find the implementation/definition of setSelectedConcept
rg -n -B 2 'setSelectedConcept\s*[=:]' --type=js --type=jsx -g '!node_modules' | head -50Repository: KPMP/pegasus-web
Length of output: 87
🏁 Script executed:
rg -n -A 2 'setSelectedConcept\s*\(' --type=js -g '!node_modules'Repository: KPMP/pegasus-web
Length of output: 3249
🏁 Script executed:
# Search for setSelectedConcept definition without type restriction
rg -n 'setSelectedConcept\s*[:=]' -g '!node_modules' | head -50Repository: KPMP/pegasus-web
Length of output: 322
🏁 Script executed:
sed -n '12,40p' src/actions/Concepts/conceptActions.jsRepository: KPMP/pegasus-web
Length of output: 946
🏁 Script executed:
# Also check what type of parameter concept should be
cat -n src/actions/Concepts/conceptActions.js | head -50Repository: KPMP/pegasus-web
Length of output: 1513
Fix incorrect setSelectedConcept call signature in SamplesByDataTypeTable.js.
The call at line 50 passes (dataType, this.props) but the function signature expects a single concept object with {type, value, name} properties. All other call sites correctly pass a concept object. Update the call to match the expected format, similar to how it's done in NephronSchemaCard.js.
| fetchClusterHierarchy = async () => { | ||
| this.setState({ isLoading: true }); | ||
| if (this.props.featureNewCellClusterData){ | ||
| console.log("Using 2025 query for cluster hierarchy"); | ||
| let results = await fetchClusterHierarchy2025(this.props.cellType); | ||
|
|
||
| this.setState({ cellTypeSummary: results, isLoading: false }); | ||
| }else { | ||
| let results = await fetchClusterHierarchy(this.props.cellType); | ||
| this.setState({ cellTypeSummary: results, isLoading: false }); | ||
| } | ||
| let results = await fetchClusterHierarchy2025(this.props.cellType); | ||
| this.setState({ cellTypeSummary: results, isLoading: false }); | ||
| }; |
There was a problem hiding this comment.
Add null check for results to prevent potential runtime error.
fetchClusterHierarchy2025 returns undefined when an error occurs (per ApolloClient.js lines 110-135). Setting cellTypeSummary to undefined could cause issues when passed to the Grid component's rows prop.
🔎 Proposed fix
fetchClusterHierarchy = async () => {
this.setState({ isLoading: true });
let results = await fetchClusterHierarchy2025(this.props.cellType);
- this.setState({ cellTypeSummary: results, isLoading: false });
+ this.setState({ cellTypeSummary: results || [], isLoading: false });
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fetchClusterHierarchy = async () => { | |
| this.setState({ isLoading: true }); | |
| if (this.props.featureNewCellClusterData){ | |
| console.log("Using 2025 query for cluster hierarchy"); | |
| let results = await fetchClusterHierarchy2025(this.props.cellType); | |
| this.setState({ cellTypeSummary: results, isLoading: false }); | |
| }else { | |
| let results = await fetchClusterHierarchy(this.props.cellType); | |
| this.setState({ cellTypeSummary: results, isLoading: false }); | |
| } | |
| let results = await fetchClusterHierarchy2025(this.props.cellType); | |
| this.setState({ cellTypeSummary: results, isLoading: false }); | |
| }; | |
| fetchClusterHierarchy = async () => { | |
| this.setState({ isLoading: true }); | |
| let results = await fetchClusterHierarchy2025(this.props.cellType); | |
| this.setState({ cellTypeSummary: results || [], isLoading: false }); | |
| }; |
| handleLinkClick = (dataTypeShort, dataType) => { | ||
| handleGoogleAnalyticsEvent('Explorer', 'Navigation', `data type: ${dataTypeShort} and gene: ${this.props.gene.symbol}`); | ||
| this.props.setDataType(dataType, this.props.featureSNData, this.props.featureSCData, this.props); | ||
| this.props.setDataType(dataType, this.props); | ||
| }; |
There was a problem hiding this comment.
Remove unused second argument from setDataType call.
As noted in the container review, setDataType only accepts dataType as a parameter. The this.props argument is ignored.
🔎 Proposed fix
handleLinkClick = (dataTypeShort, dataType) => {
handleGoogleAnalyticsEvent('Explorer', 'Navigation', `data type: ${dataTypeShort} and gene: ${this.props.gene.symbol}`);
- this.props.setDataType(dataType, this.props);
+ this.props.setDataType(dataType);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| handleLinkClick = (dataTypeShort, dataType) => { | |
| handleGoogleAnalyticsEvent('Explorer', 'Navigation', `data type: ${dataTypeShort} and gene: ${this.props.gene.symbol}`); | |
| this.props.setDataType(dataType, this.props.featureSNData, this.props.featureSCData, this.props); | |
| this.props.setDataType(dataType, this.props); | |
| }; | |
| handleLinkClick = (dataTypeShort, dataType) => { | |
| handleGoogleAnalyticsEvent('Explorer', 'Navigation', `data type: ${dataTypeShort} and gene: ${this.props.gene.symbol}`); | |
| this.props.setDataType(dataType); | |
| }; |
| const mapDispatchToProps = (dispatch, props) => | ||
| ({ | ||
| setDataType(dataType) { | ||
| dispatch(setEnrollmentCategory("all")); | ||
| dispatch(setDataTypeAndRedirect(dataType, props)); | ||
| } | ||
|
|
||
| }); |
There was a problem hiding this comment.
Unused parameter in setDataType call from GeneSummary.
The setDataType function signature here accepts only dataType, but in GeneSummary.js (line 71), it's called with two arguments: this.props.setDataType(dataType, this.props). The second argument is silently ignored.
Since props is already available via the mapDispatchToProps closure, the behavior is correct, but the call site in GeneSummary.js should be updated to remove the unused argument for clarity.
🔎 Suggested fix in GeneSummary.js
// In src/components/Summary/GeneSummary.js line 71
- this.props.setDataType(dataType, this.props);
+ this.props.setDataType(dataType);Committable suggestion skipped: line range outside the PR's diff.
|
|
||
| export const getDataTypeOptionsWithEnrollmentCategory = async (geneSymbol, cluster, datasetSummary, currentEnrollmentCategory) => { | ||
| let options = await getDataTypeOptions(geneSymbol, cluster, datasetSummary); | ||
| let options = await getDataTypeOptions2025(geneSymbol, cluster, datasetSummary); |
There was a problem hiding this comment.
Extra argument passed to getDataTypeOptions2025.
getDataTypeOptions2025 (defined at lines 122-150) accepts only two parameters: (geneSymbol, cluster). The third argument datasetSummary is being passed but will be ignored. This appears to be leftover from when the older getDataTypeOptions was called. Consider removing it for clarity.
🔎 Proposed fix
export const getDataTypeOptionsWithEnrollmentCategory = async (geneSymbol, cluster, datasetSummary, currentEnrollmentCategory) => {
- let options = await getDataTypeOptions2025(geneSymbol, cluster, datasetSummary);
+ let options = await getDataTypeOptions2025(geneSymbol, cluster);
for (let index in options) {
2 participants
Summary by CodeRabbit
Release Notes
Features Removed
Updates
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.