refactor: remove dataAccessPath from initial layer configs #2583

DamonU2 · 2024-11-06T19:36:09Z

Closes #2537

Description

Removes the dataAccessPath from the source in the inital layer config source.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

https://damonu2.github.io/geoview/demos-navigator.html

Checklist:

I have build (rush build) and deploy (rush host) my PR
I have connected the issues(s) to this PR
My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have created new issue(s) related to the outcome of this PR is needed
~~I have made corresponding changes to the documentation~~
~~I have added tests that prove my fix is effective or that my feature works~~
~~New and existing unit tests pass locally with my changes~~

This change is

jolevesq

Reviewed 15 of 44 files at r1, all commit messages.
Reviewable status: 15 of 44 files reviewed, 2 unresolved discussions (waiting on @DamonU2)

packages/geoview-core/schema.json line 1493 at r1 (raw file):

        }
      },
      "required": ["geoviewLayerType", "metadataAccessPath", "listOfLayerEntryConfig"]

Is it mandatory? geocore doesn not have it

packages/geoview-core/public/templates/demos/demo-cgdi.html line 70 at r1 (raw file):

              'listOfLayerEntryConfig': [
                {
                  'layerId': 'items?f=json',

This looks weird..... should it stay hydrometric-stations for layerId but having the metadtaAccessPAth like the dataAccesssPath?

jolevesq

Reviewable status: 15 of 44 files reviewed, 3 unresolved discussions (waiting on @DamonU2)

packages/geoview-core/public/templates/demos/demo-gsc.html line 71 at r1 (raw file):

              'listOfLayerEntryConfig': [
                {
                  'layerId': '46E.JSON',

Here as well... medataDataAccesssPAth it root and layerId is last section but in this case it is duplicated

jolevesq

Reviewable status: 15 of 44 files reviewed, 4 unresolved discussions (waiting on @DamonU2)

packages/geoview-core/public/templates/demos/demo-osdp-non-currated.html line 114 at r1 (raw file):

            'geoviewLayerId': 'geojsonlyr3',
            'geoviewLayerName': 'Multi Polygon Layer',
            'metadataAccessPath': './configs/OSDP/datasets/DDFO-CURATED-1-03336A72-E5AD-4EEE-803D-E70B7FDE9FCB_en.json',

Should it be metada.. ./configs/OSDP/datasets/ and layerId DDFO...

jolevesq

Reviewed 1 of 44 files at r1.
Reviewable status: 16 of 44 files reviewed, 5 unresolved discussions (waiting on @DamonU2)

packages/geoview-core/public/templates/layers/csv.html line 75 at r1 (raw file):

              'geoviewLayerId': 'csvLYR1',
              'geoviewLayerName': 'NPRI',
              'metadataAccessPath': './datasets/csv-files/',

re is it good compared to the json sample

packages/geoview-core/public/templates/layers/csv.html line 90 at r1 (raw file):

              'geoviewLayerId': 'csvLYR2',
              'geoviewLayerName': 'Station List MELCC',
              'metadataAccessPath': './datasets/csv-files/Station_List_Minus_HQ-MELCC.csv',

Here we have duplication again. What is the difference with the item above?

jolevesq

Reviewed 17 of 44 files at r1.
Reviewable status: 33 of 44 files reviewed, 9 unresolved discussions (waiting on @DamonU2)

packages/geoview-core/public/templates/layers/geojson.html line 172 at r1 (raw file):

          geoviewLayerId: 'geoJsonSample',
          geoviewLayerName: 'Historical Flood',
          metadataAccessPath: './datasets/geojson/historical_flood_0.geojson',

duplication again

packages/geoview-core/src/core/utils/config/validation-classes/raster-validation-classes/esri-dynamic-layer-entry-config.ts line 30 at r1 (raw file):

    // We assign the metadataAccessPath of the GeoView layer to dataAccessPath.
    if (!this.source) this.source = {};
    this.source.dataAccessPath = this.geoviewLayerConfig.metadataAccessPath;

workaround to keep dataAccessPath internal but remove from schema? If so we need a GV comment.

If we need some tweeking, because each class has it, we may do it here. In new config, this is not there anymore, we need comment implement behaviour in new structure when we progress in the refactor. Please add TODO comment in each class

packages/geoview-core/src/core/utils/config/validation-classes/raster-validation-classes/xyz-layer-entry-config.ts line 19 at r1 (raw file):

    if (!this.source.dataAccessPath.endsWith('{z}/{y}/{x}'))
      this.source.dataAccessPath = this.source.dataAccessPath!.endsWith('/')
        ? `${this.source.dataAccessPath}tile/{z}/{y}/{x}`

I like the way we tweak it so config is clean and internal stuff if added after

packages/geoview-core/src/geo/map/map-schema-types.ts line 79 at r1 (raw file):

   */
  nameField?: string;
  /** Array of the outfield objects. */

Have you looked at the map-schema type in config api to see if it need to be modify as well

Alex-NRCan

General question. Was this PR tested with HYBRID_MODE = true? It should(?) be transparent, so I understand it wasn't, but I'd like to make sure we don't hit an unforeseen issue, notably with the 'source' property being removed and whatnot.

Reviewed 44 of 44 files at r1, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @DamonU2)

DamonU2

It was

Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @jolevesq)

packages/geoview-core/schema.json line 1493 at r1 (raw file):