Geospatial maps: Review feedback addressed

* Documentation of config
* Set bounding box to use dcterms.spatial by default
* Uncomment geospatial field and wrap in config test
* New config to 'enable' inclusion of geospatial field
* Docs for map and marker detail models
* Bounding box validation
* Fixed view mode control flow
* Tidied console output

Author: kshepherd

config/config.example.yml

@@ -554,11 +554,27 @@ liveRegion:
 
 # Geospatial Map display options
 geospatialMapViewer:
+  # Which fields to use for parsing as geospatial points in search maps
+  # (note, the item page field component allows any field(s) to be used
+  # and is set as an input when declaring the component)
   spatialMetadataFields:
     - 'dcterms.spatial'
+  # Which discovery configuration to use for 'geospatial search', used
+  # in the browse map
   spatialFacetDiscoveryConfiguration: 'geospatial'
+  # Which filter / facet name to use for faceted geospatial search
+  # used in the browse map
   spatialPointFilterName: 'point'
+  # Whether item page geospatial metadata should be displayed
+  # (assumes they are wrapped in a test for this config in the template as
+  # per the default templates supplied with DSpace for untyped-item and publication)
+  enableItemPageFields: false
+  # Whether the browse map should be enabled and included in the browse menu
   enableBrowseMap: false
+  # Whether a 'map view' mode should be included alongside list and grid views
+  # in search result pages
   enableSearchViewMode: false
+  # The tile provider(s) to use for the map tiles drawn in the leaflet maps.
+  # (see https://leaflet-extras.github.io/leaflet-providers/preview/) for a full list
   tileProviders:
     - 'OpenStreetMap.Mapnik'

src/app/browse-by/browse-by-geospatial-data/browse-by-geospatial-data.component.spec.ts

