-
Notifications
You must be signed in to change notification settings - Fork 32
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
Check API version compatiblity #710
Conversation
to compare semantic version of backend API
as the error is thrown from getApiVersion()
Affected libs:
|
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.
Everythings works well, thanks @fgravin.
There's just the toString() thing to check if it doesn't break anything.
@@ -37,6 +37,6 @@ export class Gn4PlatformMapper { | |||
credentialsNonExpired, | |||
...user | |||
} = apiUser | |||
return { ...apiUser, id: id + '' } as UserModel | |||
return { ...apiUser, id: id.toString() } as UserModel |
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.
The old way ensure a value is set and no Error is thrown. Didn't dig deeper but it may have impact.
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.
I also used to know that it was better too, @jahow wanted me to change that though.
Maybe id?.toString()
😄
This PR ensures the API version compatibility by explicitly testing the compatibility and throwing an error if the API is not compatible.
=> if the GeoNetwork API version is lower then
4.2.2
The error is not silent anymore and is logged in the console. One improvement could be to modify the user error message accordingly.
It introduces a new dependency
semver
for semantic version comparisons.