OCC-34: Successful payment should include the Order view #163
Conversation
| @inject IContentManager contentManager | ||
|
|
||
| @{ | ||
| var order = await contentManager.GetAsync(Context.Request.Query["Order"]); | ||
|
|
||
| order.DisplayText = T["Success"].Value; | ||
| } |
There was a problem hiding this comment.
Please do this in the controller and pass the resulting shape as the model.
In general, do as little C# in the view as possible. In most cases if you have to use @inject that's a hint the code should be refactored into the controller. The reason is, if there was an exception (for example if the Order query is missing) any error handling middlewares will be already too late by the time the view runs. So you only get the ASP.Net Core error screen. That would be a pain point for any consumer that wants custom error handling/page.
|
|
||
| return Json(new { success = true }); | ||
| return Json(new { success = true, order_content_item_id = order.ContentItemId }); |
There was a problem hiding this comment.
Why are we using snake_case here?
There was a problem hiding this comment.
To match with requires_action and payment_intent_client_secret. What should be the default, camel case?
There was a problem hiding this comment.
Yes. You can use PascalCase, because Json(...) automatically converts it to camelCase anyway.
Don't worry about being consistent with payment_intent_client_secret, because I will change that in OCC-33 as well.
| { | ||
| var order = await _contentManager.GetAsync(orderId); | ||
|
|
||
| if (order != null) |
There was a problem hiding this comment.
If the order is null it should return NotFound();. Otherwise the view will error out.
| @@ -69,7 +69,7 @@ function stripeCardForm(stripe, urlPrefix, fetchErrorText) { | |||
| .then(handleStripeJsResult); | |||
| } else { | |||
| // Show success message. | |||
| window.location.href = `${urlPrefix}/success`; | |||
| window.location.href = `${urlPrefix}/success/${response.OrderContentItemId}`; | |||
There was a problem hiding this comment.
It sends me to https://localhost:5001/success/undefined. It's because this should be response.orderContentItemId? Like I said in the other thread, return Json(new { ... }) automatically converts the result into camelCase.
OCC-34
Fixes #156