-
Notifications
You must be signed in to change notification settings - Fork 100
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
Feat: anrok - add error detail to invoice serialization #2329
Conversation
03ad076
to
a8af1fd
Compare
c62c619
to
8318ccc
Compare
40a4849
to
4e518d1
Compare
826e703
to
3154c9e
Compare
74ccbc5
to
9072e0c
Compare
62a603d
to
9d7bd8e
Compare
9d7bd8e
to
3d9861b
Compare
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.
LGTM!
graphql_name 'ErrorDetail' | ||
|
||
field :error_code, Types::ErrorDetails::ErrorCodesEnum, null: false | ||
field :error_details, GraphQL::Types::JSON, null: true |
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.
What do you think of making it string? It would be easier for FE to fetch it and it follows structure used on other similar places (e.g. netsuite object). If we will have new key in error details, we will just expose it as new field in this 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.
ah, sure!
Was thinking about making it a string, but found Jsons at some places, so thought it's more common to use Jsons 🙈
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.
yep, we also use JSON somewhere, but in those cases that field is flexible and can have various keys. In our case (until we extend error details) it is just string that is describing error code. If doable, I prefer defining type to avoid errors related to it :D
## Context When adding error_details to the invoices [PR](#2329), we added details of the errors as a jsonb object, while for now we'll be only storing there the error_code / error clarification and FE doesn't need the whole object for this ## Description Updated error_detail type to send only first value of it's detils
## Context As part of implementation logic to store errors received from Anrok through Nango we need to serialize error_details to the FE and API ## Description Added Error Details to serializers ---------
…go#2333) ## Context When adding error_details to the invoices [PR](getlago#2329), we added details of the errors as a jsonb object, while for now we'll be only storing there the error_code / error clarification and FE doesn't need the whole object for this ## Description Updated error_detail type to send only first value of it's detils
Context
As part of implementation logic to store errors received from Anrok through Nango we need to serialize error_details to the FE and API
Description
Added Error Details to serializers