Skip to content

Feat catalog integration#2506

Closed
anup39 wants to merge 7 commits into
GeoNode:masterfrom
anup39:feat_catalog_integration
Closed

Feat catalog integration#2506
anup39 wants to merge 7 commits into
GeoNode:masterfrom
anup39:feat_catalog_integration

Conversation

@anup39
Copy link
Copy Markdown
Member

@anup39 anup39 commented Apr 30, 2026

No description provided.

@allyoucanmap allyoucanmap marked this pull request as draft April 30, 2026 08:09
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the catalog system by replacing the dedicated DatasetsCatalog plugin with a more generic Catalog plugin and updates the associated configurations and epics. The review identified a critical issue where the 'Copy configuration from...' functionality in MapViewersCatalog is broken due to the removal of a modal, and suggested improvements for the readability of catalog service merging logic and configuration selection.

I am having trouble creating individual review comments. Click here to see my feedback.

geonode_mapstore_client/client/js/plugins/MapViewersCatalog.jsx (58-104)

critical

The removal of this modal breaks the 'Copy configuration from...' functionality.
The MapViewersCatalogButton enables the mapViewersCatalog control, which previously showed this modal to allow selecting a map viewer to copy from.
With this change, clicking the button has no visible effect as the MapViewersCatalogPlugin no longer responds to the enabled prop to show a catalog of viewers.
This seems to be an incomplete refactoring. The functionality should be restored, possibly by using the new generic Catalog plugin configured to display map viewers.

geonode_mapstore_client/client/js/utils/ResourceUtils.js (569-578)

medium

The logic to merge catalogServices is a bit complex and could be hard to maintain. Consider refactoring for improved readability and maintainability. This alternative approach separates the logic for determining services and selectedService from the object construction, making it easier to understand.

    const dataCatalogServices = data?.catalogServices || {};
    const baseCatalogServices = baseConfig?.catalogServices || {};
    const catalogServices = {
        ...baseCatalogServices,
        ...dataCatalogServices,
        services: !isEmpty(dataCatalogServices.services)
            ? dataCatalogServices.services
            : (baseCatalogServices.services || {}),
        selectedService: dataCatalogServices.selectedService
            || baseCatalogServices.selectedService
            || ''
    };

geonode_mapstore_client/templates/geonode-mapstore-client/_geonode_config.html (265)

medium

This condition is verbose and can be simplified for better readability. A more concise way to achieve the same result is using short-circuit evaluation.

            config.catalogServices.selectedService = (catalogueSelectedService || '').trim() || 'geonode';

@allyoucanmap
Copy link
Copy Markdown

closed in favor of #2533

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants