Skip to content

Commit

Permalink
AP-46 | Appointment fixes (Bahmni#23)
Browse files Browse the repository at this point in the history 
* Sowmika, Vinay | MOBN-877 | Handle conflicts for services with multiple slots and with no start or end time

* Sowmika, Vinay | MOBN-875 | Send NO_CONTENT response when there are no appointmnets to save

* Alekhya | MOBN-877 | Handle service time in negative milliseconds

* Sowmika | MOBN-875 | Retain the order of appointments using LinkedHashSet

* Alekhya | MOBN-907 | Clone Appointment object in conflicts flow

* Alekhya | MOBN-906 | Remove Id in cloning appointment providers

* Alekhya, Siva | MOBN-904 | Remove transactional annotation for conflicts methods

* Sowmika, Alekhya | MOBN-906 | Revert - 'Sowmika, Sneha, Vineela | MOBN-498 | Filter out voided appointment providers in appointments' commit

* Sowmika, Alekhya | MOBN-906 | Void cancelled response providers

* Alekhya | MOBN-914 | Null check when edited appointment is null

* Alekhya | MOBN-914 | Refactor

* Alekhya | MOBN-914 | Throw api exception when end date is before start date

* Alekhya | MOBN-906 | Clone the new providers

* Alekhya | MOBN-966 | Include Appointment id while cloning appointment in conflicts flow

* Alekhya | MOBN-969 | Fix extension of weekly recurring appointments when only one week day is selected

Co-authored-by: sowmika148 <[email protected]>
Co-authored-by: Alekhya Yalla <[email protected]>
  • Loading branch information
@vinay-ThoughtWorks @sowmika148
@AlekhyaYalla @mddubey
3 people authored and mddubey committed Jan 7, 2020
1 parent 2737277 commit cded3a3
Show file tree
Hide file tree
Showing 25 changed files with 475 additions and 154 deletions.
27 changes: 15 additions & 12 deletions .../openmrs/module/appointments/conflicts/impl/AppointmentServiceUnavailabilityConflict.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.openmrs.module.appointments.model.Appointment;
import org.openmrs.module.appointments.model.AppointmentServiceDefinition;
import org.openmrs.module.appointments.model.ServiceWeeklyAvailability;
import org.openmrs.module.appointments.util.DateUtil;

import java.sql.Time;
import java.text.SimpleDateFormat;
Expand All @@ -14,6 +15,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import static org.openmrs.module.appointments.model.AppointmentConflictType.SERVICE_UNAVAILABLE;
import static org.openmrs.module.appointments.util.DateUtil.getEpochTime;
Expand Down Expand Up @@ -44,28 +46,29 @@ private boolean checkConflicts(Appointment appointment, AppointmentServiceDefini
Set<ServiceWeeklyAvailability> weeklyAvailableDays = appointmentServiceDefinition.getWeeklyAvailability();
if (isObjectPresent(weeklyAvailableDays)) {
String appointmentDay = DayFormat.format(appointment.getStartDateTime());
Optional<ServiceWeeklyAvailability> dayAvailability = weeklyAvailableDays.stream()
.filter(day -> day.isSameDay(appointmentDay)).findFirst();
if (dayAvailability.isPresent()) {
ServiceWeeklyAvailability availableDay = dayAvailability.get();
return checkTimeAvailability(appointment, availableDay.getStartTime(), availableDay.getEndTime());
}
List<ServiceWeeklyAvailability> dayAvailabilities = weeklyAvailableDays.stream()
.filter(day -> day.isSameDay(appointmentDay)).collect(Collectors.toList());
if (!dayAvailabilities.isEmpty())
return dayAvailabilities.stream().allMatch(availableDay ->
checkTimeAvailability(appointment, availableDay.getStartTime().getTime(), availableDay.getEndTime().getTime()));
return true;
}
return checkTimeAvailability(appointment,
appointmentServiceDefinition.getStartTime(), appointmentServiceDefinition.getEndTime());

Time serviceStartTime = appointmentServiceDefinition.getStartTime();
Time serviceEndTime = appointmentServiceDefinition.getEndTime();
long serviceStartMillis = serviceStartTime != null ? serviceStartTime.getTime() : DateUtil.getStartOfDay().getTime();
long serviceEndMillis = serviceEndTime != null ? serviceEndTime.getTime() : DateUtil.getEndOfDay().getTime();
return checkTimeAvailability(appointment, serviceStartMillis, serviceEndMillis);
}

private boolean isObjectPresent(Collection<?> object) {
return Objects.nonNull(object) && !object.isEmpty();
}

private boolean checkTimeAvailability(Appointment appointment, Time serviceStartTime, Time serviceEndTime) {
private boolean checkTimeAvailability(Appointment appointment, long serviceStartTime, long serviceEndTime) {
long appointmentStartTimeMilliSeconds = getEpochTime(appointment.getStartDateTime().getTime());
long appointmentEndTimeMilliSeconds = getEpochTime(appointment.getEndDateTime().getTime());
long serviceStartTimeMilliSeconds = getEpochTime(serviceStartTime.getTime());
long serviceEndTimeMilliSeconds = getEpochTime(serviceEndTime.getTime());
long serviceStartTimeMilliSeconds = getEpochTime(serviceStartTime);
long serviceEndTimeMilliSeconds = getEpochTime(serviceEndTime);
boolean isConflict = (appointmentStartTimeMilliSeconds >= appointmentEndTimeMilliSeconds)
|| ((appointmentStartTimeMilliSeconds < serviceStartTimeMilliSeconds)
|| (appointmentEndTimeMilliSeconds > serviceEndTimeMilliSeconds));
Expand Down
3 changes: 2 additions & 1 deletion api/src/main/java/org/openmrs/module/appointments/helper/AppointmentServiceHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

@Component
public class AppointmentServiceHelper {
Expand Down Expand Up @@ -61,7 +62,7 @@ public String getAppointmentAsJsonString(Appointment appointment) throws IOExcep

private void validateAppointment(Appointment appointment, List<AppointmentValidator> appointmentValidators,
List<String> errors) {
if(!CollectionUtils.isEmpty(appointmentValidators)) {
if(!CollectionUtils.isEmpty(appointmentValidators) && Objects.nonNull(appointment)) {
for (AppointmentValidator validator : appointmentValidators) {
validator.validate(appointment, errors);
}
Expand Down
1 change: 0 additions & 1 deletion api/src/main/java/org/openmrs/module/appointments/model/AppointmentProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ public class AppointmentProvider extends BaseOpenmrsData implements Serializable
private Boolean voided;

public AppointmentProvider(AppointmentProvider appointmentProvider) {
this.appointmentProviderId = appointmentProvider.getAppointmentProviderId();
this.appointment = appointmentProvider.getAppointment();
this.provider = appointmentProvider.getProvider();
this.response = appointmentProvider.getResponse();
Expand Down
4 changes: 2 additions & 2 deletions api/src/main/java/org/openmrs/module/appointments/model/AppointmentRecurringPattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import org.openmrs.module.appointments.service.impl.RecurringAppointmentType;

import java.util.Date;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;
import java.util.stream.Collectors;

Expand All @@ -15,7 +15,7 @@ public class AppointmentRecurringPattern {
private Date endDate;
private RecurringAppointmentType type;
private String daysOfWeek;
private Set<Appointment> appointments = new HashSet<>();
private Set<Appointment> appointments = new LinkedHashSet<>();

public Integer getId() {
return id;
Expand Down
2 changes: 0 additions & 2 deletions api/src/main/java/org/openmrs/module/appointments/service/AppointmentsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,9 @@ public interface AppointmentsService {
@Authorized({VIEW_APPOINTMENTS, MANAGE_APPOINTMENTS})
List<Appointment> search(AppointmentSearchRequest appointmentSearchRequest);

@Transactional
@Authorized({VIEW_APPOINTMENTS, MANAGE_APPOINTMENTS})
Map<Enum, List<Appointment>> getAppointmentConflicts(Appointment appointment);

@Transactional
@Authorized({VIEW_APPOINTMENTS, MANAGE_APPOINTMENTS})
Map<Enum, List<Appointment>> getAppointmentsConflicts(List<Appointment> appointments);
}
Expand Down
3 changes: 2 additions & 1 deletion .../org/openmrs/module/appointments/service/impl/AppointmentRecurringPatternServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.TimeZone;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -96,7 +97,7 @@ public Appointment update(AppointmentRecurringPattern appointmentRecurringPatter
List<Appointment> updatedAppointments) {
Appointment editedAppointment = updatedAppointments
.stream()
.filter(app -> app.getVoided() != true)
.filter(app -> !app.getVoided())
.collect(Collectors.toList()).get(0);
updateAppointmentsDetails(appointmentRecurringPattern, updatedAppointments);
appointmentServiceHelper.validate(editedAppointment.getRelatedAppointment(), editAppointmentValidators);
Expand Down
16 changes: 15 additions & 1 deletion api/src/main/java/org/openmrs/module/appointments/service/impl/AppointmentsServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import static org.openmrs.module.appointments.constants.PrivilegeConstants.RESET_APPOINTMENT_STATUS;
import static org.openmrs.module.appointments.util.DateUtil.getStartOfDay;

@Transactional

public class AppointmentsServiceImpl implements AppointmentsService {

private static final String PRIVILEGES_EXCEPTION_CODE = "error.privilegesRequired";
Expand Down Expand Up @@ -105,6 +105,7 @@ private boolean isCurrentUserSamePersonAsOneOfTheAppointmentProviders(Set<Appoin
equals(Context.getAuthenticatedUser().getPerson()));
}

@Transactional
@Override
public Appointment validateAndSave(Appointment appointment) throws APIException {
validate(appointment, appointmentValidators);
Expand All @@ -118,6 +119,7 @@ private void save(Appointment appointment) {
appointmentDao.save(appointment);
}

@Transactional
@Override
public void validate(Appointment appointment, List<AppointmentValidator> appointmentValidators) {
if (!validateIfUserHasSelfOrAllAppointmentsAccess(appointment)) {
Expand All @@ -127,6 +129,7 @@ public void validate(Appointment appointment, List<AppointmentValidator> appoint
appointmentServiceHelper.validate(appointment, appointmentValidators);
}

@Transactional
@Override
public List<Appointment> getAllAppointments(Date forDate) {
List<Appointment> appointments = appointmentDao.getAllAppointments(forDate);
Expand All @@ -144,33 +147,39 @@ private boolean isServiceOrServiceTypeVoided(Appointment appointment) {
* @param appointment
* @return
*/
@Transactional
@Override
public List<Appointment> search(Appointment appointment) {
List<Appointment> appointments = appointmentDao.search(appointment);
return appointments.stream().filter(searchedAppointment -> !isServiceOrServiceTypeVoided(searchedAppointment)).collect(Collectors.toList());
}

@Transactional
@Override
public List<Appointment> getAllFutureAppointmentsForService(AppointmentServiceDefinition appointmentServiceDefinition) {
return appointmentDao.getAllFutureAppointmentsForService(appointmentServiceDefinition);
}

@Transactional
@Override
public List<Appointment> getAllFutureAppointmentsForServiceType(AppointmentServiceType appointmentServiceType) {
return appointmentDao.getAllFutureAppointmentsForServiceType(appointmentServiceType);
}

@Transactional
@Override
public List<Appointment> getAppointmentsForService(AppointmentServiceDefinition appointmentServiceDefinition, Date startDate, Date endDate, List<AppointmentStatus> appointmentStatusList) {
return appointmentDao.getAppointmentsForService(appointmentServiceDefinition, startDate, endDate, appointmentStatusList);
}

@Transactional
@Override
public Appointment getAppointmentByUuid(String uuid) {
Appointment appointment = appointmentDao.getAppointmentByUuid(uuid);
return appointment;
}

@Transactional
@Override
public void changeStatus(Appointment appointment, String status, Date onDate) throws APIException {
AppointmentStatus appointmentStatus = AppointmentStatus.valueOf(status);
Expand Down Expand Up @@ -200,12 +209,14 @@ private boolean isUserAllowedToResetStatus(AppointmentStatus toStatus, Appointme
return Context.hasPrivilege(RESET_APPOINTMENT_STATUS);
}

@Transactional
@Override
public List<Appointment> getAllAppointmentsInDateRange(Date startDate, Date endDate) {
List<Appointment> appointments = appointmentDao.getAllAppointmentsInDateRange(startDate, endDate);
return appointments.stream().filter(appointment -> !isServiceOrServiceTypeVoided(appointment)).collect(Collectors.toList());
}

@Transactional
@Override
public void undoStatusChange(Appointment appointment) throws APIException {
if (!validateIfUserHasSelfOrAllAppointmentsAccess(appointment)) {
Expand All @@ -221,6 +232,7 @@ public void undoStatusChange(Appointment appointment) throws APIException {
throw new APIException("No status change actions to undo");
}

@Transactional
@Override
public List<Appointment> search(AppointmentSearchRequest appointmentSearchRequest) {
if (isNull(appointmentSearchRequest.getStartDate())) {
Expand Down Expand Up @@ -257,6 +269,7 @@ private List<Appointment> getNonVoidedFutureAppointments(List<Appointment> appoi
}).collect(Collectors.toList());
}

@Transactional
@Override
public void updateAppointmentProviderResponse(AppointmentProvider providerWithNewResponse) {
Appointment appointment = providerWithNewResponse.getAppointment();
Expand Down Expand Up @@ -293,6 +306,7 @@ private void validateProviderResponseForSelf(AppointmentProvider appointmentProv
}
}

@Transactional
@Override
public Appointment reschedule(String originalAppointmentUuid, Appointment newAppointment, boolean retainAppointmentNumber) {
Appointment prevAppointment = getAppointmentByUuid(originalAppointmentUuid);
Expand Down
22 changes: 14 additions & 8 deletions api/src/main/java/org/openmrs/module/appointments/util/DateUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,22 @@ public static Date getStartOfDay() {
}

public static long getEpochTime(long date) {
long milliSeconds = 0;
if (date > 0) {
Calendar calendar = getCalendar(new Date(date));
int hours = calendar.get(Calendar.HOUR_OF_DAY);
int minutes = calendar.get(Calendar.MINUTE);
int seconds = calendar.get(Calendar.SECOND);
milliSeconds = (hours * 3600 + minutes * 60 + seconds) * 1000;
}
Calendar calendar = getCalendar(new Date(date));
int hours = calendar.get(Calendar.HOUR_OF_DAY);
int minutes = calendar.get(Calendar.MINUTE);
int seconds = calendar.get(Calendar.SECOND);
long milliSeconds = ((hours * 3600 + minutes * 60 + seconds) * 1000);
return milliSeconds;
}

public static Date getEndOfDay() {
Calendar calendar = Calendar.getInstance();
calendar.set(Calendar.HOUR_OF_DAY, calendar.getMaximum(Calendar.HOUR_OF_DAY));
calendar.set(Calendar.MINUTE, calendar.getMaximum(Calendar.MINUTE));
calendar.set(Calendar.SECOND, calendar.getMaximum(Calendar.SECOND));
calendar.set(Calendar.MILLISECOND, calendar.getMaximum(Calendar.MILLISECOND));
return calendar.getTime();
}
}


67 changes: 66 additions & 1 deletion ...nmrs/module/appointments/conflicts/impl/AppointmentServiceUnavailabilityConflictTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,77 @@ public void shouldReturnServiceUnavailableTimeSlotConflict() {
conflictingAppointments.addAll(appointmentServiceUnavailabilityConflict.getConflicts(Arrays.asList(appointmentOne, appointmentTwo, appointmentThree)));

assertNotNull(conflictingAppointments);
assertEquals(3,conflictingAppointments.size());
assertEquals(3, conflictingAppointments.size());
assertEquals(appointmentOne, conflictingAppointments.get(0));
assertEquals(appointmentTwo, conflictingAppointments.get(1));
assertEquals(appointmentThree, conflictingAppointments.get(2));
}

@Test
public void shouldNotReturnServiceUnavailableConflictsForMoreSlotsInSingleDay() {
AppointmentServiceDefinition appointmentServiceDefinition = new AppointmentServiceDefinition();
// All Appointments are on Monday
Appointment appointmentOne = new Appointment();
appointmentOne.setService(appointmentServiceDefinition);
appointmentOne.setStartDateTime(getDate(2019, 8, 23, 6, 30, 0));
appointmentOne.setEndDateTime(getDate(2019, 8, 23, 7, 0, 0));
appointmentOne.setAppointmentId(2);
Appointment appointmentTwo = new Appointment();
appointmentTwo.setService(appointmentServiceDefinition);
appointmentTwo.setStartDateTime(getDate(2019, 8, 23, 16, 30, 0));
appointmentTwo.setEndDateTime(getDate(2019, 8, 23, 17, 30, 0));
appointmentTwo.setAppointmentId(3);
Appointment appointmentThree = new Appointment();
appointmentThree.setService(appointmentServiceDefinition);
appointmentThree.setStartDateTime(getDate(2019, 8, 23, 16, 30, 0));
appointmentThree.setEndDateTime(getDate(2019, 8, 23, 17, 0, 0));
appointmentThree.setAppointmentId(4);
ServiceWeeklyAvailability day1 = new ServiceWeeklyAvailability();
day1.setStartTime(new Time(6, 30, 0));
day1.setEndTime(new Time(14, 0, 0));
day1.setDayOfWeek(DayOfWeek.MONDAY);
ServiceWeeklyAvailability day2 = new ServiceWeeklyAvailability();
day2.setStartTime(new Time(16, 30, 0));
day2.setEndTime(new Time(19, 0, 0));
day2.setDayOfWeek(DayOfWeek.MONDAY);
Set<ServiceWeeklyAvailability> availabilities = new HashSet<>(Arrays.asList(day1, day2));
appointmentServiceDefinition.setWeeklyAvailability(availabilities);

List<Appointment> conflictingAppointments = new ArrayList<>();
conflictingAppointments.addAll(appointmentServiceUnavailabilityConflict.getConflicts(Arrays.asList(appointmentOne, appointmentTwo, appointmentThree)));

assertNotNull(conflictingAppointments);
assertEquals(0, conflictingAppointments.size());
}

@Test
public void shouldNotReturnServiceUnavailableConflictsForServicesWithNoAvailabilityInformation() {
AppointmentServiceDefinition appointmentServiceDefinition = new AppointmentServiceDefinition();
Appointment appointmentOne = new Appointment();
appointmentOne.setService(appointmentServiceDefinition);
appointmentOne.setStartDateTime(getDate(2019, 8, 23, 6, 30, 0));
appointmentOne.setEndDateTime(getDate(2019, 8, 23, 7, 0, 0));
appointmentOne.setAppointmentId(2);
Appointment appointmentTwo = new Appointment();
appointmentTwo.setService(appointmentServiceDefinition);
appointmentTwo.setStartDateTime(getDate(2019, 8, 23, 16, 30, 0));
appointmentTwo.setEndDateTime(getDate(2019, 8, 23, 17, 30, 0));
appointmentTwo.setAppointmentId(3);
Appointment appointmentThree = new Appointment();
appointmentThree.setService(appointmentServiceDefinition);
appointmentThree.setStartDateTime(getDate(2019, 8, 23, 16, 30, 0));
appointmentThree.setEndDateTime(getDate(2019, 8, 23, 17, 0, 0));
appointmentThree.setAppointmentId(4);
Set<ServiceWeeklyAvailability> availabilities = new HashSet<>();
appointmentServiceDefinition.setWeeklyAvailability(availabilities);

List<Appointment> conflictingAppointments = new ArrayList<>();
conflictingAppointments.addAll(appointmentServiceUnavailabilityConflict.getConflicts(Arrays.asList(appointmentOne, appointmentTwo, appointmentThree)));

assertNotNull(conflictingAppointments);
assertEquals(0, conflictingAppointments.size());
}

@Test
public void shouldReturnConflictWhenAppointmentStartTimeAfterEndTime() {
AppointmentServiceDefinition appointmentServiceDefinition = new AppointmentServiceDefinition();
Expand Down
9 changes: 9 additions & 0 deletions api/src/test/java/org/openmrs/module/appointments/helper/AppointmentServiceHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,13 @@ public void shouldThrowExceptionForInapplicableStatusChange() {
any(AppointmentStatus.class),
anyListOf(String.class));
}

@Test
public void shouldNotCallValidateWhenAppointmentIsNull(){
List<AppointmentValidator> appointmentValidators = Collections.singletonList(appointmentValidator);
appointmentServiceHelper.validate(null, appointmentValidators);

verify(appointmentValidator, never()).validate(any(Appointment.class),
anyListOf(String.class));
}
}
11 changes: 10 additions & 1 deletion api/src/test/java/org/openmrs/module/appointments/util/DateUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ public void shouldReturnStartOfDay() {
assertEquals("00:00:00", time);
}

@Test
public void shouldReturnEndOfDay() {
Date date = DateUtil.getEndOfDay();
SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");
String time = dateFormat.format(date);

assertEquals("23:59:59", time);
}

@Test
public void shouldConvertDateToMilliSeconds() throws ParseException {
String dateString = "2017-03-15T16:57:09.0Z";
Expand All @@ -85,6 +94,6 @@ public void shouldConvertDateToMilliSeconds() throws ParseException {
@Test
public void shouldReturnZeroWhenPassedLongIsNegative() {
long milliSeconds = getEpochTime(-10000);
assertEquals(0, milliSeconds);
assertEquals(19790000, milliSeconds);
}
}
Loading

0 comments on commit cded3a3

Please sign in to comment.