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

Use JSON Schema oneOf schematic instead of enum & enumNames [#183287493] #335

Merged
merged 6 commits into from
Oct 3, 2022

Conversation

olzraiti
Copy link
Member

https://183287493.dev.laji.fi/
https://www.pivotaltracker.com/story/show/183287493

Form backend received changes so that the schema format has changed form enums like this:

enum: ["MX.a", "MX.b"], enumNames: ["aLabel", "bLabel"]
will be like this:
oneOf [{const: "MX.a", title: "aLabel"}, {const: "MX.b", title: "bLabel"}]

Reasoning (copy-pasted from pivotal):

Current form backend serves alt ranges following a rejected JSON Schema proposal [https://github.com/json-schema-org/json-schema-spec/issues/57](which got rejected). In order to serve a proper standard, we are migrating from the enum & enumNames schema to using oneOf (in format {const: string, title: string}.

@esko-piirainen
Copy link
Contributor

@aorin Would be good to check if this breaks 1) label designed 2) vihko import

Copy link
Contributor

@aorin aorin left a comment

Choose a reason for hiding this comment

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

Looks good, I tested the Vihko import and it seems to work. Because there are changes to label designer, we must remember to update the label designer npm package before the update (can't do it before merging to development since there are other pull requests with changes to label designer).

enum: string[];
enumNames: string[];
}
export type IEnum = {const: string; title: string}[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this interface be oneOf: {const: string; title: string}[]; (like in the file projects/laji-api-client/src/lib/models/UISchemaContext.ts above)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch - it worked as it was, but I made it like you assumed since that makes sense. So the areas are now served as proper schema (wrapped into a oneOf object instead of just const & title pairs). I fixed this on also on laji-form side.

Copy link
Member Author

Choose a reason for hiding this comment

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

Because there are changes to label designer, we must remember to update the label designer npm package before the update

Right. The label designer schema service is used only on laji.fi, not on Kotka right? Does laji.fi use the label-designer from the npm package, or from directly from the source? If it uses the label designer from npm, we have to make a release for it and include a dependency update to this PR... It's in package.json deps at least so I assume we have to do it that way. So this PR is recursively depending on it's changes. Pretty crazy stuff!

Copy link
Contributor

Choose a reason for hiding this comment

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

That's correct but this pull request needs to be merged first because I made release 4.0.6 for label designer there

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, so I'll merge that to dev now and then merge dev here.

@olzraiti olzraiti merged commit e91b26a into development Oct 3, 2022
@olzraiti olzraiti deleted the jsonschema-enums-migration-183287493 branch October 3, 2022 15:21
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.

3 participants