Feat catalog integration#2506
Conversation
…ted and available
There was a problem hiding this comment.
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)
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)
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)
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';
|
closed in favor of #2533 |
No description provided.