788 - Waiting list bookings are not auto promoted #788

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 @@
   private final GroupManager groupManager;
   private final IUserAccountManager userAccountManager;
   private final ITransactionManager transactionManager;
+  private final GitContentManager contentManager;
 
   /**
    * EventBookingManager.
@@ -127,6 +129,7 @@
    * @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,14 +138,16 @@
                              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;
     this.propertiesLoader = propertiesLoader;
     this.groupManager = groupManager;
     this.userAccountManager = userAccountManager;
     this.transactionManager = transactionManager;
+    this.contentManager = contentManager;
   }
 
   /**
@@ -945,11 +950,19 @@
     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 @@
 
     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 @@
       // 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<DetailedEventBookingDTO> 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 @@
           + "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 @@
         .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<DetailedEventBookingDTO> 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;
+    }
+  }
+
 }

src/main/java/uk/ac/cam/cl/dtg/isaac/dao/EventBookingPersistenceManager.java

@@ -216,6 +216,23 @@ public List<DetailedEventBookingDTO> adminGetBookingsByEventId(final String even
   public List<DetailedEventBookingDTO> 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<DetailedEventBookingDTO> 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<DetailedEventBookingDTO> 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<DetailedEventBookingDTO> 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<DetailedEventBookingDTO> getExpiredReservations() throws SegueDatabaseException {
+    return convertToDTO(Lists.newArrayList(dao.findExpiredReservations()));
+  }
 }

src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/EventBookings.java

@@ -146,6 +146,19 @@ Map<BookingStatus, Map<Role, Integer>> getEventBookingStatusCounts(String eventI
   Iterable<EventBooking> 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<EventBooking> findAllByEventIdAndStatus(String eventId, @Nullable BookingStatus status,
+                                                   boolean includeDeletedUsers)
+      throws SegueDatabaseException;
+
   /**
    * Find all bookings for a given event.
    *
@@ -200,4 +213,12 @@ Iterable<EventBooking> 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<EventBooking> findExpiredReservations() throws SegueDatabaseException;
 }

src/main/java/uk/ac/cam/cl/dtg/isaac/dos/eventbookings/PgEventBookings.java

@@ -54,6 +54,7 @@
  * <br>
  * 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<BookingStatus, Map<Role, Integer>> getEventBookingStatusCounts(final
   @Override
   public Iterable<EventBooking> 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.
+   * <br>
+   * 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<EventBooking> 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<EventBooking> 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<EventBooking> 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<EventBooking> 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"),

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",

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);
+        }
       }
     }