Feature/add packages checker#2
Conversation
…h the center node
Integrates the Vite 4→7 / Vitest upgrade, use-case migration to
domain/usecases, and combos lazy-load UI cleanup from chore/migrate-legacy
on top of the JSON Package Explorer tab added on this branch.
Conflict resolutions:
- vite.config.ts: drop nodePolyfills (Vite 7 upgrade); keep injectAppTitlePlugin.
- IdenticonAvatar: adopt JSX SVG rendering (no dangerouslySetInnerHTML);
keep type: string prop to avoid cascading into GraphNode.type.
- MetadataGraphPanel: adopt cleaned-up lazy combos UI (showLazyCombos gate,
simpler button); drop unused cocTotal / lazyButtonLabel.
- MetadataQueryBuilder: keep both MAX_PAGE_SIZE and i18n imports.
- MetadataExplorerPage: combine new JSON Package tab with the useFuture
refactor and MAX_PAGE_SIZE / ensureFields helpers.
- JsonPackageExplorer: use default import for MetadataGraphView3D (now
default-exported for React.lazy).
- i18n/en.pot, i18n/es.po: regenerated via yarn localize.
MatiasArriola
left a comment
MatiasArriola
left a comment
There was a problem hiding this comment.
The PR looks solid.
Main concern that I think it is worth addressing, is that json-package-utils.ts lives in webapp/pages/ but contains all pure business logic with zero React or presentation dependencies. Per the project's Clean Architecture rules, this belongs in src/domain/ (e.g., src/domain/metadata/JsonPackageIndex.ts for entities/types, and a use case for buildJsonPackageDependencyGraph).
Other minor comments that I curated from Claude. I don't consider them to be required at this point. Some maybe are easy to check and implement, but we could leave for future refactor if time is constrained:
-
getMetadataTypeLabelappears to be a no-op for camelCase inputs. It splits on boundaries, lowercases everything, then re-joins withcapitalizeWordon all words except the first — which reconstructs camelCase. So"categoryOptionCombos"→"categoryOptionCombos". Compare withresourceTypeLabelswhich returns"Category option combos". If the intent is a human-readable label, the first word should also be capitalized and words should be space-separated. (src/domain/metadata/ResourceType.ts:25-41) -
New types are mutable — missing
Readonly<T>.JsonPackageEntry,JsonPackageReference,JsonPackageIncomingReference,JsonPackageIndex, andJsonTypeGraphPolicyare all plain mutable types. These are data structures that shouldn't be mutated after creation. (src/webapp/pages/metadata/json-package-utils.ts:54,:263,:271,:276,:291) -
as Typecasts on select values bypass type checking.event.target.value as GraphViewModeandas JsonPackageGraphModeare unchecked. A const array + guard (or at minimum a runtime check) would be safer. (src/webapp/pages/metadata/JsonPackageExplorer.tsx:247,:266) -
useEffecthooks have intentionally incomplete deps. The first effect omitsselectedEntryKeyand the second omits logic that could live in the event handlers (loadFromText,setSelectedTypeonChange). Consider moving auto-selection into those handlers to avoid the sync-via-effects pattern. (src/webapp/pages/metadata/JsonPackageExplorer.tsx:78-85,:87-96) -
typePriorityOrderandcoreMetadataTypesare plain arrays, notas const.securityMetadataTypesalready usesas const; the other two should match for consistency and to prevent accidental mutation. (src/webapp/pages/metadata/json-package-utils.ts:6,:31) -
CSS uses hardcoded hex colors (
#2c73ff,#40506b,#f7f9fc, etc.) instead of theme variables or design tokens. (src/webapp/pages/metadata/MetadataExplorerPage.css— tabs section, lines ~10-40)
Thanks
- Created src/domain/metadata/JsonPackageIndex.ts -- entity types (JsonPackageEntry, JsonPackageReference, JsonPackageIncomingReference, JsonPackageIndex, JsonPackageGraphMode, JsonTypeGraphPolicy), domain constants (typePriorityOrder, graphPolicyByCenterType), the indexJsonPackage function, and shared utilities - Created src/domain/usecases/metadata/BuildJsonPackageDependencyGraphUseCase.ts -- buildJsonPackageDependencyGraph function and all traversal/graph policy helpers - Deleted src/webapp/pages/metadata/json-package-utils.ts - Updated imports in JsonPackageExplorer.tsx and the test file
|
Fixed,could you review again? |
MatiasArriola
left a comment
MatiasArriola
left a comment
There was a problem hiding this comment.
Thanks @idelcano! Looks good to me.
Only minor thing left: after the refactor, the src/webapp/pages/metadata/__tests__/json-package-utils.spec.ts should also move to the domain layer and update the describe.
3 participants
📌 References
A new tab has been added: JSON loading for metadata to enable visualization.
It groups metadata by type; once selected, you can click on a specific item to display its details in the viewer.
The visibility for each metadata type is restricted in the code here:
https://github.com/EyeSeeTea/metadata-visualizer/pull/2/changes#diff-78e22b10000b68626aff2d656d2483edd12852e39e83106a876742d12efafd8dR71
The goal is to be able to visualize metadata without having to import it into DHIS2.
📝 Implementation
📹 Screenshots/Screen capture
🔥 Notes to the tester