fix(payment): persist amountPaid on payment confirmation (#20) #27
|
|
@ -1,6 +1,7 @@
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
package se.bilhalsning.service;
|
package se.bilhalsning.service;
|
||||||
|
|
||||||
import lombok.RequiredArgsConstructor;
|
import lombok.RequiredArgsConstructor;
|
||||||
|
import org.springframework.beans.factory.annotation.Value;
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
import se.bilhalsning.entity.Order;
|
import se.bilhalsning.entity.Order;
|
||||||
import se.bilhalsning.entity.OrderStatus;
|
import se.bilhalsning.entity.OrderStatus;
|
||||||
|
|
@ -8,6 +9,7 @@ import se.bilhalsning.exception.InvalidOrderStateException;
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
import se.bilhalsning.exception.OrderNotFoundException;
|
import se.bilhalsning.exception.OrderNotFoundException;
|
||||||
import se.bilhalsning.repository.OrderRepository;
|
import se.bilhalsning.repository.OrderRepository;
|
||||||
|
|
||||||
|
import java.math.BigDecimal;
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
||||||
|
|
@ -18,6 +20,9 @@ public class OrderService {
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
private final OrderRepository orderRepository;
|
private final OrderRepository orderRepository;
|
||||||
private final OrderNotificationService orderNotificationService;
|
private final OrderNotificationService orderNotificationService;
|
||||||
|
|
||||||
|
@Value("${app.payment.letter-price:49}")
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
|
private int letterPrice = 49;
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
|
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
public Order createOrder(UUID userId, String plate, String letterText) {
|
public Order createOrder(UUID userId, String plate, String letterText) {
|
||||||
Order order = new Order();
|
Order order = new Order();
|
||||||
order.setUserId(userId);
|
order.setUserId(userId);
|
||||||
|
|
@ -72,6 +77,7 @@ public class OrderService {
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
"Beställningen kan inte ändras i detta tillstånd");
|
"Beställningen kan inte ändras i detta tillstånd");
|
||||||
}
|
}
|
||||||
order.setStatus(OrderStatus.PROCESSING);
|
order.setStatus(OrderStatus.PROCESSING);
|
||||||
|
order.setAmountPaid(BigDecimal.valueOf(letterPrice));
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
Order saved = orderRepository.save(order);
|
Order saved = orderRepository.save(order);
|
||||||
orderNotificationService.notifyOrderProcessing(saved);
|
orderNotificationService.notifyOrderProcessing(saved);
|
||||||
return saved;
|
return saved;
|
||||||
|
|
@ -93,6 +99,7 @@ public class OrderService {
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
public Order confirmPayment(UUID orderId, UUID userId) {
|
public Order confirmPayment(UUID orderId, UUID userId) {
|
||||||
Order order = requirePendingOwnedBy(orderId, userId);
|
Order order = requirePendingOwnedBy(orderId, userId);
|
||||||
order.setStatus(OrderStatus.PROCESSING);
|
order.setStatus(OrderStatus.PROCESSING);
|
||||||
|
order.setAmountPaid(BigDecimal.valueOf(letterPrice));
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
Order saved = orderRepository.save(order);
|
Order saved = orderRepository.save(order);
|
||||||
orderNotificationService.notifyOrderProcessing(saved);
|
orderNotificationService.notifyOrderProcessing(saved);
|
||||||
return saved;
|
return saved;
|
||||||
|
|
|
||||||
|
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
hermes
commented
💡 Redundant 💡 Redundant `= 49` initializer + `int` money type. The `49` default lives in three places (the `:49`, the field init, and `application.yml`), and the `= 49` masks `@Value` failures in the unit test (Mockito's `@InjectMocks` never resolves `@Value`, so the test rides on the initializer). Consider `private BigDecimal letterPrice;` and `order.setAmountPaid(letterPrice)` directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is already `scale=2`). See review body for the full test-coverage note.
hermes
commented
♻️ The ♻️ The `setStatus -> setAmountPaid -> save -> notify -> return` block is duplicated in `confirmPayment` (line 102). Extract `private Order markPaidAndProcessing(Order order)` to collapse it (the duplication pre-existed but is deepened here).
hermes
commented
⚠️ ⚠️ `amountPaid` here is the configured `letterPrice`, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.
|
|||||||
|
|
@ -13,6 +13,7 @@ import se.bilhalsning.exception.InvalidOrderStateException;
|
||||||
|
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
|
|||||||
import se.bilhalsning.exception.OrderNotFoundException;
|
import se.bilhalsning.exception.OrderNotFoundException;
|
||||||
import se.bilhalsning.repository.OrderRepository;
|
import se.bilhalsning.repository.OrderRepository;
|
||||||
|
|
||||||
|
import java.math.BigDecimal;
|
||||||
|
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
|
|||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
import java.util.UUID;
|
import java.util.UUID;
|
||||||
|
|
@ -222,6 +223,7 @@ class OrderServiceTest {
|
||||||
|
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
|
|||||||
Order result = orderService.confirmPayment(orderId, userId);
|
Order result = orderService.confirmPayment(orderId, userId);
|
||||||
|
|
||||||
assertEquals(OrderStatus.PROCESSING, result.getStatus());
|
assertEquals(OrderStatus.PROCESSING, result.getStatus());
|
||||||
|
assertEquals(new BigDecimal("49"), result.getAmountPaid());
|
||||||
|
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|
@ -345,6 +347,7 @@ class OrderServiceTest {
|
||||||
|
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
|
|||||||
Order result = orderService.confirmGuestPayment(token);
|
Order result = orderService.confirmGuestPayment(token);
|
||||||
|
|
||||||
assertEquals(OrderStatus.PROCESSING, result.getStatus());
|
assertEquals(OrderStatus.PROCESSING, result.getStatus());
|
||||||
|
assertEquals(new BigDecimal("49"), result.getAmountPaid());
|
||||||
|
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
|
|||||||
verify(orderNotificationService).notifyOrderProcessing(result);
|
verify(orderNotificationService).notifyOrderProcessing(result);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
hermes
commented
🧪 This assertion passes via the 🧪 This assertion passes via the `= 49` field initializer, not via `@Value`: `@InjectMocks` doesn't resolve Spring `@Value`, so this test cannot catch a misnamed `app.payment.letter-price` PropertySource or a removed `@Value`. The guest-path twin at line 350 has the same gap. Strengthen with `ReflectionTestUtils.setField(orderService, "letterPrice", 99)` + assert `99`, or a `@SpringBootTest` slice that asserts the resolved config differs from a sentinel.
|
|||||||
💡 Redundant
= 49initializer +intmoney type. The49default lives in three places (the:49, the field init, andapplication.yml), and the= 49masks@Valuefailures in the unit test (Mockito's@InjectMocksnever resolves@Value, so the test rides on the initializer). Considerprivate BigDecimal letterPrice;andorder.setAmountPaid(letterPrice)directly — Spring converts the string to BigDecimal and you're future-proofed for öre (the column is alreadyscale=2). See review body for the full test-coverage note.♻️ The
setStatus -> setAmountPaid -> save -> notify -> returnblock is duplicated inconfirmPayment(line 102). Extractprivate Order markPaidAndProcessing(Order order)to collapse it (the duplication pre-existed but is deepened here).⚠️
amountPaidhere is the configuredletterPrice, not an amount verified as received from a payment provider. Fine for Phase 0 honor-system, but once Stripe/Swish webhooks land this should come from the captured payment-intent amount (handles partial payments / refunds / price drift). Worth a Javadoc note documenting the current assumption for finance.