Skip to content

Commit 7e0346c

Browse files
lingeswaranTWlingeswaranTW
committed
BAH-4464 Fix for Recurring Appointments Generate Inconsistent Appointment Numbers Across Occurrences
1 parent 2b06021 commit 7e0346c

File tree

3 files changed

+264
-14
lines changed

3 files changed

+264
-14
lines changed

‎api/src/main/java/org/openmrs/module/appointments/service/impl/AppointmentRecurringPatternServiceImpl.java‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,27 +89,48 @@ public AppointmentRecurringPattern update(AppointmentRecurringPattern appointmen
8989
return appointmentRecurringPattern;
9090
}
9191

92-
private void checkAndAssignAppointmentNumber(Appointment appointment) {
93-
if (appointment.getAppointmentNumber() == null) {
94-
AppointmentNumberGenerator appointmentNumberGenerator =
95-
appointmentNumberGeneratorLocator.retrieveAppointmentNumberGenerator();
96-
if (appointmentNumberGenerator == null) {
97-
log.warn("Can not generate appointment number. No generator found");
98-
return;
99-
}
100-
String generateAppointmentNumber = appointmentNumberGenerator.generateAppointmentNumber(appointment);
101-
appointment.setAppointmentNumber(generateAppointmentNumber);
102-
}
103-
}
104-
10592
private void updateAppointmentsDetails(AppointmentRecurringPattern appointmentRecurringPattern, List<Appointment> appointments) {
93+
// Generate or reuse appointment number for the entire recurring series
94+
String sharedAppointmentNumber = getOrGenerateSharedAppointmentNumber(appointments);
95+
10696
appointments.forEach(appointment -> {
107-
checkAndAssignAppointmentNumber(appointment);
97+
// Use the shared appointment number for all appointments
98+
if (appointment.getAppointmentNumber() == null) {
99+
appointment.setAppointmentNumber(sharedAppointmentNumber);
100+
}
108101
setAppointmentAudit(appointment);
109102
appointment.setAppointmentRecurringPattern(appointmentRecurringPattern);
110103
});
111104
}
112105

106+
private String getOrGenerateSharedAppointmentNumber(List<Appointment> appointments) {
107+
if (appointments.isEmpty()) {
108+
return null;
109+
}
110+
111+
// Check if any appointment already has a number (for adding to existing series)
112+
String existingAppointmentNumber = appointments.stream()
113+
.map(Appointment::getAppointmentNumber)
114+
.filter(number -> number != null)
115+
.findFirst()
116+
.orElse(null);
117+
118+
if (existingAppointmentNumber != null) {
119+
return existingAppointmentNumber;
120+
}
121+
122+
// Generate new number for new series
123+
AppointmentNumberGenerator appointmentNumberGenerator =
124+
appointmentNumberGeneratorLocator.retrieveAppointmentNumberGenerator();
125+
126+
if (appointmentNumberGenerator == null) {
127+
log.warn("Can not generate appointment number. No generator found");
128+
return null;
129+
}
130+
131+
return appointmentNumberGenerator.generateAppointmentNumber(appointments.get(0));
132+
}
133+
113134
@Override
114135
public Appointment update(AppointmentRecurringPattern appointmentRecurringPattern,
115136
List<Appointment> updatedAppointments) {

‎api/src/test/java/org/openmrs/module/appointments/service/impl/AppointmentRecurringPatternServiceImplTest.java‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.util.List;
3838
import java.util.Set;
3939

40+
import static org.junit.Assert.assertNull;
4041
import static org.junit.Assert.assertEquals;
4142
import static org.junit.Assert.assertTrue;
4243
import static org.mockito.Matchers.anyString;
@@ -338,6 +339,7 @@ public void shouldAddAuditAppointmentNumberToOnlyUpdatedAppointments() throws IO
338339
verify(appointmentServiceHelper, times(2)).getAppointmentAsJsonString(any(Appointment.class));
339340
verify(appointmentServiceHelper, times(2)).getAppointmentAuditEvent(any(Appointment.class), nullable(String.class));
340341
updatedAppointments.forEach(app -> assertTrue("Should have generated appointment number", app.getAppointmentNumber().startsWith(expectedAppointmentNumberPart)));
342+
assertEquals(voidedAppointment.getAppointmentNumber(), newAppointment.getAppointmentNumber());
341343
assertEquals(newAppointment, appointment);
342344
}
343345

