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

Add options for share json support #11

Merged
merged 32 commits into from
Oct 22, 2020
Merged

Add options for share json support #11

merged 32 commits into from
Oct 22, 2020

Conversation

nf-s
Copy link
Contributor

@nf-s nf-s commented Sep 9, 2020

Add options to support share json conversion

Mother issue - Model architecture: Share & story migration

For my assumptions - see https://github.com/TerriaJS/catalog-converter/blob/master/architecture/0001-share-conversion-assumptions.md

Main changes

  • Add partial property in ConversionOption - this means that errors aren't thrown for required props (eg name or url)
  • Add convertShare function with Share, ShareResult, and Story types
    • So far this seems to support everything except nested user-added-data groups (eg WMS-group) - but I'm sure we will find bugs as we continue testing.
  • Add the following catalogMemberProps
    • { v7: "isShown", v8: "show" }
    • splitDirection

TerriaJS-Server changes

See TerriaJS/terriajs-server#119

Missing share properties:

  • pickedFeatures - not in v8 share links (create or load)
  • timeline - v7 timeline is different from v8
  • locationMarker - not in v8
  • sharedFromExplorerPanel - not in v8

This also contains WMS support (see #13 PR and #12 Issue)

@nf-s nf-s marked this pull request as draft September 9, 2020 11:46
@nf-s nf-s linked an issue Sep 9, 2020 that may be closed by this pull request
@nf-s nf-s self-assigned this Sep 14, 2020
@nf-s
Copy link
Contributor Author

nf-s commented Sep 16, 2020

Currently user added data has duplicate models: This has been fixed, but left here for context

{
  "version": "8.0.0",
  "initSources": [
    {
      "stratum": "user",
      "models": {
        "//Example Datasets": {
          "type": "group",
          "isOpen": true,
          "knownContainerUniqueIds": [
            "/"
          ]
        },
        "__User-Added_Data__": {
          "type": "group",
          "name": "User-Added Data",
          "members": [
            {
              "type": "csv",
              "name": "http://localhost:3001/data/2011Census_TOT_LGA.csv",
              "description": "",
              "info": [],
              "show": true,
              "splitDirection": 0,
              "url": "http://localhost:3001/data/2011Census_TOT_LGA.csv",
              "opacity": 0.8
            }
          ],
          "description": "The group for data that was added by the user via the Add Data panel.",
          "info": [],
          "isOpen": true,
          "knownContainerUniqueIds": [
            "/"
          ]
        },
        "/http://localhost:3001/data/2011Census_TOT_LGA.csv": {
          "type": "csv",
          "name": "http://localhost:3001/data/2011Census_TOT_LGA.csv",
          "show": true,
          "splitDirection": 0,
          "url": "http://localhost:3001/data/2011Census_TOT_LGA.csv",
          "opacity": 0.8,
          "knownContainerUniqueIds": [
            "__User-Added_Data__"
          ]
        }
      },
      "stories": [],
      "workbench": [
        "/http://localhost:3001/data/2011Census_TOT_LGA.csv"
      ],

@nf-s
Copy link
Contributor Author

nf-s commented Sep 17, 2020

@soyarsauce
Copy link
Contributor

Documenting this as you go is so great @nf-s - get them into an ADR (doesn't need to be perfect, just copy what you have in the PR into the .md) so that it lives in the repo as well.

@nf-s nf-s marked this pull request as ready for review September 26, 2020 16:53
@nf-s nf-s requested a review from tephenavies September 28, 2020 05:09
Copy link
Member

@tephenavies tephenavies left a comment

Choose a reason for hiding this comment

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

This is great. There's a small number of changes needed.

@@ -1,6 +1,6 @@
{
"name": "catalog-converter",
"version": "0.0.2",
"version": "0.0.2-alpha.2",
Copy link
Member

Choose a reason for hiding this comment

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

Odd version number, to go from 0.0.2 to 0.0.2-alpha. Any reason? There's no good versioning policy for this repo yet so I'll take suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think @soyarsauce did this for terrace stuff

src/convert.ts Outdated Show resolved Hide resolved
src/convert.ts Outdated Show resolved Hide resolved
src/convert.ts Outdated Show resolved Hide resolved
src/converters/helpers.ts Outdated Show resolved Hide resolved
src/converters/helpers.ts Outdated Show resolved Hide resolved
@nf-s nf-s requested a review from tephenavies October 2, 2020 08:35
@AnaBelgun AnaBelgun assigned tephenavies and unassigned nf-s Oct 12, 2020
Copy link
Member

@tephenavies tephenavies left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. Sorry that I didn't make it clear which mergeRecursive I was referring to.

@nf-s nf-s merged commit f2dbc36 into master Oct 22, 2020
@nf-s nf-s deleted the share-support branch October 22, 2020 04:37
@soyarsauce soyarsauce mentioned this pull request Oct 26, 2020
3 tasks
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.

Support share JSON
3 participants