Skip to content

Commit

Permalink
OLMIS-8068: Fix NPE on adding programs to products (#136)
Browse files Browse the repository at this point in the history 
* OLMIS-8068: Fix NPE on adding programs to products

* OLMIS-8068: Refactor setPriceChanges method

* OLMIS-8068: Add requested changes
  • Loading branch information
@mgrochalskisoldevelo
mgrochalskisoldevelo authored Feb 5, 2025
1 parent 84be0ee commit 0703455
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 24 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -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
==================
Expand Down
43 changes: 31 additions & 12 deletions src/main/java/org/openlmis/referencedata/util/OrderableBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -100,24 +107,36 @@ public Orderable newOrderable(Orderable.Importer importer, Orderable persistedOr
private void setPriceChanges(Orderable persistedOrderable, ProgramOrderableDto item,
ProgramOrderable programOrderable, Program program) {
List<PriceChange> 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<PriceChange> getPreviousPriceChanges(ProgramOrderableDto item,
ProgramOrderable programOrderable) {
return item.getPriceChanges().stream()
Expand Down
24 changes: 13 additions & 11 deletions src/main/java/org/openlmis/referencedata/validate/OrderableValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -87,20 +86,23 @@ public void validate(Object target, Errors errors) {
rejectIfNull(errors, "netContent", ERROR_NET_CONTENT_REQUIRED);

OrderableDto dto = (OrderableDto) target;
Set<ProgramOrderableDto> 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<ProgramOrderableDto> 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) {
Expand Down
30 changes: 29 additions & 1 deletion src/test/java/org/openlmis/referencedata/util/OrderableBuilderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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");
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 0703455

Please sign in to comment.