New shared trait for automatic payload validation #518
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
annappropriate
force-pushed
the
phoenix/payload-validation
branch
from
3274eac to
5905c28
Compare
vkorbut
suggested changes
Dec 1, 2016
| with Validation[CartLineItem] { | ||
|
|
||
| override def validate: ValidatedNel[Failure, CartLineItem] = | ||
| attributes.fold(ok)(_.validate.map(_ ⇒ Unit)).map { case _ ⇒ this } |
There was a problem hiding this comment.
Seems like you don't need to validate attributes anymore as soon as they already validated at LineItemAttributes object creation
| @@ -63,6 +65,9 @@ case class OrderLineItem(id: Int = 0, | |||
| override def updateTo(newModel: OrderLineItem): Failures Xor OrderLineItem = | |||
| super.transitionModel(newModel) | |||
|
|
|||
| override def validate: ValidatedNel[Failure, OLI] = | |||
| attributes.fold(ok)(_.validate.map(_ ⇒ Unit)).map { case _ ⇒ this } | |||
There was a problem hiding this comment.
The same as for CartLineItem
There was a problem hiding this comment.
Yeah lol forgot to clean that up. Thanks!
annappropriate
force-pushed
the
phoenix/payload-validation
branch
from
5905c28 to
c991446
Compare
+ validation for line item attrs. All payloads should extend `ValidatedPayload` trait and define their `validate` method. If payload doesn't pass validation, exception is thrown immediately. This has several consequences: 1. We never enter service end don't initiate DB transaction, rejection happens in routes. 2. It's impossible to forget to call `_ ← * <~ payload.validate` and end up with invalid payload, because exception will willingly blow up in your face. 3. If we switch to this approach, DbResultT type definition can get rid of tail on `Failures` in favor of single `Failure`. Validation is the only reason we have `NonEmptyList[Failure]` despite we always return only one `Failure` from service.
annappropriate
force-pushed
the
phoenix/payload-validation
branch
from
c991446 to
e9abd4b
Compare
|
nice, did you test this using TPG's storefront when adding a gift card? |
vkorbut
approved these changes
Dec 3, 2016
|
I am not sure how do I do that given my GCE doesn't provision @mempko |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(plus validation for line item attrs)
All payloads should extend
ValidatedPayloadtrait and define theirvalidatemethod. If payload doesn't pass validation, exception is thrown immediately. This has several consequences:We never enter service end don't initiate DB transaction, rejection happens in routes.
It's impossible to forget to call
_ ← * <~ payload.validateand end up with invalid payload, because exception will willingly blow up in your face.If we switch to this approach,
DbResultTtype definition can get rid of tail onFailuresin favor of singleFailure. Validation is the only reason we haveNonEmptyList[Failure]despite we always return only oneFailurefrom service.