Fix #2532 Replace of DatasetCatalog with the new Catalog#2533
Fix #2532 Replace of DatasetCatalog with the new Catalog#2533allyoucanmap wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request performs a significant refactoring to align the GeoNode client with the MapStore framework by migrating local logic—such as facets, dataset cataloging, and resource utilities—directly into the framework. It replaces the custom DatasetsCatalog and MetadataExplorer plugins with the standard framework Catalog plugin and updates the backend context processor to provide default catalog service configurations. Feedback from the review highlights a potential casing mismatch when destructuring catalogueServices in ResourceUtils.js and suggests standardizing the inconsistent spelling of 'catalog' and 'catalogue' used throughout the modified files.
| .filter(mLayer => !layers.find(layer => layer.id !== undefined && mLayer?.extra_params?.msId === layer.id)) | ||
| .map(mLayer => resourceToLayerConfig(mLayer?.dataset)); | ||
|
|
||
| const { catalogueServices = {}, catalogueSelectedService = '' } = getConfigProp('geoNodeSettings') || {}; |
There was a problem hiding this comment.
There is a potential casing mismatch here. The context_processors.py file defines the keys as CATALOGUE_SERVICES and CATALOGUE_SELECTED_SERVICE (all caps), but they are being destructured here using camelCase (catalogueServices, catalogueSelectedService). Please verify if there is a mapping layer that handles this conversion, otherwise this will result in undefined values.
There was a problem hiding this comment.
/gemini the mapping layer is in _geonode-config.html
There was a problem hiding this comment.
Thank you for the clarification. Since the mapping is handled in _geonode-config.html, the destructuring in ResourceUtils.js is correct as long as the configuration is correctly passed through that template. I will consider this thread resolved.
| const { catalogueServices = {}, catalogueSelectedService = '' } = getConfigProp('geoNodeSettings') || {}; | ||
| const existingServices = data?.catalogServices?.services || {}; | ||
| const missingServices = Object.fromEntries( | ||
| Object.entries(catalogueServices).filter(([key]) => !existingServices[key]) | ||
| ); | ||
| const catalogServices = Object.keys(missingServices).length > 0 | ||
| ? { | ||
| services: { ...missingServices, ...existingServices }, | ||
| selectedService: data?.catalogServices?.selectedService || catalogueSelectedService | ||
| } | ||
| : data?.catalogServices; |
There was a problem hiding this comment.
There is an inconsistency in the spelling of 'catalog' vs 'catalogue' within this block. While GeoNode settings use the British spelling (catalogueServices), the MapStore configuration key and local variables use the American spelling (catalogServices). While this might be intentional for compatibility with MapStore's internal state, it makes the code harder to maintain. Consider standardizing the local variable names or adding a comment explaining the mapping.
3 participants
This PR introduces following changes: