diff --git a/CHANGELOG.md b/CHANGELOG.md index 8654299f..0801ab3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ 15.3.0 / wip ================== +Bux fixes: +* [OLMIS-8068](https://openlmis.atlassian.net/browse/OLMIS-8068): Fix NPE on adding programs to products 15.2.9 / 2025-01-14 ================== diff --git a/src/main/java/org/openlmis/referencedata/util/OrderableBuilder.java b/src/main/java/org/openlmis/referencedata/util/OrderableBuilder.java index e2315a5d..4251abc0 100644 --- a/src/main/java/org/openlmis/referencedata/util/OrderableBuilder.java +++ b/src/main/java/org/openlmis/referencedata/util/OrderableBuilder.java @@ -20,9 +20,12 @@ import java.time.ZonedDateTime; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; + +import org.joda.money.CurrencyUnit; import org.joda.money.Money; import org.openlmis.referencedata.domain.Orderable; import org.openlmis.referencedata.domain.OrderableChild; @@ -38,11 +41,15 @@ import org.openlmis.referencedata.service.AuthenticationHelper; import org.openlmis.referencedata.util.messagekeys.UserMessageKeys; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Component; @Component public class OrderableBuilder { + @Value("${currencyCode}") + private String currencyCode; + @Autowired private ProgramRepository programRepository; @@ -100,24 +107,36 @@ public Orderable newOrderable(Orderable.Importer importer, Orderable persistedOr private void setPriceChanges(Orderable persistedOrderable, ProgramOrderableDto item, ProgramOrderable programOrderable, Program program) { List priceChanges = getPreviousPriceChanges(item, programOrderable); + Money newPrice = getOrInitializePrice(programOrderable); + + boolean priceHasChanged = hasPriceChanged(persistedOrderable, program, newPrice); - if (persistedOrderable != null) { - boolean priceHasChanged = true; - Money newPrice = programOrderable.getPricePerPack(); - if (persistedOrderable.getProgramOrderable(program) != null) { - Money currentPrice = persistedOrderable.getProgramOrderable(program) - .getPricePerPack(); - priceHasChanged = !currentPrice.equals(newPrice); - } - - if (priceHasChanged) { - addPriceChange(programOrderable, newPrice, priceChanges); - } + if (priceHasChanged) { + addPriceChange(programOrderable, newPrice, priceChanges); } programOrderable.setPriceChanges(priceChanges); } + private Money getOrInitializePrice(ProgramOrderable programOrderable) { + if (programOrderable.getPricePerPack() != null) { + return programOrderable.getPricePerPack(); + } + + Money defaultPrice = Money.zero(CurrencyUnit.of(currencyCode)); + + programOrderable.setPricePerPack(defaultPrice); + return defaultPrice; + } + + private boolean hasPriceChanged(Orderable persistedOrderable, Program program, Money newPrice) { + return Optional.ofNullable(persistedOrderable) + .map(orderable -> orderable.getProgramOrderable(program)) + .map(ProgramOrderable::getPricePerPack) + .map(currentPrice -> !currentPrice.equals(newPrice)) + .orElse(true); + } + private List getPreviousPriceChanges(ProgramOrderableDto item, ProgramOrderable programOrderable) { return item.getPriceChanges().stream() diff --git a/src/main/java/org/openlmis/referencedata/validate/OrderableValidator.java b/src/main/java/org/openlmis/referencedata/validate/OrderableValidator.java index 955c0cc1..4ba139da 100644 --- a/src/main/java/org/openlmis/referencedata/validate/OrderableValidator.java +++ b/src/main/java/org/openlmis/referencedata/validate/OrderableValidator.java @@ -22,7 +22,6 @@ import static org.openlmis.referencedata.util.messagekeys.OrderableMessageKeys.ERROR_ROUND_TO_ZERO_REQUIRED; import java.math.BigDecimal; -import java.util.Iterator; import java.util.Set; import org.joda.money.Money; @@ -87,20 +86,23 @@ public void validate(Object target, Errors errors) { rejectIfNull(errors, "netContent", ERROR_NET_CONTENT_REQUIRED); OrderableDto dto = (OrderableDto) target; - Set programs = dto.getPrograms(); - if (programs != null && !programs.isEmpty()) { - Iterator iterator = programs.iterator(); - ProgramOrderableDto programOrderableDto = (ProgramOrderableDto)iterator.next(); - Money pricePerPack = programOrderableDto.getPricePerPack(); - BigDecimal amount = pricePerPack.getAmount(); - if (amount.compareTo(BigDecimal.ZERO) < 0) { - throw new ValidationMessageException(OrderableMessageKeys.ERROR_NEGATIVE_PRICE_PER_PACK); - } - } + validatePrograms(dto); validateTemperature(dto, errors); validateVolumeMeasurement(dto, errors); validateProductCode(dto, errors); + } + private void validatePrograms(OrderableDto dto) { + Set programs = dto.getPrograms(); + if (programs != null && !programs.isEmpty()) { + for (ProgramOrderableDto programOrderableDto : programs) { + Money pricePerPack = programOrderableDto.getPricePerPack(); + BigDecimal amount = pricePerPack != null ? pricePerPack.getAmount() : BigDecimal.ZERO; + if (amount.compareTo(BigDecimal.ZERO) < 0) { + throw new ValidationMessageException(OrderableMessageKeys.ERROR_NEGATIVE_PRICE_PER_PACK); + } + } + } } private void validateTemperature(OrderableDto dto, Errors errors) { diff --git a/src/test/java/org/openlmis/referencedata/util/OrderableBuilderTest.java b/src/test/java/org/openlmis/referencedata/util/OrderableBuilderTest.java index 205be971..50894a0e 100644 --- a/src/test/java/org/openlmis/referencedata/util/OrderableBuilderTest.java +++ b/src/test/java/org/openlmis/referencedata/util/OrderableBuilderTest.java @@ -25,12 +25,15 @@ import static org.mockito.Mockito.when; import com.google.common.collect.Sets; + import java.util.HashSet; import java.util.Optional; import java.util.Set; import java.util.UUID; + import org.joda.money.CurrencyUnit; import org.joda.money.Money; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -52,10 +55,14 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.PageRequest; +import org.springframework.test.util.ReflectionTestUtils; +@SuppressWarnings("PMD.TooManyMethods") @RunWith(MockitoJUnitRunner.class) public class OrderableBuilderTest { + private static final String CURRENCY_CODE = "USD"; + @Mock private ProgramRepository programRepository; @@ -68,6 +75,11 @@ public class OrderableBuilderTest { @InjectMocks private OrderableBuilder orderableBuilder; + @Before + public void setUp() { + ReflectionTestUtils.setField(orderableBuilder, "currencyCode", CURRENCY_CODE); + } + @Test public void shouldCreateOrderableWithSingleProgram() throws Exception { Program program = createProgram("test_program"); @@ -135,13 +147,29 @@ public void shouldIncrementVersionNumber() { public void shouldAddNewPriceChange() { Program program = createProgram("test_program"); Orderable orderable = createOrderable(program.getId()); - orderable.getProgramOrderable(program).setPricePerPack(Money.of(CurrencyUnit.of("USD"), 10)); OrderableDto orderableDto = createOrderableDto(program.getId()); + orderableDto.getPrograms().forEach(programOrderableDto -> + programOrderableDto.setPricePerPack(Money.of(CurrencyUnit.of(CURRENCY_CODE), 15))); when(authenticationHelper.getCurrentUser()).thenReturn(new UserDataBuilder().build()); Orderable updatedOrderable = orderableBuilder.newOrderable(orderableDto, orderable); assertThat(updatedOrderable.getProgramOrderable(program).getPriceChanges(), hasSize(1)); + assertThat(updatedOrderable.getProgramOrderable(program).getPriceChanges().get(0).getPrice(), + is(Money.of(CurrencyUnit.of(CURRENCY_CODE), 15))); + } + + @Test + public void shouldAddNewDefaultPriceChangeIfNull() { + Program program = createProgram("test_program"); + OrderableDto orderableDto = createOrderableDto(program.getId()); + when(authenticationHelper.getCurrentUser()).thenReturn(new UserDataBuilder().build()); + + Orderable updatedOrderable = orderableBuilder.newOrderable(orderableDto, null); + + assertThat(updatedOrderable.getProgramOrderable(program).getPriceChanges(), hasSize(1)); + assertThat(updatedOrderable.getProgramOrderable(program).getPricePerPack(), + is(Money.zero(CurrencyUnit.of(CURRENCY_CODE)))); } private Program createProgram(String code) {