@@ -366,6 +368,128 @@ public void shouldThrowExceptionIfValidationFailsOnSingleAppointmentUpdate() thr
366368
}
367369

368370

371+
@Test
372+
public void shouldAssignSameAppointmentNumberToAllNewRecurringAppointments() throws IOException {
373+
AppointmentRecurringPattern appointmentRecurringPattern = new AppointmentRecurringPattern();
374+
Appointment appointmentOne = new Appointment();
375+
Appointment appointmentTwo = new Appointment();
376+
Appointment appointmentThree = new Appointment();
377+
List<Appointment> appointments = Arrays.asList(appointmentOne, appointmentTwo, appointmentThree);
378+
appointmentRecurringPattern.setAppointments(new HashSet<>(appointments));
379+
380+
String sharedAppointmentNumber = "TEST123";
381+
String notes = "Notes";
382+
AppointmentAudit appointmentAudit = new AppointmentAudit();
383+
384+
when(appointmentNumberGenerator.generateAppointmentNumber(any(Appointment.class)))
385+
.thenReturn(sharedAppointmentNumber);
386+
doReturn(notes).when(appointmentServiceHelper).getAppointmentAsJsonString(any());
387+
doReturn(appointmentAudit).when(appointmentServiceHelper).getAppointmentAuditEvent(any(), anyString());
388+
doNothing().when(appointmentRecurringPatternDao).save(appointmentRecurringPattern);
389+
390+
recurringAppointmentService.setAppointmentNumberGeneratorLocator(
391+
new AppointmentNumberGeneratorLocatorImpl(appointmentNumberGenerator));
392+
AppointmentRecurringPattern savedPattern = recurringAppointmentService
393+
.validateAndSave(appointmentRecurringPattern);
394+
395+
assertEquals(sharedAppointmentNumber, appointmentOne.getAppointmentNumber());
396+
assertEquals(sharedAppointmentNumber, appointmentTwo.getAppointmentNumber());
397+
assertEquals(sharedAppointmentNumber, appointmentThree.getAppointmentNumber());
398+
verify(appointmentNumberGenerator, times(1)).generateAppointmentNumber(any());
399+
}
400+
401+
@Test
402+
public void shouldReuseExistingAppointmentNumberWhenAddingAppointmentsToExistingSeries() throws IOException {
403+
AppointmentRecurringPattern appointmentRecurringPattern = new AppointmentRecurringPattern();
404+
String existingNumber = "EXISTING123";
405+
406+
Appointment existingAppointmentOne = new Appointment();
407+
existingAppointmentOne.setAppointmentNumber(existingNumber);
408+
Appointment existingAppointmentTwo = new Appointment();
409+
existingAppointmentTwo.setAppointmentNumber(existingNumber);
410+
Appointment newAppointment = new Appointment();
411+
newAppointment.setAppointmentNumber(null);
412+
413+
List<Appointment> appointments = Arrays.asList(
414+
existingAppointmentOne, existingAppointmentTwo, newAppointment);
415+
appointmentRecurringPattern.setAppointments(new HashSet<>(appointments));
416+
417+
String notes = "Notes";
418+
AppointmentAudit appointmentAudit = new AppointmentAudit();
419+
doReturn(notes).when(appointmentServiceHelper).getAppointmentAsJsonString(any());
420+
doReturn(appointmentAudit).when(appointmentServiceHelper).getAppointmentAuditEvent(any(), anyString());
421+
doNothing().when(appointmentRecurringPatternDao).save(appointmentRecurringPattern);
422+
423+
recurringAppointmentService.setAppointmentNumberGeneratorLocator(
424+
new AppointmentNumberGeneratorLocatorImpl(appointmentNumberGenerator));
425+
AppointmentRecurringPattern savedPattern = recurringAppointmentService
426+
.validateAndSave(appointmentRecurringPattern);
427+
428+
assertEquals(existingNumber, newAppointment.getAppointmentNumber());
429+
assertEquals(existingNumber, existingAppointmentOne.getAppointmentNumber());
430+
assertEquals(existingNumber, existingAppointmentTwo.getAppointmentNumber());
431+
verify(appointmentNumberGenerator, never()).generateAppointmentNumber(any());
432+
}
433+
434+
@Test
435+
public void shouldPreserveSingleAppointmentNumberWhenAlreadyAssigned() throws IOException {
436+
AppointmentRecurringPattern appointmentRecurringPattern = new AppointmentRecurringPattern();
437+
String preservedNumber = "PRESERVE123";
438+
439+
Appointment appointment = new Appointment();
440+
appointment.setAppointmentNumber(preservedNumber);
441+
442+
List<Appointment> appointments = Collections.singletonList(appointment);
443+
appointmentRecurringPattern.setAppointments(new HashSet<>(appointments));
444+
445+
String notes = "Notes";
446+
AppointmentAudit appointmentAudit = new AppointmentAudit();
447+
doReturn(notes).when(appointmentServiceHelper).getAppointmentAsJsonString(any());
448+
doReturn(appointmentAudit).when(appointmentServiceHelper).getAppointmentAuditEvent(any(), anyString());
449+
doNothing().when(appointmentRecurringPatternDao).save(appointmentRecurringPattern);
450+
451+
recurringAppointmentService.setAppointmentNumberGeneratorLocator(
452+
new AppointmentNumberGeneratorLocatorImpl(appointmentNumberGenerator));
453+
AppointmentRecurringPattern savedPattern = recurringAppointmentService
454+
.validateAndSave(appointmentRecurringPattern);
455+
456+
assertEquals(preservedNumber, appointment.getAppointmentNumber());
457+
verify(appointmentNumberGenerator, never()).generateAppointmentNumber(any());
458+
}
459+
460+
@Test
461+
public void shouldHandleNullGeneratorGracefullyForAllAppointments() throws IOException {
462+
AppointmentRecurringPattern appointmentRecurringPattern = new AppointmentRecurringPattern();
463+
Appointment appointmentOne = new Appointment();
464+
Appointment appointmentTwo = new Appointment();
465+
List<Appointment> appointments = Arrays.asList(appointmentOne, appointmentTwo);
466+
appointmentRecurringPattern.setAppointments(new HashSet<>(appointments));
467+
468+
String notes = "Notes";
469+
AppointmentAudit appointmentAudit = new AppointmentAudit();
470+
doReturn(notes).when(appointmentServiceHelper).getAppointmentAsJsonString(any());
471+
doReturn(appointmentAudit).when(appointmentServiceHelper).getAppointmentAuditEvent(any(), anyString());
472+
doNothing().when(appointmentRecurringPatternDao).save(appointmentRecurringPattern);
473+
474+
recurringAppointmentService.setAppointmentNumberGeneratorLocator(
475+
new AppointmentNumberGeneratorLocatorImpl(null));
476+
AppointmentRecurringPattern savedPattern = recurringAppointmentService
477+
.validateAndSave(appointmentRecurringPattern);
478+
479+
assertNull(appointmentOne.getAppointmentNumber());
480+
assertNull(appointmentTwo.getAppointmentNumber());
481+
}
482+
483+
@Test(expected = IndexOutOfBoundsException.class)
484+
public void shouldThrowExceptionForEmptyAppointmentList() throws IOException {
485+
AppointmentRecurringPattern appointmentRecurringPattern = new AppointmentRecurringPattern();
486+
appointmentRecurringPattern.setAppointments(new HashSet<>());
487+
488+
recurringAppointmentService.setAppointmentNumberGeneratorLocator(
489+
new AppointmentNumberGeneratorLocatorImpl(appointmentNumberGenerator));
490+
recurringAppointmentService.validateAndSave(appointmentRecurringPattern);
491+
}
492+
369493
private Appointment getAppointment(String uuid, Patient patient, AppointmentStatus appointmentStatus,
370494
AppointmentKind appointmentKind, Date start, Date end) {
371495
return new AppointmentBuilder().withUuid(uuid)

‎omod/src/test/java/org/openmrs/module/appointments/web/controller/RecurringAppointmentsControllerIT.java‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static org.junit.Assert.assertEquals;
2828
import static org.junit.Assert.assertNotNull;
2929
import static org.junit.Assert.assertNull;
30+
import static org.junit.Assert.assertTrue;
3031

3132
public class RecurringAppointmentsControllerIT extends BaseIntegrationTest {
3233

@@ -616,4 +617,108 @@ public void shouldUpdateRecurringAppointmentForRequestedAppointmentWhenApplyForA
616617
assertNotNull(response);
617618
assertEquals(200, response.getStatus());
618619
}
620+
621+
@Test
622+
public void shouldAssignSameAppointmentNumberToDailyRecurringAppointments() throws Exception {
623+
String content = "{ \"appointmentRequest\":{" +
624+
"\"providerUuid\": \"823fdcd7-3f10-11e4-adec-0800271c1b75\", " +
625+
"\"patientUuid\": \"2c33920f-7aa6-48d6-998a-60412d8ff7d5\", " +
626+
"\"serviceUuid\": \"c36006d4-9fbb-4f20-866b-0ece245615c1\", " +
627+
"\"startDateTime\": \"2019-05-19T23:45:00.000Z\", " +
628+
"\"endDateTime\": \"2019-05-20T00:15:00.000Z\", " +
629+
"\"appointmentKind\": \"WalkIn\", " +
630+
"\"providers\": []}," +
631+
"\"recurringPattern\":{" +
632+
"\"frequency\":3," +
633+
"\"period\":1," +
634+
"\"daysOfWeek\":[]," +
635+
"\"endDate\":\"\"," +
636+
"\"type\":\"Day\"" +
637+
"}}";
638+
MockHttpServletResponse response = handle(newPostRequest("/rest/v1/recurring-appointments", content));
639+
assertNotNull(response);
640+
assertEquals(200, response.getStatus());
641+
642+
List<RecurringAppointmentDefaultResponse> appointmentResponses = deserialize(response,
643+
new TypeReference<List<RecurringAppointmentDefaultResponse>>() {});
644+
assertEquals(3, appointmentResponses.size());
645+
646+
String appointmentNumber = appointmentResponses.get(0).getAppointmentDefaultResponse().getAppointmentNumber();
647+
assertNotNull(appointmentNumber);
648+
for (RecurringAppointmentDefaultResponse appointmentResponse : appointmentResponses) {
649+
assertEquals(appointmentNumber, appointmentResponse.getAppointmentDefaultResponse().getAppointmentNumber());
650+
}
651+
652+
// Verify appointment numbers are persisted in the database
653+
String firstAppointmentUuid = appointmentResponses.get(0).getAppointmentDefaultResponse().getUuid();
654+
Appointment persistedAppointment = appointmentsService.getAppointmentByUuid(firstAppointmentUuid);
655+
assertNotNull(persistedAppointment.getAppointmentNumber());
656+
assertEquals(appointmentNumber, persistedAppointment.getAppointmentNumber());
657+
}
658+
659+
@Test
660+
public void shouldAssignSameAppointmentNumberToWeeklyRecurringAppointments() throws Exception {
661+
String content = "{ \"appointmentRequest\":{" +
662+
"\"providerUuid\": \"823fdcd7-3f10-11e4-adec-0800271c1b75\", " +
663+
"\"patientUuid\": \"2c33920f-7aa6-48d6-998a-60412d8ff7d5\", " +
664+
"\"serviceUuid\": \"c36006d4-9fbb-4f20-866b-0ece245615c1\", " +
665+
"\"startDateTime\": \"2019-05-19T23:45:00.000Z\", " +
666+
"\"endDateTime\": \"2019-05-20T00:15:00.000Z\", " +
667+
"\"appointmentKind\": \"WalkIn\", " +
668+
"\"providers\": []}," +
669+
"\"recurringPattern\":{" +
670+
"\"period\":1," +
671+
"\"daysOfWeek\":[\"SUNDAY\",\"WEDNESDAY\",\"FRIDAY\"]," +
672+
"\"endDate\":\"2019-05-25T23:45:00.000Z\"," +
673+
"\"type\":\"WEEK\"" +
674+
"}}";
675+
MockHttpServletResponse response = handle(newPostRequest("/rest/v1/recurring-appointments", content));
676+
assertNotNull(response);
677+
assertEquals(200, response.getStatus());
678+
679+
List<RecurringAppointmentDefaultResponse> appointmentResponses = deserialize(response,
680+
new TypeReference<List<RecurringAppointmentDefaultResponse>>() {});
681+
assertTrue(appointmentResponses.size() > 1);
682+
683+
String appointmentNumber = appointmentResponses.get(0).getAppointmentDefaultResponse().getAppointmentNumber();
684+
assertNotNull(appointmentNumber);
685+
for (RecurringAppointmentDefaultResponse appointmentResponse : appointmentResponses) {
686+
assertEquals(appointmentNumber, appointmentResponse.getAppointmentDefaultResponse().getAppointmentNumber());
687+
}
688+
}
689+
690+
@Test
691+
public void shouldPreserveAppointmentNumberWhenIncreasingFrequency() throws Exception {
692+
String content = "{ \"appointmentRequest\":{" +
693+
"\"uuid\": \"c36006e5-9fbb-4f20-866b-0ece245615a7\", " +
694+
"\"appointmentNumber\": \"1\", " +
695+
"\"patientUuid\": \"2c33920f-7aa6-48d6-998a-60412d8ff7d5\", " +
696+
"\"serviceUuid\": \"c36006d4-9fbb-4f20-866b-0ece245615c1\", " +
697+
"\"serviceTypeUuid\": \"672546e5-9fbb-4f20-866b-0ece24564578\", " +
698+
"\"startDateTime\": \"2017-07-20\", " +
699+
"\"endDateTime\": \"2017-07-20\", " +
700+
"\"comments\": \"Some notes\", " +
701+
"\"appointmentKind\": \"WalkIn\"," +
702+
"\"providers\": []}," +
703+
"\"applyForAll\": true," +
704+
"\"timeZone\": \"UTC\"," +
705+
"\"recurringPattern\":{" +
706+
"\"frequency\":4," +
707+
"\"period\":1," +
708+
"\"daysOfWeek\":[]," +
709+
"\"type\":\"Day\"" +
710+
"}}";
711+
MockHttpServletResponse response = handle(newPutRequest("/rest/v1/recurring-appointments/c36006e5-9fbb-4f20-866b-0ece245615a7", content));
712+
assertNotNull(response);
713+
assertEquals(200, response.getStatus());
714+
715+
List<RecurringAppointmentDefaultResponse> appointmentResponses = deserialize(response,
716+
new TypeReference<List<RecurringAppointmentDefaultResponse>>() {});
717+
assertEquals(4, appointmentResponses.size());
718+
719+
String expectedAppointmentNumber = "1";
720+
for (RecurringAppointmentDefaultResponse appointmentResponse : appointmentResponses) {
721+
assertEquals(expectedAppointmentNumber, appointmentResponse.getAppointmentDefaultResponse().getAppointmentNumber());
722+
}
723+
}
619724
}

0 commit comments

Comments
 (0)