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
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 49 additions & 34 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,7 @@
"jsonpath-plus": "^6.0.1",
"jw-bootstrap-switch-ng2": "^2.0.5",
"label-designer": "^4.0.5",
"laji-form": "^12.5.17",
"laji-map": "^3.16.16",
"laji-form": "^14.0.3",
"laji-map": "^3.16.17",
"leaflet.sync": "^0.2.4",
"localforage": "^1.10.0",
Expand Down
8 changes: 4 additions & 4 deletions projects/label-designer/src/lib/schema.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,16 @@ export class SchemaService {
private getValueMap(item): undefined|{[value: string]: string} {
function pick(from) {
const result = {};
for (let i = 0; i < from.enum.length; i++) {
result[from.enum[i]] = from.enumNames[i];
for (const one of from.oneOf) {
result[one.const] = one.title;
}
return result;
}

if (Array.isArray(item.enum) && Array.isArray(item.enumNames)) {
if (Array.isArray(item.oneOf)) {
return pick(item);
}
if (item.items && Array.isArray(item.items.enum) && Array.isArray(item.items.enumNames)) {
if (item.items && Array.isArray(item.items.oneOf)) {
return pick(item.items);
}

Expand Down
3 changes: 1 addition & 2 deletions projects/laji-api-client/src/lib/models/UISchemaContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ export interface UISchemaContext {
export namespace UISchemaContext {

export interface ListEnum {
enum: string[];
enumNames: string[];
oneOf: {const: string, title: string}[];
}
export interface AnnotationMap {
[targetID: string]: Annotation[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,9 @@ export class NpEditFormComponent implements OnInit {
type: keyof Pick<AreaService, 'getMunicipalities' | 'getBiogeographicalProvinces' | 'getBirdAssociationAreas'>
): Observable<Form.IEnum> => (this.areaService[type](this.translate.currentLang)).pipe(
map(areas => areas.reduce((enums, area) => {
enums.enum.push(area.id);
enums.enumNames.push(area.value);
enums.push({const: area.id, title: area.value});
return enums;
}, { enum: [], enumNames: [] }))
}, []))
);
return this.projectFormService.getPlaceForm$(data.documentForm).pipe(switchMap(placeForm => forkJoin([
placeForm.schema.properties.municipality ? getAreaEnum('getMunicipalities') : of(null),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class CellValueSelectComponent {
this.labels = [];

if (field.enum) {
this.labels = [VALUE_IGNORE, ...field.enumNames];
this.labels = [VALUE_IGNORE, ...field.enum.map(item => item.title)];
}
}

Expand All @@ -45,10 +45,10 @@ export class CellValueSelectComponent {

if (to === VALUE_IGNORE) {
mapping[value] = to;
} else if (this._field.enumNames) {
const idx = this._field.enumNames.indexOf(to);
if (idx > -1) {
mapping[value] = this._field.enum[idx];
} else if (this._field.enum) {
const enu = this._field.enum.find(item => item.title === to);
if (enu) {
mapping[value] = enu.const;
}
} else if (this._field.type === 'integer') {
mapping[value] = +to;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ export interface IFormField {
type: string;
splitType?: SplitType;
subGroup?: string;
enum?: string[];
enumNames?: string[];
enum?: {title: string; const: string}[];
default?: any;
col?: string;
previousValue?: any;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ export class GeneratorService {
}

if (useLabels && field.enum && field.default) {
const valueIdx = field.enum.indexOf(field.default);
value = field.enumNames[valueIdx];
value = field.enum.find(item => item.const === field.default).title;
} else if (field.type === 'boolean') {
value = this.mappingService.reverseMap(value, field);
}
Expand Down Expand Up @@ -191,7 +190,7 @@ export class GeneratorService {
}

if (field.enum) {
validValues = (useLabels ? field.enumNames : field.enum).filter(val => val !== '');
validValues = field.enum.map(item => useLabels ? item.title : item.const).filter(val => val !== '');
} else if (field.type === 'boolean') {
validValues = [this.mappingService.mapFromBoolean(true), this.mappingService.mapFromBoolean(false)];
} else if (special) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,10 @@ export class MappingService {
}
switch (field.type) {
case 'string':
if (field.enum && field.enumNames) {
const idx = field.enum.indexOf(value);
if (idx !== -1) {
return field.enumNames[idx];
if (field.enum) {
const enu = field.enum.find(item => item.const === value);
if (enu !== undefined) {
return enu.title;
}
}
break;
Expand Down Expand Up @@ -270,11 +270,12 @@ export class MappingService {
return;
}
this.mapping.string[field.key] = {};
field.enum.map((value, idx) => {
field.enum.map((item) => {
const value = item.const;
if (value === '') {
return;
}
const label = field.enumNames[idx].toLowerCase();
const label = item.title.toLowerCase();
this.mapping.string[field.key][value.toLowerCase()] = value;
this.mapping.string[field.key][label.toLowerCase()] = value;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,7 @@ export class SpreadsheetService {
isArray: root.endsWith('[*]'),
required: this.hasRequiredValidator(schema.id, lastKey, validators, required, root),
subGroup: this.analyzeSubGroup(root, parent, unitSubGroups),
enum: schema.enum,
enumNames: schema.enumNames,
enum: schema.oneOf,
default: schema.default
});

Expand Down
5 changes: 1 addition & 4 deletions projects/laji/src/app/shared/model/Form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ export namespace Form {
options: ListOptions;
}

export interface IEnum {
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.


export interface IAnnotationMap {
[targetID: string]: Annotation[];
Expand Down