feat(DataSource): Allow for dynamic data source configuration and creation #3543
Conversation
Added interface in ExtensionManager to add and replace/set data sources. DataSource.initialize is now where a data source is configured. Added onConfiguration hook for a data source to dynamically configure a data source via the URL when initialize is called.
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ohif-platform-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov Report
@@ Coverage Diff @@
## master #3543 +/- ##
==========================================
+ Coverage 42.75% 42.93% +0.17%
==========================================
Files 82 80 -2
Lines 1450 1444 -6
Branches 338 338
==========================================
Hits 620 620
+ Misses 667 661 -6
Partials 163 163
Continue to review full report in Codecov by Sentry.
|
| for (const [key, value] of searchParams) { | ||
| lowerCaseSearchParams.set(key.toLowerCase(), value); | ||
| } |
There was a problem hiding this comment.
this lower casing utility probably is usefull for dicom tags too, can we put it in utilties?
| useEffect(() => { | ||
| const dataSourceChangedCallback = () => { | ||
| setIsDataSourceInitialized(false); | ||
| setDataPath(''); | ||
| setDataSource(extensionManager.getActiveDataSource()[0]); | ||
| setData(DEFAULT_DATA); | ||
| }; | ||
|
|
||
| const sub = extensionManager.subscribe( | ||
| ExtensionManager.EVENTS.ACTIVE_DATA_SOURCE_CHANGED, | ||
| dataSourceChangedCallback | ||
| ); | ||
| return () => sub.unsubscribe(); | ||
| }, []); |
There was a problem hiding this comment.
I prefer the [] useEffects at first but I might be weird
There was a problem hiding this comment.
I rearranged this file to have state followed by effects (since effects might have dependencies on state).
| const [dataSource, setDataSource] = useState(getInitialDataSource()); | ||
| const [isDataSourceInitialized, setIsDataSourceInitialized] = useState(false); |
There was a problem hiding this comment.
useStates are spread out in the file which makes reading the file hard. Can you please group them to the top? if you need those functions just put them int the useState like
const [dataSource, setDataSource] = useState(() => {
const dataSourceName = getInitialDataSourceName();
if (!dataSourceName) {
return extensionManager.getActiveDataSource()[0];
}
const dataSource = extensionManager.getDataSources(dataSourceName)?.[0];
if (!dataSource) {
throw new Error(`No data source found for ${dataSourceName}`);
}
return dataSource;
});
There was a problem hiding this comment.
I rearranged this file to have state followed by effects (since effects might have dependencies on state).
| return dataSourceName; | ||
| }, []); | ||
|
|
||
| const getInitialDataPath = useCallback(() => { |
There was a problem hiding this comment.
what is DataPath? is it datasourcePath? it requires doc please
There was a problem hiding this comment.
I added a comment and renamed it to dataSourcePath
…SourceConfiguration. Arranged DataSourceWrapper to have state followed by effects. Moved a data source's friendly name from its definition to its configuration.
…-api-data-source-configuration
Context
Addresses the following issues...
Changes & Results
Added interface in ExtensionManager to add and replace/set data sources. DataSource.initialize is now where a data source is configured. Added onConfiguration hook for a data source to dynamically configure a data source via the URL when initialize is called.
Testing
onConfigurationto a data source to ensure that hook works. The configuration docs has an example which can be followed.extensionManager.addDataSource) a data source or set/update (viaextensionManager.setDataSource) an existing data source.Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment