-
Notifications
You must be signed in to change notification settings - Fork 212
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat(payments-next): actions response validation and dto updates #18457
Conversation
c845324
to
74e2881
Compare
cartId, | ||
}) | ||
); | ||
const cart = await actionsService.restartCart({ |
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 is the rationale behind sometimes doing the const { something }
pojo initialization vs the const something
assignment? I see both at play, but am not sure I understand the reason behind it
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.
Maintaining compatibility. Class validator requires that values be classes, and primitive values can't be transformed to a class like an object can. Because of that, I have moved some methods that previously returned a primitive type over to an object return type that then subsequently gets spread out in the actions layer to return the primitive we want in the frontend.
* Will capture timing for this method and send them to StatsD. Requires the target class to have statsd exposed as a public property. | ||
* Timings will appear as ClassName_methodName in StatsD. | ||
*/ | ||
export function NextIOValidator< |
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.
can we change this to something like MethodTypeValidator
? IO
is an overloaded term that feels like less of a "clear" fit here
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 term type
is overloaded imo, especially in TypeScript, and additionally means something that doesn't impact runtime.
Because: - We want response validation - We want a better way of doing IO validation for actions without the hassle of conversion to class-validator format each time This commit: - Adds a decorator that does the above Closes FXA-10708
Because
This pull request
Issue that this pull request solves
Closes FXA-10708