Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strcturedClone template in getLayer #1014

Merged
merged 5 commits into from
Nov 22, 2023

Conversation

dbauszus-glx
Copy link
Member

The template object must be structuredCloned() prior to merging in the getLayer mod.

The objcet.hasOwn() check should continue the for loop for ease of understanding.

@dbauszus-glx dbauszus-glx added the Bug A genuine bug. There must be some form of error exception to work with. label Nov 21, 2023
@dbauszus-glx
Copy link
Member Author

The geom check should not happen whenever a layer is loaded. This should be done in the client format method.

// Check for layer geom[s].
if ((layer.table || layer.tables) && (!layer.geom && !layer.geoms)) {
  console.warn(`Layer: ${layer.key},has a table or tables defined, but no geom or geoms.`)
}

Copy link
Contributor

@simon-leech simon-leech left a comment

Choose a reason for hiding this comment

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

This fixes the described error with templates that were unrelated being merged together.

I'm still seeing issues with templates array merging. Sample config here as an example

 "test": {
          "name": "Test",
          "group": "Competitors",
          "template": "test",
          "display": true,
          "templates": [
            "streetview_entry",
            "style_theme",
            "style_dataview"
          ]
}

In this example, streetview_entry is simply an infoj entry and style_dataview just changes the layer dataview title. Neither of these are being shown but no errors or warnings.

Copy link

sonarcloud bot commented Nov 21, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@dbauszus-glx
Copy link
Member Author

An already merged template will be returned without having to be merged again.

Please note the reversed order between layer > template but templates[] > layer

https://github.com/GEOLYTIX/xyz/wiki/Workspace-Configuration#template-2

Copy link
Contributor

@simon-leech simon-leech left a comment

Choose a reason for hiding this comment

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

I think this all working for me now - thanks @dbauszus-glx
Config Docs make sense to me too, so i'm happy with this!

@RobAndrewHurst RobAndrewHurst merged commit dc22e27 into GEOLYTIX:main Nov 22, 2023
6 checks passed
@dbauszus-glx dbauszus-glx deleted the clone-template branch January 29, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A genuine bug. There must be some form of error exception to work with.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants