diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManager.java index ee5c23bcae..7e38b5773d 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManager.java @@ -94,6 +94,7 @@ import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.dao.associations.InvalidUserAssociationTokenException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; +import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import uk.ac.cam.cl.dtg.util.PropertiesLoader; /** @@ -115,6 +116,7 @@ public class EventBookingManager { private final GroupManager groupManager; private final IUserAccountManager userAccountManager; private final ITransactionManager transactionManager; + private final GitContentManager contentManager; /** * EventBookingManager. @@ -127,6 +129,7 @@ public class EventBookingManager { * @param userAccountManager Instance of User Account Manager, for retrieving users * @param transactionManager Instance of Transaction Manager, used for locking database while managing * bookings + * @param contentManager Instance of Content Manager, for retrieving event content */ @Inject public EventBookingManager(final EventBookingPersistenceManager bookingPersistenceManager, @@ -135,7 +138,8 @@ public EventBookingManager(final EventBookingPersistenceManager bookingPersisten final PropertiesLoader propertiesLoader, final GroupManager groupManager, final IUserAccountManager userAccountManager, - final ITransactionManager transactionManager) { + final ITransactionManager transactionManager, + final GitContentManager contentManager) { this.bookingPersistenceManager = bookingPersistenceManager; this.emailManager = emailManager; this.userAssociationManager = userAssociationManager; @@ -143,6 +147,7 @@ public EventBookingManager(final EventBookingPersistenceManager bookingPersisten this.groupManager = groupManager; this.userAccountManager = userAccountManager; this.transactionManager = transactionManager; + this.contentManager = contentManager; } /** @@ -945,11 +950,19 @@ private Integer getPlacesAvailable(final IsaacEventPageDTO event, final boolean if (isStudentEvent) { // For Student events we only limit the number of students, other roles do not count against the capacity Integer studentCount = roleCounts.getOrDefault(Role.STUDENT, 0); - return Math.max(0, numberOfPlaces - studentCount); + Integer placesAvailable = Math.max(0, numberOfPlaces - studentCount); + + log.info("Event {} capacity: total={}, studentBooked={}, available={}, includeDeleted={}", + event.getId(), numberOfPlaces, studentCount, placesAvailable, includeDeletedUsersInCounts); + return placesAvailable; } else { // For other events, count all roles Integer totalBooked = roleCounts.values().stream().reduce(0, Integer::sum); - return Math.max(0, numberOfPlaces - totalBooked); + Integer placesAvailable = Math.max(0, numberOfPlaces - totalBooked); + log.info("Event {} capacity: total={}, totalBooked={}, available={}, includeDeleted={}", + event.getId(), numberOfPlaces, totalBooked, placesAvailable, includeDeletedUsersInCounts); + + return placesAvailable; } } @@ -1055,6 +1068,7 @@ public void cancelBooking(final IsaacEventPageDTO event, final RegisteredUserDTO Long reservedById; BookingStatus previousBookingStatus; + DetailedEventBookingDTO oldestValidWaitingListBooking = null; EventBookingDTO updatedWaitingListBooking; try (ITransaction transaction = transactionManager.getTransaction()) { // Obtain an exclusive database lock to lock the booking @@ -1070,16 +1084,49 @@ public void cancelBooking(final IsaacEventPageDTO event, final RegisteredUserDTO // If the status was CONFIRMED or RESERVED then promote the oldest booking from the waiting list, if one exists // WAITING_LIST bookings can also be cancelled but do not trigger promotion if (previousBookingStatus == BookingStatus.CONFIRMED || previousBookingStatus == BookingStatus.RESERVED) { + // Use same logic as capacity calculation for consistency: include deleted users only if event is in the past + boolean includeDeletedUsersInWaitingList = event.getDate() != null && event.getDate().isBefore(Instant.now()); + List waitingListBookings = this.bookingPersistenceManager.adminGetBookingsByEventIdAndStatus(event.getId(), - BookingStatus.WAITING_LIST); + BookingStatus.WAITING_LIST, includeDeletedUsersInWaitingList); if (!waitingListBookings.isEmpty()) { - DetailedEventBookingDTO oldestWaitingListBooking = - Collections.min(waitingListBookings, Comparator.comparing(EventBookingDTO::getBookingDate)); - updatedWaitingListBooking = this.bookingPersistenceManager.updateBookingStatus(transaction, event.getId(), - oldestWaitingListBooking.getUserBooked().getId(), BookingStatus.CONFIRMED, - oldestWaitingListBooking.getAdditionalInformation()); + log.info("Event {} has {} users on waiting list after cancellation. Attempting auto-promotion.", + event.getId(), waitingListBookings.size()); + + // Filter out deleted users - try to get each user's details + // If getUserDTOById throws NoUserException, the user is deleted/not found + for (DetailedEventBookingDTO booking : waitingListBookings.stream() + .sorted(Comparator.comparing(EventBookingDTO::getBookingDate)) + .toList()) { + try { + userAccountManager.getUserDTOById(booking.getUserBooked().getId()); + // User exists and is not deleted - this is our candidate + oldestValidWaitingListBooking = booking; + break; + } catch (NoUserException e) { + // User deleted or not found - skip to next + log.debug("Skipping deleted/not found user {} on waiting list for event {}", + booking.getUserBooked().getId(), event.getId()); + } + } + + if (oldestValidWaitingListBooking != null) { + log.info("Promoting user {} from waiting list for event {}", + oldestValidWaitingListBooking.getUserBooked().getId(), event.getId()); + + updatedWaitingListBooking = this.bookingPersistenceManager.updateBookingStatus(transaction, event.getId(), + oldestValidWaitingListBooking.getUserBooked().getId(), BookingStatus.CONFIRMED, + oldestValidWaitingListBooking.getAdditionalInformation()); + } else { + log.info( + "Event {} has no eligible waiting list users to promote after cancellation (all deleted/not found)", + event.getId()); + updatedWaitingListBooking = null; + } } else { + log.info("Event {} has no eligible waiting list users to promote after cancellation (includeDeleted={})", + event.getId(), includeDeletedUsersInWaitingList); updatedWaitingListBooking = null; } } else { @@ -1102,8 +1149,8 @@ public void cancelBooking(final IsaacEventPageDTO event, final RegisteredUserDTO + "student cancellation email was not sent", reservedById); } - if (updatedWaitingListBooking != null) { - Long promotedBookingUserId = updatedWaitingListBooking.getUserBooked().getId(); + if (updatedWaitingListBooking != null && oldestValidWaitingListBooking != null) { + Long promotedBookingUserId = oldestValidWaitingListBooking.getUserBooked().getId(); try { RegisteredUserDTO promotedBookingUser = userAccountManager.getUserDTOById(promotedBookingUserId); @@ -1612,4 +1659,55 @@ public Set getCompetitionProjectTitlesForUsers(String competitionId, Lis .collect(Collectors.toSet()); } + /** + * Cancel all expired RESERVED bookings, triggering waiting list promotion for each. + * This method is called by the scheduled job to handle reservations that haven't been confirmed within the timeout. + * + * @throws SegueDatabaseException if an error occurs. + */ + public void cancelExpiredReservations() throws SegueDatabaseException { + try { + List expiredReservations = bookingPersistenceManager.getExpiredReservations(); + + if (expiredReservations.isEmpty()) { + log.debug("No expired reservations found to cancel"); + return; + } + + log.info("Found {} expired reservations to cancel", expiredReservations.size()); + + for (DetailedEventBookingDTO booking : expiredReservations) { + try { + IsaacEventPageDTO event = (IsaacEventPageDTO) contentManager.getContentById(booking.getEventId()); + if (event == null) { + log.warn("Event {} for expired reservation not found, skipping", booking.getEventId()); + continue; + } + + RegisteredUserDTO user = (RegisteredUserDTO) userAccountManager.getUserDTOById( + booking.getUserBooked().getId()); + + cancelBooking(event, user); + log.debug("Successfully cancelled expired reservation for user {} on event {}", + booking.getUserBooked().getId(), booking.getEventId()); + } catch (NoUserException e) { + log.warn("User {} for expired reservation not found, skipping: {}", + booking.getUserBooked().getId(), e.getMessage()); + // Continue processing remaining expired reservations + } catch (ContentManagerException e) { + log.warn("Content manager error processing expired reservation for user {} on event {}: {}", + booking.getUserBooked().getId(), booking.getEventId(), e.getMessage(), e); + // Continue processing remaining expired reservations + } catch (Exception e) { + log.warn("Failed to cancel expired reservation for user {} on event {}: {}", + booking.getUserBooked().getId(), booking.getEventId(), e.getMessage(), e); + // Continue processing remaining expired reservations + } + } + } catch (SegueDatabaseException e) { + log.error("Error retrieving expired reservations", e); + throw e; + } + } + } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java index 3969bc634a..1ad3aecc06 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java @@ -216,6 +216,23 @@ public List adminGetBookingsByEventId(final String even public List adminGetBookingsByEventIdAndStatus(final String eventId, final BookingStatus status) throws SegueDatabaseException { + return adminGetBookingsByEventIdAndStatus(eventId, status, false); + } + + /** + * Get event bookings by an event id and status with control over deleted user inclusion. + * WARNING: This pulls PII such as dietary info, email, and other stuff that should not (always) make it to end users. + * + * @param eventId of interest + * @param status of interest + * @param includeDeletedUsers if true, include bookings from deleted users; if false, exclude them + * @return event bookings + * @throws SegueDatabaseException if an error occurs. + */ + public List adminGetBookingsByEventIdAndStatus(final String eventId, + final BookingStatus status, + final boolean includeDeletedUsers) + throws SegueDatabaseException { try { ContentDTO c = this.contentManager.getContentById(eventId); @@ -224,7 +241,8 @@ public List adminGetBookingsByEventIdAndStatus(final St } if (c instanceof IsaacEventPageDTO eventPageDTO) { - return this.convertToDTO(Lists.newArrayList(dao.findAllByEventIdAndStatus(eventId, status)), eventPageDTO); + return this.convertToDTO(Lists.newArrayList(dao + .findAllByEventIdAndStatus(eventId, status, includeDeletedUsers)), eventPageDTO); } else { log.error(EXCEPTION_MESSAGE_NOT_EVENT); throw new SegueDatabaseException(EXCEPTION_MESSAGE_NOT_EVENT); @@ -491,4 +509,14 @@ public List getBookingsByEventIdForUsers(String competi throws SegueDatabaseException { return this.getBookingByEventIdAndUsersId(competitionId, userIds); } + + /** + * Get all RESERVED bookings that have expired. + * + * @return list of expired reservations + * @throws SegueDatabaseException if an error occurs. + */ + public List getExpiredReservations() throws SegueDatabaseException { + return convertToDTO(Lists.newArrayList(dao.findExpiredReservations())); + } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java index b2f30634b1..720fefb832 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java @@ -146,6 +146,19 @@ Map> getEventBookingStatusCounts(String eventI Iterable findAllByEventIdAndStatus(String eventId, @Nullable BookingStatus status) throws SegueDatabaseException; + /** + * Find all bookings for a given event with a given status, with control over deleted user inclusion. + * + * @param eventId the event of interest. + * @param status - The event status that should match in the bookings returned. + * @param includeDeletedUsers if true, include bookings from deleted users; if false, exclude them. + * @return an iterable with all the events matching the criteria. + * @throws SegueDatabaseException - if an error occurs. + */ + Iterable findAllByEventIdAndStatus(String eventId, @Nullable BookingStatus status, + boolean includeDeletedUsers) + throws SegueDatabaseException; + /** * Find all bookings for a given event. * @@ -200,4 +213,12 @@ Iterable findAllByEventIdAndStatus(String eventId, @Nullable Booki * @param userId - user id */ void deleteAdditionalInformation(Long userId) throws SegueDatabaseException; + + /** + * Find all RESERVED bookings that have expired (reservationCloseDate has passed). + * + * @return an iterable with all expired reservations. + * @throws SegueDatabaseException - if an error occurs. + */ + Iterable findExpiredReservations() throws SegueDatabaseException; } \ No newline at end of file diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java index e6fc5920d4..f2432c5414 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java @@ -54,6 +54,7 @@ *
* Postgres aware EventBookings. */ +@SuppressWarnings("checkstyle:InvalidJavadocPosition") public class PgEventBookings implements EventBookings { private static final Logger log = LoggerFactory.getLogger(PgEventBookings.class); @@ -634,11 +635,34 @@ public Map> getEventBookingStatusCounts(final @Override public Iterable findAllByEventIdAndStatus(final String eventId, @Nullable final BookingStatus status) throws SegueDatabaseException { + return findAllByEventIdAndStatus(eventId, status, false); + } + + /** + * Find all bookings for a given event with a given status. + *
+ * Useful for finding all on a waiting list or confirmed. + * + * @param eventId the event of interest. + * @param status The event status that should match in the bookings returned. Can be null + * @param includeDeletedUsers if true, include bookings from deleted users; if false, exclude them + * @return an iterable with all the events matching the criteria. + * @throws SegueDatabaseException if an error occurs. + */ + @Override + public Iterable findAllByEventIdAndStatus( + final String eventId, + @Nullable final BookingStatus status, + final boolean includeDeletedUsers) + throws SegueDatabaseException { Validate.notBlank(eventId); StringBuilder sb = new StringBuilder(); - sb.append("SELECT event_bookings.* FROM event_bookings JOIN users ON users.id=user_id WHERE event_id=?" - + " AND NOT users.deleted"); + sb.append("SELECT event_bookings.* FROM event_bookings JOIN users ON users.id=user_id WHERE event_id=?"); + + if (!includeDeletedUsers) { + sb.append(" AND NOT users.deleted"); + } if (status != null) { sb.append(" AND status = ?"); @@ -767,6 +791,35 @@ public Iterable findAllReservationsByUserId(final Long userId) thr * @return a new PgEventBooking * @throws SQLException if an error occurs. */ + /** + * Find all bookings that are RESERVED and have expired reservation close dates. + * + * @return an iterable with all expired reservations. + * @throws SegueDatabaseException if an error occurs. + */ + @Override + public Iterable findExpiredReservations() throws SegueDatabaseException { + String query = "SELECT event_bookings.* FROM event_bookings " + + "WHERE status = ? " + + "AND (additional_booking_information->>'reservationCloseDate')::timestamptz < NOW()"; + + try (Connection conn = ds.getDatabaseConnection(); + PreparedStatement pst = conn.prepareStatement(query) + ) { + pst.setString(1, BookingStatus.RESERVED.name()); + + try (ResultSet results = pst.executeQuery()) { + List returnResult = Lists.newArrayList(); + while (results.next()) { + returnResult.add(buildPgEventBooking(results)); + } + return returnResult; + } + } catch (SQLException e) { + throw new SegueDatabaseException(EXCEPTION_MESSAGE_POSTGRES_ERROR, e); + } + } + private PgEventBooking buildPgEventBooking(final ResultSet results) throws SQLException, SegueDatabaseException { return new PgEventBooking( results.getLong("id"), diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java index 25fa9eae8b..98b6ec99dd 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/configuration/SegueGuiceConfigurationModule.java @@ -177,6 +177,7 @@ import uk.ac.cam.cl.dtg.segue.scheduler.SegueJobService; import uk.ac.cam.cl.dtg.segue.scheduler.SegueScheduledDatabaseScriptJob; import uk.ac.cam.cl.dtg.segue.scheduler.SegueScheduledJob; +import uk.ac.cam.cl.dtg.segue.scheduler.jobs.CancelExpiredReservationsJob; import uk.ac.cam.cl.dtg.segue.scheduler.jobs.DeleteEventAdditionalBookingInformationJob; import uk.ac.cam.cl.dtg.segue.scheduler.jobs.DeleteEventAdditionalBookingInformationOneYearJob; import uk.ac.cam.cl.dtg.segue.scheduler.jobs.EventFeedbackEmailJob; @@ -1002,11 +1003,14 @@ private static SegueJobService getSegueJobService(final PropertiesLoader propert "SQL scheduled job that deletes old AnonymousUsers", CRON_STRING_0230_DAILY, "db_scripts/scheduled/anonymous-user-clean-up.sql"); - SegueScheduledJob cleanUpExpiredReservations = new SegueScheduledDatabaseScriptJob( + SegueScheduledJob cleanUpExpiredReservations = SegueScheduledJob.createCustomJob( "cleanUpExpiredReservations", - CRON_GROUP_NAME_SQL_MAINTENANCE, - "SQL scheduled job that deletes expired reservations for the event booking system", - CRON_STRING_0700_DAILY, "db_scripts/scheduled/expired-reservations-clean-up.sql"); + CRON_GROUP_NAME_JAVA_JOB, + "Java scheduled job that cancels expired reservations and promotes waiting list users", + CRON_STRING_0700_DAILY, + Maps.newHashMap(), + new CancelExpiredReservationsJob() + ); SegueScheduledJob deleteEventAdditionalBookingInformation = SegueScheduledJob.createCustomJob( "deleteEventAdditionalBookingInformation", diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java b/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java index 2b8e54e884..26d5b2a2d5 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/etl/ContentIndexer.java @@ -408,8 +408,11 @@ private Content augmentChildContent(final Content content, final String canonica // hack to get cards to count as children: if (content instanceof IsaacCardDeck) { - for (IsaacCard card : ((IsaacCardDeck) content).getCards()) { - this.augmentChildContent(card, canonicalSourceFile, newParentId, parentPublished); + IsaacCardDeck deck = (IsaacCardDeck) content; + if (deck.getCards() != null) { + for (IsaacCard card : deck.getCards()) { + this.augmentChildContent(card, canonicalSourceFile, newParentId, parentPublished); + } } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/CancelExpiredReservationsJob.java b/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/CancelExpiredReservationsJob.java new file mode 100644 index 0000000000..ed9b4084d0 --- /dev/null +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/scheduler/jobs/CancelExpiredReservationsJob.java @@ -0,0 +1,36 @@ +package uk.ac.cam.cl.dtg.segue.scheduler.jobs; + +import com.google.inject.Injector; +import org.quartz.Job; +import org.quartz.JobExecutionContext; +import org.quartz.JobExecutionException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import uk.ac.cam.cl.dtg.isaac.api.managers.EventBookingManager; +import uk.ac.cam.cl.dtg.segue.configuration.SegueGuiceConfigurationModule; +import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; + +public class CancelExpiredReservationsJob implements Job { + private static final Logger log = LoggerFactory.getLogger(CancelExpiredReservationsJob.class); + private final EventBookingManager eventBookingManager; + + /** + * This class is required by quartz and must be executable by any instance of the segue api relying only on the + * jobdata context provided. + */ + public CancelExpiredReservationsJob() { + Injector injector = SegueGuiceConfigurationModule.getGuiceInjector(); + eventBookingManager = injector.getInstance(EventBookingManager.class); + } + + @Override + public void execute(final JobExecutionContext context) throws JobExecutionException { + try { + eventBookingManager.cancelExpiredReservations(); + log.info("Ran CancelExpiredReservationsJob successfully"); + } catch (SegueDatabaseException e) { + log.error("Error running CancelExpiredReservationsJob", e); + throw new JobExecutionException("Failed to cancel expired reservations", e); + } + } +} diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTest.java index e96b55b9eb..83d0141500 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/IsaacIntegrationTest.java @@ -330,7 +330,7 @@ public static void setUpClass() { PgTransactionManager pgTransactionManager = new PgTransactionManager(postgresSqlDb); eventBookingManager = new EventBookingManager(bookingPersistanceManager, emailManager, userAssociationManager, properties, - groupManager, userAccountManager, pgTransactionManager); + groupManager, userAccountManager, pgTransactionManager, contentManager); userBadgeManager = createNiceMock(UserBadgeManager.class); replay(userBadgeManager); assignmentManager = new AssignmentManager(assignmentPersistenceManager, groupManager, diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManagerTest.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManagerTest.java index 58957d112d..8e77f53cde 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManagerTest.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/managers/EventBookingManagerTest.java @@ -75,6 +75,7 @@ import uk.ac.cam.cl.dtg.segue.comm.EmailType; import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import uk.ac.cam.cl.dtg.segue.dao.content.ContentManagerException; +import uk.ac.cam.cl.dtg.segue.dao.content.GitContentManager; import uk.ac.cam.cl.dtg.util.PropertiesLoader; /** @@ -95,6 +96,7 @@ class EventBookingManagerTest { private IUserAccountManager dummyUserAccountManager; private ITransactionManager dummyTransactionManager; private ITransaction dummyTransaction; + private GitContentManager dummyContentManager; private Object[] mockedObjects; /** @@ -110,10 +112,11 @@ public final void setUp() { this.dummyUserAccountManager = createMock(IUserAccountManager.class); this.dummyTransactionManager = createMock(ITransactionManager.class); this.dummyTransaction = createMock(ITransaction.class); + this.dummyContentManager = createMock(GitContentManager.class); // Replay and Verify this collection of mocks each test to be sure of catching all behaviour! this.mockedObjects = new Object[] { dummyEmailManager, dummyEventBookingPersistenceManager, dummyUserAssociationManager, dummyGroupManager, - dummyPropertiesLoader, dummyUserAccountManager, dummyTransactionManager, dummyTransaction + dummyPropertiesLoader, dummyUserAccountManager, dummyTransactionManager, dummyTransaction, dummyContentManager }; expect(this.dummyPropertiesLoader.getProperty(HOST_NAME)).andReturn("hostname.com").anyTimes(); @@ -1132,7 +1135,9 @@ void cancelBookingPromotesOldestWaitingListEntry() prepareCancellationEmailExpectations("email-event-booking-cancellation-confirmed", testEvent, confirmedUser); - expect(dummyUserAccountManager.getUserDTOById(4L)).andReturn(waitingListUser); + // User 4 is the oldest (someLessFutureDate), so it will be selected first during filtering + expect(dummyUserAccountManager.getUserDTOById(4L)).andReturn(waitingListUser).times(2); // During filtering and post-promotion + EmailTemplateDTO bookingPromotionNotificationTemplate = new EmailTemplateDTO(); expect(dummyEmailManager.getEmailTemplateDTO("email-event-booking-waiting-list-promotion-confirmed")).andReturn( bookingPromotionNotificationTemplate); @@ -1180,7 +1185,7 @@ void cancelConfirmedBookingTriggersWaitingListPromotion() prepareCancellationEmailExpectations("email-event-booking-cancellation-confirmed", testEvent, confirmedUser); - expect(dummyUserAccountManager.getUserDTOById(4L)).andReturn(waitingListUser); + expect(dummyUserAccountManager.getUserDTOById(4L)).andReturn(waitingListUser).times(2); // Once during selection, once for post-promotion EmailTemplateDTO bookingPromotionNotificationTemplate = new EmailTemplateDTO(); expect(dummyEmailManager.getEmailTemplateDTO("email-event-booking-waiting-list-promotion-confirmed")).andReturn( bookingPromotionNotificationTemplate); @@ -1245,7 +1250,7 @@ void cancelReservedBookingTriggersWaitingListPromotion() "givenName familyName"), EmailType.SYSTEM); expectLastCall(); - expect(dummyUserAccountManager.getUserDTOById(4L)).andReturn(waitingListUser); + expect(dummyUserAccountManager.getUserDTOById(4L)).andReturn(waitingListUser).times(2); // Once during selection, once for post-promotion EmailTemplateDTO bookingPromotionNotificationTemplate = new EmailTemplateDTO(); expect(dummyEmailManager.getEmailTemplateDTO("email-event-booking-waiting-list-promotion-confirmed")).andReturn( bookingPromotionNotificationTemplate); @@ -1312,20 +1317,24 @@ void cancelBooking_noUserExceptionShouldBeCaughtIfPromotedUserNotFound() RegisteredUserDTO confirmedUser = new RegisteredUserDTO(); confirmedUser.setId(2L); - DetailedEventBookingDTO waitingListBookingToBeUpdated = + // Test that when the waiting list user is deleted, promotion is skipped + DetailedEventBookingDTO deletedWaitingListBooking = prepareDetailedEventBookingDto(prepareUserSummaryDto(3L), BookingStatus.WAITING_LIST, testEvent.getId()); - waitingListBookingToBeUpdated.setBookingDate(someFutureDate); - List waitingListBookingsList = List.of(waitingListBookingToBeUpdated); - DetailedEventBookingDTO updatedWaitingListBooking = - prepareDetailedEventBookingDto(prepareUserSummaryDto(3L), BookingStatus.CONFIRMED, testEvent.getId()); + deletedWaitingListBooking.setBookingDate(someFutureDate); + List waitingListBookingsList = List.of(deletedWaitingListBooking); prepareTransactionExpectations(testEvent); prepareConfirmedBookingExpectations(testEvent, confirmedUser); - prepareNonEmptyWaitingListExpectations(testEvent, waitingListBookingToBeUpdated, waitingListBookingsList, - updatedWaitingListBooking); + // Mock the waiting list query to return the booking + expect(dummyEventBookingPersistenceManager.adminGetBookingsByEventIdAndStatus( + eq(testEvent.getId()), + eq(BookingStatus.WAITING_LIST), + EasyMock.anyBoolean())).andReturn(waitingListBookingsList); + // Don't expect updateBookingStatus to be called since user is deleted prepareCancellationEmailExpectations("email-event-booking-cancellation-confirmed", testEvent, confirmedUser); + // Mock getUserDTOById to throw NoUserException for the deleted user expect(dummyUserAccountManager.getUserDTOById(3L)).andThrow(new NoUserException("No user found with this ID!")); replay(mockedObjects); @@ -1364,7 +1373,7 @@ void cancelBooking_EventBookingUpdateExceptionShouldBeCaughtIfEmailCouldNotBeCon prepareCancellationEmailExpectations("email-event-booking-cancellation-confirmed", testEvent, confirmedUser); - expect(dummyUserAccountManager.getUserDTOById(3L)).andReturn(waitingListUser); + expect(dummyUserAccountManager.getUserDTOById(3L)).andReturn(waitingListUser).times(2); // Once during selection, once for post-promotion expect(dummyEmailManager.getEmailTemplateDTO("email-event-booking-waiting-list-promotion-confirmed")).andThrow( new ContentManagerException("Content is of incorrect type:notAnEmailTemplateDTO")); @@ -1515,8 +1524,10 @@ private void prepareReservedBookingExpectations(IsaacEventPageDTO testEvent, Reg private void prepareNonEmptyWaitingListExpectations(IsaacEventPageDTO testEvent, DetailedEventBookingDTO waitingListBookingToBeUpdated, List waitingListBookingsList, DetailedEventBookingDTO updatedWaitingListBooking) throws SegueDatabaseException { - expect(dummyEventBookingPersistenceManager.adminGetBookingsByEventIdAndStatus(testEvent.getId(), - BookingStatus.WAITING_LIST)).andReturn(waitingListBookingsList); + expect(dummyEventBookingPersistenceManager.adminGetBookingsByEventIdAndStatus( + eq(testEvent.getId()), + eq(BookingStatus.WAITING_LIST), + EasyMock.anyBoolean())).andReturn(waitingListBookingsList); expect(dummyEventBookingPersistenceManager.updateBookingStatus(dummyTransaction, testEvent.getId(), waitingListBookingToBeUpdated.getUserBooked().getId(), BookingStatus.CONFIRMED, waitingListBookingToBeUpdated.getAdditionalInformation())).andReturn(updatedWaitingListBooking); @@ -1524,8 +1535,10 @@ private void prepareNonEmptyWaitingListExpectations(IsaacEventPageDTO testEvent, private void prepareEmptyWaitingListExpectations(IsaacEventPageDTO testEvent) throws SegueDatabaseException { List waitingListBookingsList = List.of(); - expect(dummyEventBookingPersistenceManager.adminGetBookingsByEventIdAndStatus(testEvent.getId(), - BookingStatus.WAITING_LIST)).andReturn(waitingListBookingsList); + expect(dummyEventBookingPersistenceManager.adminGetBookingsByEventIdAndStatus( + eq(testEvent.getId()), + eq(BookingStatus.WAITING_LIST), + EasyMock.anyBoolean())).andReturn(waitingListBookingsList); } private void prepareCancellationEmailExpectations(String emailTemplateId, IsaacEventPageDTO testEvent, @@ -1539,6 +1552,110 @@ private void prepareCancellationEmailExpectations(String emailTemplateId, IsaacE EMAIL_TEMPLATE_TOKEN_EVENT_DETAILS, "", EMAIL_TEMPLATE_TOKEN_EVENT, testEvent), EmailType.SYSTEM); expectLastCall(); } + + @Test + void cancelConfirmedBooking_skipsDeletedWaitingListUsers_promotesNextValidUser() + throws SegueDatabaseException, ContentManagerException, NoUserException { + // Test that when the oldest waiting list user is deleted, promotion skips to the next valid user + IsaacEventPageDTO testEvent = prepareIsaacEventPageDto(studentCSTags); + + RegisteredUserDTO confirmedUser = new RegisteredUserDTO(); + confirmedUser.setId(2L); + + // Create waiting list users: first one is "deleted", second is valid + DetailedEventBookingDTO deletedWaitingListUser = + prepareDetailedEventBookingDto(prepareUserSummaryDto(3L), BookingStatus.WAITING_LIST, testEvent.getId()); + deletedWaitingListUser.setBookingDate(someLessFutureDate); + + DetailedEventBookingDTO validWaitingListUser = + prepareDetailedEventBookingDto(prepareUserSummaryDto(4L), BookingStatus.WAITING_LIST, testEvent.getId()); + validWaitingListUser.setBookingDate(someFutureDate); + + List allWaitingListBookings = + List.of(deletedWaitingListUser, validWaitingListUser); + + DetailedEventBookingDTO updatedValidWaitingListBooking = + prepareDetailedEventBookingDto(prepareUserSummaryDto(4L), BookingStatus.CONFIRMED, testEvent.getId()); + + RegisteredUserDTO promotedUser = new RegisteredUserDTO(); + promotedUser.setId(4L); + + prepareTransactionExpectations(testEvent); + prepareConfirmedBookingExpectations(testEvent, confirmedUser); + + // Mock the waiting list query to return both users (includeDeleted might be true for past events) + expect(dummyEventBookingPersistenceManager.adminGetBookingsByEventIdAndStatus( + eq(testEvent.getId()), + eq(BookingStatus.WAITING_LIST), + EasyMock.anyBoolean())).andReturn(allWaitingListBookings); + + // Mock getUserDTOById to simulate user 3 being deleted + expect(dummyUserAccountManager.getUserDTOById(3L)) + .andThrow(new NoUserException("User deleted")); + + // Mock getUserDTOById for valid user 4 + expect(dummyUserAccountManager.getUserDTOById(4L)) + .andReturn(promotedUser) + .times(2); // Once during selection, once for post-promotion operations + + // Now expect updateBookingStatus to be called with user 4's ID + expect(dummyEventBookingPersistenceManager.updateBookingStatus(dummyTransaction, testEvent.getId(), + validWaitingListUser.getUserBooked().getId(), BookingStatus.CONFIRMED, + validWaitingListUser.getAdditionalInformation())).andReturn(updatedValidWaitingListBooking); + + prepareCancellationEmailExpectations("email-event-booking-cancellation-confirmed", testEvent, + confirmedUser); + EmailTemplateDTO bookingPromotionNotificationTemplate = new EmailTemplateDTO(); + expect(dummyEmailManager.getEmailTemplateDTO("email-event-booking-waiting-list-promotion-confirmed")).andReturn( + bookingPromotionNotificationTemplate); + dummyEmailManager.sendTemplatedEmailToUser(eq(promotedUser), eq(bookingPromotionNotificationTemplate), + EasyMock.anyObject(), eq(EmailType.SYSTEM), EasyMock.>anyObject()); + expectLastCall(); + + replay(mockedObjects); + + EventBookingManager ebm = buildEventBookingManager(); + try { + ebm.cancelBooking(testEvent, confirmedUser); + } catch (SegueDatabaseException | ContentManagerException e) { + fail("No exception is expected for this test"); + } + + verify(mockedObjects); + } + + @Test + void cancelConfirmedBooking_noWaitingListUsersToPromote_whenAllDeleted() + throws SegueDatabaseException, ContentManagerException, NoUserException { + // Test that when all waiting list users are deleted on a future event, none are promoted + IsaacEventPageDTO testEvent = prepareIsaacEventPageDto(studentCSTags); + + RegisteredUserDTO confirmedUser = new RegisteredUserDTO(); + confirmedUser.setId(2L); + + prepareTransactionExpectations(testEvent); + prepareConfirmedBookingExpectations(testEvent, confirmedUser); + + // Mock the waiting list query to return empty list (all deleted users, future event) + expect(dummyEventBookingPersistenceManager.adminGetBookingsByEventIdAndStatus( + eq(testEvent.getId()), + eq(BookingStatus.WAITING_LIST), + eq(false))).andReturn(List.of()); // includeDeleted=false for future events + + prepareCancellationEmailExpectations("email-event-booking-cancellation-confirmed", testEvent, + confirmedUser); + + replay(mockedObjects); + + EventBookingManager ebm = buildEventBookingManager(); + try { + ebm.cancelBooking(testEvent, confirmedUser); + } catch (SegueDatabaseException | ContentManagerException e) { + fail("No exception is expected for this test"); + } + + verify(mockedObjects); + } } @Nested @@ -1768,7 +1885,8 @@ static class ReservationTestDefaults { private EventBookingManager buildEventBookingManager() { return new EventBookingManager( dummyEventBookingPersistenceManager, dummyEmailManager, dummyUserAssociationManager, - dummyPropertiesLoader, dummyGroupManager, dummyUserAccountManager, dummyTransactionManager); + dummyPropertiesLoader, dummyGroupManager, dummyUserAccountManager, dummyTransactionManager, + dummyContentManager); } private void prepareCommonTransactionExpectations(IsaacEventPageDTO testEvent) throws SegueDatabaseException {