-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix update only stream name also set default to timezone and country code #587
Conversation
… default timezone and UTC, add more tests
@@ -23,8 +23,12 @@ module.exports = class Conversion { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
```
if (this.required && this.src[this.property] == null) {
this.throwError('the parameter is required but was not provided')
}
```
we should probablyu also throw error if required and this.src[this.property] === undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ([null, undefined].includes(this.src[this.property]) {
throw...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or even just undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
== null
is already included null
and undefined
@veckatimest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. I just feel like we should avoid '==' at all costs (but it's up to you).
core/streams/dao/index.js
Outdated
if (data.longitude === undefined) { | ||
data.longitude = stream.longitude | ||
} | ||
additions = (data.latitude === undefined && data.longitude === undefined) ? {} : await computedAdditions(data, stream) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at this point we already know that latitude and longitude are not undefined, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes you're right
✅ DoD
(use na when API docs (Release notes, etc) do not need to be updated)
📝 Summary
null
value in request query and bodynull
to request body, it means we want to set it asnull
.null
. it causes uslatitude = 10
,longitude = 100
. and we update it withlatitude = null
. in the current API it will not update tonull
and also causetimezone
andcountry code
to be default