Update Purchase API v2 #122
Conversation
|
SonarCloud Quality Gate failed. |
| @@ -152,7 +153,26 @@ public async Task<SinglePurchaseResponse> GetPurchase(int purchaseId, User user) | |||
| $"No purchase was found by Purchase Id: {purchaseId} and User Id: {user.Id}"); | |||
| } | |||
|
|
|||
| var paymentDetails = await _mobilePayPaymentsService.GetPayment(Guid.Parse(purchase.TransactionId)); | |||
| PaymentDetails paymentDetails; | |||
| if (purchase.PaymentType == null) | |||
There was a problem hiding this comment.
From my understanding, this assumes that all purchases done prior to this pull request were done using mobilePay. This however is not correct, due to us having issued vouchers as part of the datamodel. As such, I believe line 159 will result in unintended behaviour.
There was a problem hiding this comment.
Looking at this again, we would also have the same problem with the older purchases issued by the ipad in analog.
For addressing my first comment, we could wrap the call to mobilepay in a try-catch, and then assign the payment details to a unknownPurchase type or something like that in the catch if mobilepay were to throw an exception.
If we wanted to assign a specific purchase type for purchases from the ipad, we could check the OrderId, since they should all have the id "Analog". Anything that haven't been matched so far, should then be vouchers, assuming no discrepancies in the datamodel.
|
@jonasanker This has been stale for quite a while. Do we want it fixed, and merged, or do we want a rework of the er model as a "proper" fix instead? |
|
@TTA777 Yea, I think it was lost when the focus changed to Azure. Can we maybe discuss next time or online how the new data model might look like so we can get a sense of the amount of changes? |
|
Awaits completion of #130 |
Purchase Type is currently used to distinguish between Free Purchases and MobilePay. However could be extended in the future. PaymentType is nullable as we old purchases has no assumed payment type.
marfavi
force-pushed
the
feature/jonas/purchase-apiv2-fix
branch
from
a97d595 to
52712d5
Compare
|
Branch is now up to date. What now❓ @jonasanker @TTA777 |
marfavi
force-pushed
the
feature/jonas/purchase-apiv2-fix
branch
from
f9d01aa to
abc4b4b
Compare
|
Closes #120 , #121