Skip to content

Commit ddd7a74

Browse files
ymahtat-devymahtat-dev
committed
MOSIP-20836 - fix holiday “Missing Data” false positives by reusing existing holidayId on create
Signed-off-by: Youssef MAHTAT <youssef.mahtat.as.developer@gmail.com>
1 parent e6a3856 commit ddd7a74

3 files changed

Lines changed: 144 additions & 12 deletions

File tree

‎admin/kernel-masterdata-service/src/main/java/io/mosip/kernel/masterdata/repository/HolidayRepository.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ Holiday findHolidayByHolidayNameHolidayDateLocationCodeLangCode(String holidayNa
163163
@Query(value = "FROM Holiday where holidayDate = ?1 and locationCode = ?2 and langCode=?3 and (isDeleted = false or isDeleted is null) ")
164164
Optional<Holiday> findFirstByHolidayByHolidayDateLocationCodeLangCode(LocalDate holidayDate, String locationCode, String langCode);
165165

166+
@Query("FROM Holiday WHERE holidayDate = ?1 AND locationCode = ?2 AND (isDeleted = false OR isDeleted IS NULL)")
167+
Optional<Holiday> findFirstByHolidayDateAndLocationCode(LocalDate holidayDate, String locationCode);
168+
169+
166170
/**
167171
* Get all the holidays
168172
* @param locationCode

‎admin/kernel-masterdata-service/src/main/java/io/mosip/kernel/masterdata/service/impl/HolidayServiceImpl.java‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -243,20 +243,29 @@ else if(existingHoliday.isPresent()){
243243
entity.setIsActive(existingHoliday.get().getIsActive());
244244
}
245245
else {
246-
entity.setHolidayId(holidayRepository.findMaxHolidayId()+1);
246+
Optional<Holiday> existingSameHoliday = holidayRepository.findFirstByHolidayDateAndLocationCode(
247+
holidayDto.getHolidayDate(),
248+
holidayDto.getLocationCode()
249+
);
250+
if (existingSameHoliday.isPresent()) {
251+
entity.setHolidayId(existingSameHoliday.get().getHolidayId());
252+
entity.setIsActive(existingSameHoliday.get().getIsActive());
253+
} else {
254+
entity.setHolidayId(holidayRepository.findMaxHolidayId() + 1);
255+
}
247256
}
248257

249-
holiday = holidayRepository.save(entity);
250-
/*
251-
* if (holiday.getIsActive() == true &&
252-
* supportedLang.contains(holiday.getLangCode())) { Holiday
253-
* primholiday=holidayRepository.
254-
* findHolidayByHolidayNameHolidayDateLocationCodeLangCode(holiday.
255-
* getHolidayName(),holiday.getHolidayDate(),
256-
* holiday.getLocationCode(),primaryLang); primholiday.setIsActive(true);
257-
* holidayRepository.update(primholiday); }
258-
*/
259-
MapperUtils.map(holiday, holidayId);
258+
holiday = holidayRepository.save(entity);
259+
/*
260+
* if (holiday.getIsActive() == true &&
261+
* supportedLang.contains(holiday.getLangCode())) { Holiday
262+
* primholiday=holidayRepository.
263+
* findHolidayByHolidayNameHolidayDateLocationCodeLangCode(holiday.
264+
* getHolidayName(),holiday.getHolidayDate(),
265+
* holiday.getLocationCode(),primaryLang); primholiday.setIsActive(true);
266+
* holidayRepository.update(primholiday); }
267+
*/
268+
MapperUtils.map(holiday, holidayId);
260269
}
261270
}catch (DataAccessLayerException | DataAccessException | IllegalArgumentException | SecurityException e) {
262271
auditUtil.auditRequest(String.format(MasterDataConstant.FAILURE_UPDATE, Holiday.class.getSimpleName()),

‎admin/kernel-masterdata-service/src/test/java/io/mosip/kernel/masterdata/test/service/HolidayServiceImplTest.java‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,24 @@
11
package io.mosip.kernel.masterdata.test.service;
22

3+
import io.mosip.kernel.masterdata.dto.HolidayDto;
34
import io.mosip.kernel.masterdata.dto.HolidayIDDto;
45
import io.mosip.kernel.masterdata.dto.HolidayUpdateDto;
56
import io.mosip.kernel.masterdata.dto.getresponse.extn.HolidayExtnDto;
67
import io.mosip.kernel.masterdata.dto.request.SearchFilter;
78
import io.mosip.kernel.masterdata.dto.response.HolidaySearchDto;
9+
import io.mosip.kernel.masterdata.entity.Holiday;
810
import io.mosip.kernel.masterdata.entity.Location;
11+
import io.mosip.kernel.masterdata.repository.HolidayRepository;
12+
import io.mosip.kernel.masterdata.repository.LocationRepository;
913
import io.mosip.kernel.masterdata.service.impl.HolidayServiceImpl;
14+
import io.mosip.kernel.masterdata.utils.AuditUtil;
1015
import io.mosip.kernel.masterdata.validator.FilterTypeEnum;
1116
import org.junit.Before;
1217
import org.junit.Test;
1318
import org.junit.runner.RunWith;
19+
import org.mockito.ArgumentCaptor;
1420
import org.mockito.InjectMocks;
21+
import org.mockito.Mock;
1522
import org.mockito.junit.MockitoJUnitRunner;
1623
import org.springframework.boot.test.context.SpringBootTest;
1724
import org.springframework.test.util.ReflectionTestUtils;
@@ -22,6 +29,7 @@
2229
import java.util.*;
2330

2431
import static org.junit.Assert.*;
32+
import static org.mockito.Mockito.*;
2533

2634
@SpringBootTest
2735
@RunWith(MockitoJUnitRunner.class)
@@ -30,9 +38,20 @@ public class HolidayServiceImplTest {
3038
@InjectMocks
3139
HolidayServiceImpl holidayService;
3240

41+
@Mock
42+
private HolidayRepository holidayRepository;
43+
44+
@Mock
45+
private LocationRepository locationRepository;
46+
47+
@Mock
48+
private AuditUtil auditUtil;
49+
50+
3351
private HolidayUpdateDto holidayUpdateDto;
3452
private Location location;
3553

54+
3655
@Before
3756
public void setUp() {
3857
holidayUpdateDto = new HolidayUpdateDto();
@@ -117,6 +136,106 @@ public void testSetMetaData_Success() {
117136
assertNotNull(locations);
118137
}
119138

139+
@Test
140+
public void saveHoliday_reusesExistingHolidayId_whenSameDateAndLocationDifferentLanguage() {
141+
// GIVEN
142+
final int holidayId = 123;
143+
final String existingHolidayName = "Test Holiday";
144+
final String existingHolidayDesc = "Test Description";
145+
final String holidayName = "Test Holiday Clone";
146+
final String holidayDesc = "Test Description Clone";
147+
final LocalDate date = LocalDate.now();
148+
final String locationCode = "10036";
149+
final String englishLanguageCode = "eng";
150+
final String arabicLanguageCode = "ara";
151+
final boolean activeStatus = true;
152+
// mock audit call
153+
doNothing().when(auditUtil).auditRequest(anyString(), anyString(), anyString(), anyString());
154+
// Existing location with code = "10036"
155+
final Location locEntity = new Location();
156+
locEntity.setCode(locationCode);
157+
locEntity.setLangCode(englishLanguageCode);
158+
final List<Location> locationsResult = Collections.singletonList(locEntity);
159+
when(locationRepository.findByCode(locationCode)).thenReturn(locationsResult);
160+
// Existing holiday (any language, e.g. ENG) for same (date, locationCode) with holidayId = 123
161+
final Holiday existing = new Holiday();
162+
existing.setHolidayId(holidayId);
163+
existing.setHolidayName(existingHolidayName);
164+
existing.setHolidayDesc(existingHolidayDesc);
165+
existing.setHolidayDate(date);
166+
existing.setLocationCode(locationCode);
167+
existing.setLangCode(englishLanguageCode);
168+
existing.setIsActive(activeStatus);
169+
// No existing ARABIC row (same date+locationCode+lang)
170+
when(holidayRepository.findFirstByHolidayByHolidayDateLocationCodeLangCode(
171+
date, locationCode, arabicLanguageCode
172+
)).thenReturn(Optional.empty());
173+
// But an existing row exists for same (date, locationCode) in another language → reuse its holidayId
174+
when(holidayRepository.findFirstByHolidayDateAndLocationCode(
175+
date, locationCode
176+
)).thenReturn(Optional.of(existing));
177+
// Avoid “empty table” branch if the service checks it
178+
when(holidayRepository.count()).thenReturn(1L);
179+
// Capture entity passed to save(...)
180+
ArgumentCaptor<Holiday> captor = ArgumentCaptor.forClass(Holiday.class);
181+
when(holidayRepository.save(captor.capture())).thenAnswer(inv -> {
182+
final Holiday h = captor.getValue();
183+
final Holiday persisted = new Holiday();
184+
persisted.setHolidayId(h.getHolidayId());
185+
persisted.setHolidayDate(h.getHolidayDate());
186+
persisted.setLocationCode(h.getLocationCode());
187+
persisted.setLangCode(h.getLangCode());
188+
persisted.setHolidayName(h.getHolidayName());
189+
persisted.setHolidayDesc(h.getHolidayDesc());
190+
persisted.setIsActive(h.getIsActive());
191+
return persisted;
192+
});
193+
when(holidayRepository.findHolidayByHolidayNameHolidayDateLocationCodeLangCode(
194+
holidayName,
195+
date,
196+
locationCode,
197+
arabicLanguageCode
198+
)).thenReturn(null);
199+
// DTO to create for Arabic, same (date, locationCode)
200+
HolidayDto dto = new HolidayDto();
201+
dto.setHolidayDate(date);
202+
dto.setLocationCode(locationCode);
203+
dto.setLangCode(arabicLanguageCode);
204+
dto.setHolidayName(holidayName);
205+
dto.setHolidayDesc(holidayDesc);
206+
// WHEN
207+
HolidayIDDto out = holidayService.saveHoliday(dto);
208+
// THEN — the new row must reuse holidayId = 123
209+
Holiday saved = captor.getValue();
210+
assertNotNull(saved);
211+
assertEquals(holidayId, saved.getHolidayId());
212+
assertEquals(arabicLanguageCode, saved.getLangCode());
213+
assertEquals(date, saved.getHolidayDate());
214+
assertEquals(locationCode, saved.getLocationCode());
215+
assertEquals(activeStatus, saved.getIsActive());
216+
assertEquals(holidayName, saved.getHolidayName());
217+
assertEquals(holidayDesc, saved.getHolidayDesc());
218+
assertNotNull(out);
219+
assertEquals(holidayId, out.getHolidayId());
220+
assertEquals(arabicLanguageCode, out.getLangCode());
221+
assertEquals(holidayName, out.getHolidayName());
222+
// Should not request a new max id
223+
verify(holidayRepository, never()).findMaxHolidayId();
224+
// Verification of flow
225+
verify(holidayRepository).count();
226+
verify(locationRepository).findByCode(locationCode);
227+
verify(holidayRepository).findFirstByHolidayByHolidayDateLocationCodeLangCode(date, locationCode, arabicLanguageCode);
228+
verify(holidayRepository).findFirstByHolidayDateAndLocationCode(date, locationCode);
229+
verify(holidayRepository).findHolidayByHolidayNameHolidayDateLocationCodeLangCode(
230+
holidayName,
231+
date,
232+
locationCode,
233+
arabicLanguageCode
234+
);
235+
verify(holidayRepository).save(any(Holiday.class));
236+
}
237+
238+
120239
private SearchFilter buildExpectedSearchFilter_Success() {
121240
SearchFilter filter = new SearchFilter();
122241
filter.setColumnName("locationCode");

0 commit comments

Comments
 (0)