-
Notifications
You must be signed in to change notification settings - Fork 12
Move feedback formatting to alert service + make documentediting use it #112
Conversation
The main changes of this PR are:
|
Any objection? |
Could anyone review this PR please? |
@@ -58,7 +58,7 @@ appx.SearchDocumentLocale; | |||
|
|||
/** | |||
* @typedef {{ | |||
* msg: string, | |||
* msg: (string|Object), |
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.
@gberaudo would want a type for this. ;)
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, I also expected @gberaudo would say something here :p
That would make sense indeed, especially since the alerts service expects this is a response
object with a response['data']['errors']
property. But are the API responses always formatted the same way? I'm also afraid that the type is a bit complicated.
Two small comments, otherwise it looks ok. |
* @private | ||
*/ | ||
app.Alerts.prototype.formatErrorMsg_ = function(response) { | ||
var errors = response['data']['errors'], |
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.
Perhaps I should add an assert
here to make sure that response['data']['errors']
is available.
Thanks for reviewing @tsauerwein ! |
5ca817e
to
525218f
Compare
Move feedback formatting to alert service + make documentediting use it
Please note that errors triggered when trying to save a document without being authenticated cannot be managed the right way because the API does not return a correct error message, see c2corg/v6_api#85