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

Fix update only stream name also set default to timezone and country code #587

Merged
merged 5 commits into from
Apr 17, 2024

Conversation

Tooseriuz
Copy link
Member

✅ DoD

(use na when API docs (Release notes, etc) do not need to be updated)

📝 Summary

  • Fix converter cannot take null value in request query and body
    • Before, if we send null value, the converter will take it out which I think it is not user's intention. If we add null to request body, it means we want to set it as null.
  • With above converter not taking null. it causes us
    • assume we have stream with latitude = 10, longitude = 100. and we update it with latitude = null. in the current API it will not update to null and also cause timezone and country code to be default

@Tooseriuz Tooseriuz self-assigned this Apr 11, 2024
@rassokhin-s rassokhin-s requested review from veckatimest and removed request for rassokhin-s April 13, 2024 07:37
@@ -23,8 +23,12 @@ module.exports = class Conversion {

Copy link
Contributor

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

Copy link
Contributor

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...
}

Copy link
Contributor

Choose a reason for hiding this comment

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

or even just undefined

Copy link
Member Author

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

Copy link
Contributor

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).

if (data.longitude === undefined) {
data.longitude = stream.longitude
}
additions = (data.latitude === undefined && data.longitude === undefined) ? {} : await computedAdditions(data, stream)
Copy link
Contributor

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes you're right

@Tooseriuz Tooseriuz merged commit d7c417c into develop Apr 17, 2024
3 checks passed
@Tooseriuz Tooseriuz deleted the bug/update-name-but-utc-default branch April 17, 2024 06:49
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.

Update site name from Arbimon make timezone to default UTC
2 participants