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.

Sign up for GitHub

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

feat: remove optional fields when unnecessary #216

Merged
merged 5 commits into from
Aug 22, 2024
Merged

feat: remove optional fields when unnecessary #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6536b11
start changes on required values
Aug 19, 2024
9a65467
Merge branch 'main' of github.com:digidem/mapeo-schema into feat/remo…
Aug 19, 2024
4c4511c
remove duplicate fields on JSONSchema, solve type errors and tests
Aug 19, 2024
c421d4d
make coords optional
Aug 22, 2024
0f3b049
wrong `[(required) = true]` on optional `Coords`
Aug 22, 2024
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
2 changes: 1 addition & 1 deletion proto/common/v1.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "versionId/v1.proto";

message Common_1 {
// 32-byte random generated number
optional bytes docId = 1 [(required) = true];
bytes docId = 1 [(required) = true];
repeated VersionId_1 links = 2;
google.protobuf.Timestamp createdAt = 3 [(required) = true];
google.protobuf.Timestamp updatedAt = 4 [(required) = true];
Expand Down
8 changes: 4 additions & 4 deletions proto/field/v1.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ message Field_1 {

Common_1 common = 1;

optional string tagKey = 5 [(required) = true];
string tagKey = 5 [(required) = true];
enum Type {
type_unspecified = 0;
text = 1;
Expand All @@ -23,7 +23,7 @@ message Field_1 {
selectMultiple = 4;
}
Type type = 6 [(required) = true];
optional string label = 7 [(required) = true];
string label = 7 [(required) = true];
enum Appearance {
appearance_unspecified = 0;
multiline = 1;
Expand All @@ -36,7 +36,7 @@ message Field_1 {
TagValue_1.PrimitiveValue value = 2;
}
repeated Option options = 10;
optional bool universal = 11;
bool universal = 11;
optional string placeholder = 12;
optional string helperText = 13;
string helperText = 13;
}
6 changes: 3 additions & 3 deletions proto/observation/v1.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ message Observation_1 {

Common_1 common = 1;

optional double lat = 5;
optional double lon = 6;
double lat = 5;
double lon = 6;

// ATTACHMENT
enum AttachmentType {
Expand All @@ -38,7 +38,7 @@ message Observation_1 {

// METADATA
message Metadata {
optional bool manualLocation = 1;
bool manualLocation = 1;

message Position {
google.protobuf.Timestamp timestamp = 1;
Expand Down
2 changes: 1 addition & 1 deletion proto/preset/v1.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ message Preset_1 {
map<string, TagValue_1> addTags = 8;
map<string, TagValue_1> removeTags = 9;
repeated FieldRef fieldRefs = 10;
optional IconRef iconRef = 11;
IconRef iconRef = 11;
repeated string terms = 12;
string color = 13;

Expand Down
9 changes: 8 additions & 1 deletion schema/field/v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,13 @@
"type": "string"
}
},
"required": ["tagKey", "type", "label", "schemaName"],
"required": [
"tagKey",
"type",
"label",
"schemaName",
"universal",
"helperText"
],
"additionalProperties": false
}
13 changes: 11 additions & 2 deletions schema/observation/v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@
"coords": {
"description": "Position details, should be self explanatory. Units in meters",
"type": "object",
"required": [
"latitude",
"longitude",
"altitude",
"heading",
"speed",
"accuracy"
],
"properties": {
"latitude": {
"type": "number"
Expand Down Expand Up @@ -179,7 +187,8 @@
}
}
},
"additionalProperties": false
"additionalProperties": false,
"required": ["manualLocation"]
},
"presetRef": {
"type": "object",
Expand All @@ -197,6 +206,6 @@
"required": ["docId", "versionId"]
}
},
"required": ["schemaName", "tags", "attachments", "metadata"],
"required": ["schemaName", "tags", "attachments", "lat", "lon"],
"additionalProperties": false
}
3 changes: 2 additions & 1 deletion schema/preset/v1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@
"fieldRefs",
"schemaName",
"terms",
"color"
"color",
"iconRef"
],
"additionalProperties": false
}
4 changes: 3 additions & 1 deletion src/lib/decode-conversions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export const convertObservation: ConvertFunction<'observation'> = (
...rest,
attachments: message.attachments.map(convertAttachment),
tags: convertTags(message.tags),
metadata: message.metadata || {},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we set up a default value for metadata it should be smth like {manualLocation:true}, since manualLocation now is required. That field will default to false if undefined, so if we're okay with that, maybe we could leave it undefined?

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as we don't accidentally get manualLocation: undefined when reading, I'm fine with whatever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to this at least, we should be fine

metadata: message.metadata,
presetRef,
}
return obs
Expand Down Expand Up @@ -158,6 +158,8 @@ export const convertPreset: ConvertFunction<'preset'> = (
docId: rest.iconRef.docId.toString('hex'),
versionId: getVersionId(rest.iconRef.versionId),
}
} else {
throw new Error('missing iconRef for preset')
}
return {
...jsonSchemaCommon,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/encode-conversions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export const convertObservation: ConvertFunction<'observation'> = (
mapeoDoc
) => {
const attachments = mapeoDoc.attachments.map(convertAttachment)
const metadata: Observation_1_Metadata = mapeoDoc.metadata && {
const metadata: Observation_1_Metadata | undefined = mapeoDoc.metadata && {
...Observation_1_Metadata.fromPartial(mapeoDoc.metadata),
}
let presetRef
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/good-docs-completed.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export const goodDocsCompleted = [
value: 'somePrimitiveTagValue',
},
],
helperText: '',
deleted: false,
},
expected: {},
Expand Down
10 changes: 9 additions & 1 deletion test/fixtures/good-docs-minimal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ export const goodDocsMinimal = [
updatedAt: cachedValues.updatedAt,
createdBy: cachedValues.createdBy,
links: [],
lat: 24.0424,
lon: 21.0214,
attachments: [],
tags: {},
metadata: {},
metadata: { manualLocation: false },
deleted: false,
},
expected: {},
Expand Down Expand Up @@ -60,6 +62,8 @@ export const goodDocsMinimal = [
label: 'my label',
type: 'text',
deleted: false,
universal: false,
helperText: '',
},
expected: {},
},
Expand All @@ -78,6 +82,10 @@ export const goodDocsMinimal = [
addTags: {},
removeTags: {},
fieldRefs: [],
iconRef: {
docId: cachedValues.refs.docId,
versionId: cachedValues.refs.versionId,
},
terms: [],
deleted: false,
color: '#ff00ff',
Expand Down
Loading