@@ -102,8 +102,6 @@ describe('BrowseByGeospatialDataComponent', () => {
   it('component should be created successfully', () => {
     expect(component).toBeTruthy();
   });
-  // return this.searchService.getFacetValuesFor(searchFilterConfig, 1, searchOptions,
-  //   null, true);
   describe('BrowseByGeospatialDataComponent component with valid facet values', () => {
     beforeEach(() => {
       fixture = TestBed.createComponent(BrowseByGeospatialDataComponent);

src/app/item-page/simple/field-components/specific-field/geospatial/geospatial-item-page-field.component.ts

@@ -51,7 +51,7 @@ export class GeospatialItemPageFieldComponent extends ItemPageFieldComponent imp
   /**
    * List of fields to parse for bounding box GeoJSON
    */
-  @Input() bboxFields = ['gnd.coordinates.bbox'];
+  @Input() bboxFields = ['dcterms.spatial'];
 
   /**
    * Whether to cluster markers into groups

src/app/item-page/simple/item-types/publication/publication.component.html

@@ -106,15 +106,15 @@
       [fields]="['datacite.relation.isReferencedBy']"
       [label]="'item.page.referenced'">
     </ds-item-page-uri-field>
-    <!-- Below is an example of how to render one or more lat/lng points and/or bounding box rectangles
-         in a tiled map viewer. Set 'cluster' to true for marker clustering -->
-<!--    <ds-geospatial-item-page-field [item]="object"-->
-<!--                                   [label]="'item.page.places'"-->
-<!--                                   [pointFields]="['dcterms.spatial']"-->
-<!--                                   [bboxFields]="['external.spatial.bbox']"-->
-<!--                                   [cluster]="true"-->
-<!--    >-->
-<!--    </ds-geospatial-item-page-field>-->
+    @if (geospatialItemPageFieldsEnabled) {
+      <ds-geospatial-item-page-field [item]="object"
+                                     [label]="'item.page.places'"
+                                     [pointFields]="['dcterms.spatial']"
+                                     [bboxFields]="['dcterms.spatial']"
+                                     [cluster]="true"
+      >
+      </ds-geospatial-item-page-field>
+    }
     <div>
       <a class="btn btn-outline-primary" role="button" [routerLink]="[itemPageRoute + '/full']">
         <i class="fas fa-info-circle"></i> {{"item.page.link.full" | translate}}

src/app/item-page/simple/item-types/shared/item.component.ts

@@ -75,9 +75,15 @@ export class ItemComponent implements OnInit {
 
   mediaViewer;
 
+  /**
+   * Enables display of geospatial item page fields
+   */
+  geospatialItemPageFieldsEnabled = false;
+
   constructor(protected routeService: RouteService,
               protected router: Router) {
     this.mediaViewer = environment.mediaViewer;
+    this.geospatialItemPageFieldsEnabled = environment.geospatialMapViewer.enableItemPageFields;
   }
 
   /**

src/app/item-page/simple/item-types/untyped-item/untyped-item.component.html

@@ -71,15 +71,15 @@
       [fields]="['dc.identifier.citation']"
       [label]="'item.page.citation'">
     </ds-generic-item-page-field>
-    <!-- Below is an example of how to render one or more lat/lng points and/or bounding box rectangles
-         in a tiled map viewer. Set 'cluster' to true for marker clustering -->
-<!--    <ds-geospatial-item-page-field [item]="object"-->
-<!--                                   [label]="'item.page.places'"-->
-<!--                                   [pointFields]="['dcterms.spatial']"-->
-<!--                                   [bboxFields]="['external.spatial.bbox']"-->
-<!--                                   [cluster]="true"-->
-<!--                                >-->
-<!--    </ds-geospatial-item-page-field>-->
+    @if (geospatialItemPageFieldsEnabled) {
+      <ds-geospatial-item-page-field [item]="object"
+                                     [label]="'item.page.places'"
+                                     [pointFields]="['dcterms.spatial']"
+                                     [bboxFields]="['dcterms.spatial']"
+                                     [cluster]="true"
+      >
+      </ds-geospatial-item-page-field>
+    }
     <ds-item-page-uri-field [item]="object"
       [fields]="['dc.identifier.uri']"
       [label]="'item.page.uri'">

src/app/shared/geospatial-map/geospatial-map.component.ts

@@ -213,7 +213,7 @@ export class GeospatialMapComponent implements AfterViewInit, OnInit, OnDestroy
 
     // Map bounds / zoom fitting tends to be smoother when done after a short delay
     setTimeout(() => {
-      if (isNotEmpty(this.parsedBoundingBoxes) && this.coordinates.length === 1) {
+      if (bboxBounds && this.coordinates.length === 1) {
         // One point, at least one bbox, use its bounds. Otherwise, use the calculation based on points.
         bounds = bboxBounds;
       }
@@ -315,12 +315,34 @@ export class GeospatialMapComponent implements AfterViewInit, OnInit, OnDestroy
    * @param bbox
    * @private
    */
-  private renderBoundingBox(L, bbox: string): string[][] {
+  private renderBoundingBox(L, bbox: string): number[][] {
     if (hasValue(bbox)) {
       let parsedBbox = bbox.replace(/[{} ]/g, '');
       parsedBbox = parsedBbox.replace(/[^=,]+=/g, '');
-      const parsedBboxParts = parsedBbox.split(',');
-      const bounds = [[parsedBboxParts[1], parsedBboxParts[0]], [parsedBboxParts[2], parsedBboxParts[3]]];
+      const parsedBboxParts = parsedBbox.split(',', 4);
+      // Validate that we have exactly 4 parts
+      if (parsedBboxParts.length !== 4) {
+        console.error('Invalid bounding box format: expected 4 coordinates but got ' + parsedBboxParts.length);
+        return;
+      }
+      // Convert to numbers and validate
+      const coordinates = parsedBboxParts.map(part => parseFloat(part));
+      if (coordinates.some(isNaN)) {
+        console.error('Invalid bounding box: contains non-numeric values', parsedBboxParts);
+        return;
+      }
+      // Create bounds array with proper structure [[lat1, lng1], [lat2, lng2]]
+      const bounds = [[coordinates[1], coordinates[0]], [coordinates[2], coordinates[3]]];
+      // Validate latitude values (-90 to 90)
+      if (bounds[0][0] < -90 || bounds[0][0] > 90 || bounds[1][0] < -90 || bounds[1][0] > 90) {
+        console.error('Invalid bounding box: latitude values must be between -90 and 90', bounds);
+        return;
+      }
+      // Validate longitude values (-180 to 180)
+      if (bounds[0][1] < -180 || bounds[0][1] > 180 || bounds[1][1] < -180 || bounds[1][1] > 180) {
+        console.error('Invalid bounding box: longitude values must be between -180 and 180', bounds);
+        return;
+      }
       const boundingBox = L.rectangle(bounds, { color: '#5574BB', weight: 2, fill: false });
       this.map.addLayer(boundingBox);
       // return bounds so the map can be centred / zoomed appropriately

src/app/shared/geospatial-map/models/geospatial-map-detail.model.ts

@@ -1,5 +1,9 @@
 import { GeospatialMarkerDetail } from './geospatial-marker-detail.model';
 
+/**
+ * This class is used by the ObjectGeospatialMap list view, to supply point data as well as
+ * routing and title data for the DSO associated with the geospatial points.
+ */
 export class GeospatialMapDetail {
   points: GeospatialMarkerDetail[] = [];
   route: string;

src/app/shared/geospatial-map/models/geospatial-marker-detail.model.ts

@@ -1,3 +1,7 @@
+/**
+ * This class is used by GeospatialMapDetail, to keep richer data for a list of points
+ * so that they can be rendered as links and alt-text or hover text when drawn on a map
+ */
 export class GeospatialMarkerDetail {
   latitude: number;
   longitude: number;

src/app/shared/view-mode-switch/view-mode-switch.component.html

@@ -40,20 +40,20 @@
       [attr.data-test]="'detail-view' | dsBrowserOnly">
       <i class="far fa-square"></i>
     </button>
-    @if (isToShow(viewModeEnum.GeospatialMap)) {
-      <button
-        [attr.aria-current]="currentMode === viewModeEnum.GeospatialMap"
-        [attr.aria-label]="'search.view-switch.show-geospatialMap' | translate"
-         routerLink="."
-         [queryParams]="{view: 'geospatialMap'}"
-         queryParamsHandling="merge"
-         (click)="switchViewTo(viewModeEnum.GeospatialMap)"
-         routerLinkActive="active"
-         [class.active]="currentMode === viewModeEnum.GeospatialMap"
-         class="btn btn-secondary"
-         [attr.data-test]="'geospatial-view' | dsBrowserOnly">
-         <span class="fas fa-map"></span><span class="sr-only">{{'search.view-switch.show-geospatialMap' | translate}}</span>
-      </button>
-    }
+  }
+  @if (isToShow(viewModeEnum.GeospatialMap)) {
+    <button
+      [attr.aria-current]="currentMode === viewModeEnum.GeospatialMap"
+      [attr.aria-label]="'search.view-switch.show-geospatialMap' | translate"
+       routerLink="."
+       [queryParams]="{view: 'geospatialMap'}"
+       queryParamsHandling="merge"
+       (click)="switchViewTo(viewModeEnum.GeospatialMap)"
+       routerLinkActive="active"
+       [class.active]="currentMode === viewModeEnum.GeospatialMap"
+       class="btn btn-secondary"
+       [attr.data-test]="'geospatial-view' | dsBrowserOnly">
+       <span class="fas fa-map"></span><span class="sr-only">{{'search.view-switch.show-geospatialMap' | translate}}</span>
+    </button>
   }
 </div>