Skip to content

MOSIP-20836 - fix holiday “Missing Data” false positives by reusing existing holidayId on create #1267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ymahtat-dev wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

MOSIP-20836 - fix holiday “Missing Data” false positives by reusing existing holidayId on create #1267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...erdata-service/src/main/java/io/mosip/kernel/masterdata/repository/HolidayRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,10 @@ Holiday findHolidayByHolidayNameHolidayDateLocationCodeLangCode(String holidayNa
@Query(value = "FROM Holiday where holidayDate = ?1 and locationCode = ?2 and langCode=?3 and (isDeleted = false or isDeleted is null) ")
Optional<Holiday> findFirstByHolidayByHolidayDateLocationCodeLangCode(LocalDate holidayDate, String locationCode, String langCode);

@Query("FROM Holiday WHERE holidayDate = ?1 AND locationCode = ?2 AND (isDeleted = false OR isDeleted IS NULL)")
Optional<Holiday> findFirstByHolidayDateAndLocationCode(LocalDate holidayDate, String locationCode);


/**
* Get all the holidays
* @param locationCode
Expand Down
33 changes: 21 additions & 12 deletions ...ata-service/src/main/java/io/mosip/kernel/masterdata/service/impl/HolidayServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,20 +243,29 @@ else if(existingHoliday.isPresent()){
entity.setIsActive(existingHoliday.get().getIsActive());
}
else {
entity.setHolidayId(holidayRepository.findMaxHolidayId()+1);
Optional<Holiday> existingSameHoliday = holidayRepository.findFirstByHolidayDateAndLocationCode(
holidayDto.getHolidayDate(),
holidayDto.getLocationCode()
);
if (existingSameHoliday.isPresent()) {
entity.setHolidayId(existingSameHoliday.get().getHolidayId());
entity.setIsActive(existingSameHoliday.get().getIsActive());
} else {
entity.setHolidayId(holidayRepository.findMaxHolidayId() + 1);
}
}

holiday = holidayRepository.save(entity);
/*
* if (holiday.getIsActive() == true &&
* supportedLang.contains(holiday.getLangCode())) { Holiday
* primholiday=holidayRepository.
* findHolidayByHolidayNameHolidayDateLocationCodeLangCode(holiday.
* getHolidayName(),holiday.getHolidayDate(),
* holiday.getLocationCode(),primaryLang); primholiday.setIsActive(true);
* holidayRepository.update(primholiday); }
*/
MapperUtils.map(holiday, holidayId);
holiday = holidayRepository.save(entity);
/*
* if (holiday.getIsActive() == true &&
* supportedLang.contains(holiday.getLangCode())) { Holiday
* primholiday=holidayRepository.
* findHolidayByHolidayNameHolidayDateLocationCodeLangCode(holiday.
* getHolidayName(),holiday.getHolidayDate(),
* holiday.getLocationCode(),primaryLang); primholiday.setIsActive(true);
* holidayRepository.update(primholiday); }
*/
MapperUtils.map(holiday, holidayId);
}
}catch (DataAccessLayerException | DataAccessException | IllegalArgumentException | SecurityException e) {
auditUtil.auditRequest(String.format(MasterDataConstant.FAILURE_UPDATE, Holiday.class.getSimpleName()),
Expand Down
119 changes: 119 additions & 0 deletions ...service/src/test/java/io/mosip/kernel/masterdata/test/service/HolidayServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
package io.mosip.kernel.masterdata.test.service;

import io.mosip.kernel.masterdata.dto.HolidayDto;
import io.mosip.kernel.masterdata.dto.HolidayIDDto;
import io.mosip.kernel.masterdata.dto.HolidayUpdateDto;
import io.mosip.kernel.masterdata.dto.getresponse.extn.HolidayExtnDto;
import io.mosip.kernel.masterdata.dto.request.SearchFilter;
import io.mosip.kernel.masterdata.dto.response.HolidaySearchDto;
import io.mosip.kernel.masterdata.entity.Holiday;
import io.mosip.kernel.masterdata.entity.Location;
import io.mosip.kernel.masterdata.repository.HolidayRepository;
import io.mosip.kernel.masterdata.repository.LocationRepository;
import io.mosip.kernel.masterdata.service.impl.HolidayServiceImpl;
import io.mosip.kernel.masterdata.utils.AuditUtil;
import io.mosip.kernel.masterdata.validator.FilterTypeEnum;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.util.ReflectionTestUtils;
Expand All @@ -22,6 +29,7 @@
import java.util.*;

import static org.junit.Assert.*;
import static org.mockito.Mockito.*;

@SpringBootTest
@RunWith(MockitoJUnitRunner.class)
Expand All @@ -30,9 +38,20 @@ public class HolidayServiceImplTest {
@InjectMocks
HolidayServiceImpl holidayService;

@Mock
private HolidayRepository holidayRepository;

@Mock
private LocationRepository locationRepository;

@Mock
private AuditUtil auditUtil;


private HolidayUpdateDto holidayUpdateDto;
private Location location;


@Before
public void setUp() {
holidayUpdateDto = new HolidayUpdateDto();
Expand Down Expand Up @@ -117,6 +136,106 @@ public void testSetMetaData_Success() {
assertNotNull(locations);
}

@Test
public void saveHoliday_reusesExistingHolidayId_whenSameDateAndLocationDifferentLanguage() {
// GIVEN
final int holidayId = 123;
final String existingHolidayName = "Test Holiday";
final String existingHolidayDesc = "Test Description";
final String holidayName = "Test Holiday Clone";
final String holidayDesc = "Test Description Clone";
final LocalDate date = LocalDate.now();
final String locationCode = "10036";
final String englishLanguageCode = "eng";
final String arabicLanguageCode = "ara";
final boolean activeStatus = true;
// mock audit call
doNothing().when(auditUtil).auditRequest(anyString(), anyString(), anyString(), anyString());
// Existing location with code = "10036"
final Location locEntity = new Location();
locEntity.setCode(locationCode);
locEntity.setLangCode(englishLanguageCode);
final List<Location> locationsResult = Collections.singletonList(locEntity);
when(locationRepository.findByCode(locationCode)).thenReturn(locationsResult);
// Existing holiday (any language, e.g. ENG) for same (date, locationCode) with holidayId = 123
final Holiday existing = new Holiday();
existing.setHolidayId(holidayId);
existing.setHolidayName(existingHolidayName);
existing.setHolidayDesc(existingHolidayDesc);
existing.setHolidayDate(date);
existing.setLocationCode(locationCode);
existing.setLangCode(englishLanguageCode);
existing.setIsActive(activeStatus);
// No existing ARABIC row (same date+locationCode+lang)
when(holidayRepository.findFirstByHolidayByHolidayDateLocationCodeLangCode(
date, locationCode, arabicLanguageCode
)).thenReturn(Optional.empty());
// But an existing row exists for same (date, locationCode) in another language → reuse its holidayId
when(holidayRepository.findFirstByHolidayDateAndLocationCode(
date, locationCode
)).thenReturn(Optional.of(existing));
// Avoid “empty table” branch if the service checks it
when(holidayRepository.count()).thenReturn(1L);
// Capture entity passed to save(...)
ArgumentCaptor<Holiday> captor = ArgumentCaptor.forClass(Holiday.class);
when(holidayRepository.save(captor.capture())).thenAnswer(inv -> {
final Holiday h = captor.getValue();
final Holiday persisted = new Holiday();
persisted.setHolidayId(h.getHolidayId());
persisted.setHolidayDate(h.getHolidayDate());
persisted.setLocationCode(h.getLocationCode());
persisted.setLangCode(h.getLangCode());
persisted.setHolidayName(h.getHolidayName());
persisted.setHolidayDesc(h.getHolidayDesc());
persisted.setIsActive(h.getIsActive());
return persisted;
});
when(holidayRepository.findHolidayByHolidayNameHolidayDateLocationCodeLangCode(
holidayName,
date,
locationCode,
arabicLanguageCode
)).thenReturn(null);
// DTO to create for Arabic, same (date, locationCode)
HolidayDto dto = new HolidayDto();
dto.setHolidayDate(date);
dto.setLocationCode(locationCode);
dto.setLangCode(arabicLanguageCode);
dto.setHolidayName(holidayName);
dto.setHolidayDesc(holidayDesc);
// WHEN
HolidayIDDto out = holidayService.saveHoliday(dto);
// THEN — the new row must reuse holidayId = 123
Holiday saved = captor.getValue();
assertNotNull(saved);
assertEquals(holidayId, saved.getHolidayId());
assertEquals(arabicLanguageCode, saved.getLangCode());
assertEquals(date, saved.getHolidayDate());
assertEquals(locationCode, saved.getLocationCode());
assertEquals(activeStatus, saved.getIsActive());
assertEquals(holidayName, saved.getHolidayName());
assertEquals(holidayDesc, saved.getHolidayDesc());
assertNotNull(out);
assertEquals(holidayId, out.getHolidayId());
assertEquals(arabicLanguageCode, out.getLangCode());
assertEquals(holidayName, out.getHolidayName());
// Should not request a new max id
verify(holidayRepository, never()).findMaxHolidayId();
// Verification of flow
verify(holidayRepository).count();
verify(locationRepository).findByCode(locationCode);
verify(holidayRepository).findFirstByHolidayByHolidayDateLocationCodeLangCode(date, locationCode, arabicLanguageCode);
verify(holidayRepository).findFirstByHolidayDateAndLocationCode(date, locationCode);
verify(holidayRepository).findHolidayByHolidayNameHolidayDateLocationCodeLangCode(
holidayName,
date,
locationCode,
arabicLanguageCode
);
verify(holidayRepository).save(any(Holiday.class));
}


private SearchFilter buildExpectedSearchFilter_Success() {
SearchFilter filter = new SearchFilter();
filter.setColumnName("locationCode");
Expand Down