From e36a950f0e5045223f248271cd4f40cb97d0529c Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 5 May 2026 12:34:43 +0100 Subject: [PATCH 1/6] Add optional date parameter to assignment progress CSV endpoints Allows the CSVs to be filtered to include only question attempts before a given timestamp. --- .../cl/dtg/isaac/api/AssignmentFacade.java | 14 ++++-- .../isaac/quiz/IQuestionAttemptManager.java | 2 +- .../cl/dtg/isaac/quiz/PgQuestionAttempts.java | 46 ++++++++++++++----- .../segue/api/managers/QuestionManager.java | 22 ++++++++- .../cl/dtg/isaac/api/AssignmentFacadeIT.java | 8 ++-- 5 files changed, 69 insertions(+), 23 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java index d12a9f8e6e..7fc451f238 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java @@ -459,6 +459,8 @@ public Response getAssignmentProgress(@Context final HttpServletRequest request, * @param formatMode * - whether to format the file in a special way. Currently only "excel" is supported, * to include a UTF-8 BOM to allow Unicode student names to show correctly in Microsoft Excel. + * @param toDate + * - if specified, only question attempts before this date will be included in the results. * @param request * - so that we can identify the current user. * @return the assignment object. @@ -471,7 +473,8 @@ public Response getAssignmentProgress(@Context final HttpServletRequest request, @Operation(summary = "Download the progress of a specific assignment.") public Response getAssignmentProgressDownloadCSV(@Context final HttpServletRequest request, @PathParam("assignment_id") final Long assignmentId, - @QueryParam("format") final String formatMode) { + @QueryParam("format") final String formatMode, + @QueryParam("to_date") final Long toDate) { try { RegisteredUserDTO currentlyLoggedInUser = userManager.getCurrentRegisteredUser(request); @@ -499,7 +502,7 @@ public Response getAssignmentProgressDownloadCSV(@Context final HttpServletReque questionPageIds.add(questionPage.getId()); } Map>>> questionAttempts; - questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds); + questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, new Date(toDate)); Map>>> questionAttemptsForAllUsersOfInterest = new HashMap<>(); @@ -666,6 +669,8 @@ public Response getAssignmentProgressDownloadCSV(@Context final HttpServletReque * @param formatMode * - whether to format the file in a special way. Currently only "excel" is supported, * to include a UTF-8 BOM to allow Unicode student names to show correctly in Microsoft Excel. + * @param toDate + * - if specified, only question attempts before this date will be included in the results. * @param request * - so that we can identify the current user. * @return the assignment object. @@ -678,7 +683,8 @@ public Response getAssignmentProgressDownloadCSV(@Context final HttpServletReque @Operation(summary = "Download the progress of a group on all assignments set.") public Response getGroupAssignmentsProgressDownloadCSV(@Context final HttpServletRequest request, @PathParam("group_id") final Long groupId, - @QueryParam("format") final String formatMode) { + @QueryParam("format") final String formatMode, + @QueryParam("to_date") final Long toDate) { try { // Fetch the currently logged in user @@ -723,7 +729,7 @@ public Response getGroupAssignmentsProgressDownloadCSV(@Context final HttpServle List questionPageIds = gameboardItems.stream().map(GameboardItem::getId).collect(Collectors.toList()); Map>>> questionAttempts; try { - questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds); + questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, new Date(toDate)); } catch (IllegalArgumentException e) { questionAttempts = new HashMap<>(); } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IQuestionAttemptManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IQuestionAttemptManager.java index 71b9dd99cd..05a8bd5261 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IQuestionAttemptManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IQuestionAttemptManager.java @@ -86,7 +86,7 @@ Map>> getQuestionAttempts(L * - if a database error occurrs */ Map>>> - getMatchingLightweightQuestionAttempts(List userIds, List questionPage) + getMatchingLightweightQuestionAttempts(List userIds, List questionPage, Date toDate) throws SegueDatabaseException; /** diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/PgQuestionAttempts.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/PgQuestionAttempts.java index 9c9b6756d2..185be9c75d 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/PgQuestionAttempts.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/PgQuestionAttempts.java @@ -37,6 +37,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.sql.Timestamp; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -260,15 +261,21 @@ public Map>> getQuestionAtt } } - public Map>>> getLightweightQuestionAttemptsByUsers(final List userIds) - throws SegueDatabaseException { + public Map>>> getLightweightQuestionAttemptsByUsers( + final List userIds, final Date toDate) throws SegueDatabaseException { if (userIds.isEmpty()) { return Collections.emptyMap(); } - String query = "SELECT id, user_id, question_id, correct, marks, timestamp FROM question_attempts" - + " WHERE user_id = ANY(?) ORDER BY \"timestamp\" ASC"; + String query; + if (null != toDate) { + query = "SELECT id, user_id, question_id, correct, marks, timestamp FROM question_attempts" + + " WHERE user_id = ANY(?) AND timestamp < ? ORDER BY \"timestamp\" ASC"; + } else { + query = "SELECT id, user_id, question_id, correct, marks, timestamp FROM question_attempts" + + " WHERE user_id = ANY(?) ORDER BY \"timestamp\" ASC"; + } Map>>> mapToReturn = userIds.stream().collect(Collectors.toMap(Function.identity(), k -> Maps.newLinkedHashMap())); @@ -279,25 +286,29 @@ public Map>> getLightweightQuestionAttempts(Long userId) throws SegueDatabaseException { - return this.getLightweightQuestionAttemptsByUsers(Collections.singletonList(userId)).getOrDefault(userId, Collections.emptyMap()); + public Map>> getLightweightQuestionAttempts(final Long userId) throws SegueDatabaseException { + return this.getLightweightQuestionAttemptsByUsers(Collections.singletonList(userId), null).getOrDefault(userId, Collections.emptyMap()); } @Override public Map>>> - getMatchingLightweightQuestionAttempts(final List userIds, final List allQuestionPageIds) + getMatchingLightweightQuestionAttempts(final List userIds, final List allQuestionPageIds, final Date toDate) throws SegueDatabaseException { if (allQuestionPageIds.isEmpty() || userIds.isEmpty()) { @@ -307,16 +318,23 @@ public Map>> get List uniquePageIds = allQuestionPageIds.stream().distinct().toList(); if (uniquePageIds.size() > MAX_PAGE_IDS_TO_MATCH) { log.debug("Attempting to match too many ({}) question page IDs; returning all attempts for these users instead!", uniquePageIds.size()); - return this.getLightweightQuestionAttemptsByUsers(userIds); + return this.getLightweightQuestionAttemptsByUsers(userIds, toDate); } Map>>> mapToReturn = userIds.stream().collect(Collectors.toMap(Function.identity(), k -> Maps.newHashMap()));; try (Connection conn = database.getDatabaseConnection()) { - String query = "SELECT id, user_id, question_id, correct, marks, timestamp FROM question_attempts" - + " WHERE user_id = ANY(?) AND page_id = ANY(?)" - + " ORDER BY \"timestamp\" ASC"; + String query; + if (null != toDate) { + query = "SELECT id, user_id, question_id, correct, marks, timestamp FROM question_attempts" + + " WHERE user_id = ANY(?) AND page_id = ANY(?) AND timestamp < ?" + + " ORDER BY \"timestamp\" ASC"; + } else { + query = "SELECT id, user_id, question_id, correct, marks, timestamp FROM question_attempts" + + " WHERE user_id = ANY(?) AND page_id = ANY(?)" + + " ORDER BY \"timestamp\" ASC"; + } try (PreparedStatement pst = conn.prepareStatement(query)) { @@ -325,6 +343,10 @@ public Map>> get pst.setArray(1, userIdArray); pst.setArray(2, pageIdArray); + if (null != toDate) { + pst.setTimestamp(3, new Timestamp(toDate.getTime())); + } + try (ResultSet results = pst.executeQuery()) { augmentMapLightweightValidationResponseByUserPagePartWithResults(mapToReturn, results); return mapToReturn; diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/QuestionManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/QuestionManager.java index f342e8146a..614525433c 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/QuestionManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/QuestionManager.java @@ -417,17 +417,35 @@ public Map>> getQuestionAtt /** * @param users who we are interested in. * @param questionPageIds we want to look up. + * @param toDate only include question attempts before this date * @return a map of user id to question page id to question_id to list of attempts. * @throws SegueDatabaseException if there is a database error. */ public Map>>> getMatchingLightweightQuestionAttempts( - final List users, final List questionPageIds) throws SegueDatabaseException { + final List users, final List questionPageIds, final Date toDate) + throws SegueDatabaseException { List userIds = Lists.newArrayList(); for (RegisteredUserDTO user : users) { userIds.add(user.getId()); } - return this.questionAttemptPersistenceManager.getMatchingLightweightQuestionAttempts(userIds, questionPageIds); + return this.questionAttemptPersistenceManager.getMatchingLightweightQuestionAttempts(userIds, questionPageIds, toDate); + } + + /** + * Helper method for when we don't want to filter question attempts by date. + * + * @see #getMatchingLightweightQuestionAttempts(List, List, Date) + * + * @param users who we are interested in. + * @param questionPageIds we want to look up. + * @return a map of user id to question page id to question_id to list of attempts. + * @throws SegueDatabaseException if there is a database error. + */ + public Map>>> getMatchingLightweightQuestionAttempts( + final List users, final List questionPageIds) + throws SegueDatabaseException { + return getMatchingLightweightQuestionAttempts(users, questionPageIds, null); } /** diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacadeIT.java index 86b9c1be4d..d8cc518e82 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacadeIT.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacadeIT.java @@ -574,7 +574,7 @@ void getAssignmentProgressDownloadCSV_getAsOwner_succeeds() throws Exception { // Act Response downloadAssignmentResponse = assignmentFacade.getAssignmentProgressDownloadCSV(downloadAssignmentRequest, - ITConstants.ASSIGNMENTS_TEST_EXISTING_HARRY_AB_ASSIGNMENT_ID, "excel"); + ITConstants.ASSIGNMENTS_TEST_EXISTING_HARRY_AB_ASSIGNMENT_ID, "excel", null); String downloadAssignmentContents = downloadAssignmentResponse.getEntity().toString(); // Assert @@ -598,7 +598,7 @@ void getAssignmentProgressDownloadCSV_getAsOther_permissionDenied() throws Excep // Act Response downloadAssignmentResponse = assignmentFacade.getAssignmentProgressDownloadCSV(downloadAssignmentRequest, - ITConstants.ASSIGNMENTS_TEST_EXISTING_HARRY_AB_ASSIGNMENT_ID, "excel"); + ITConstants.ASSIGNMENTS_TEST_EXISTING_HARRY_AB_ASSIGNMENT_ID, "excel", null); // Assert assertEquals(Response.Status.FORBIDDEN.getStatusCode(), downloadAssignmentResponse.getStatus()); @@ -616,7 +616,7 @@ void getGroupProgressDownloadCSV_getAsOwner_succeeds() throws Exception { // Act Response downloadAssignmentResponse = assignmentFacade.getGroupAssignmentsProgressDownloadCSV(downloadAssignmentRequest, - ITConstants.HARRY_TEACHERS_AB_GROUP_ID, "excel"); + ITConstants.HARRY_TEACHERS_AB_GROUP_ID, "excel", null); String downloadAssignmentContents = downloadAssignmentResponse.getEntity().toString(); // Assert @@ -640,7 +640,7 @@ void getGroupProgressDownloadCSV_getAsOther_permissionDenied() throws Exception // Act Response downloadAssignmentResponse = assignmentFacade.getGroupAssignmentsProgressDownloadCSV(downloadAssignmentRequest, - ITConstants.HARRY_TEACHERS_AB_GROUP_ID, "excel"); + ITConstants.HARRY_TEACHERS_AB_GROUP_ID, "excel", null); // Assert assertEquals(Response.Status.FORBIDDEN.getStatusCode(), downloadAssignmentResponse.getStatus()); From da03516182d941e2640ffe87ff5176ff33384897 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 5 May 2026 13:58:44 +0100 Subject: [PATCH 2/6] Avoid NPE --- .../java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java index 7fc451f238..7e08e3347e 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java @@ -502,7 +502,12 @@ public Response getAssignmentProgressDownloadCSV(@Context final HttpServletReque questionPageIds.add(questionPage.getId()); } Map>>> questionAttempts; - questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, new Date(toDate)); + + if (null != toDate) { + questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, new Date(toDate)); + } else { + questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds); + } Map>>> questionAttemptsForAllUsersOfInterest = new HashMap<>(); From 4bbb464acc5f5e1f8ab4ab69bd527a724f00839a Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 5 May 2026 14:00:27 +0100 Subject: [PATCH 3/6] Note date cutoff in progress CSV --- .../java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java index 7e08e3347e..5be579c8f2 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java @@ -527,6 +527,10 @@ public Response getAssignmentProgressDownloadCSV(@Context final HttpServletReque assignmentId, timestampFormat.format(Date.from(Instant.now(clock))), currentlyLoggedInUser.getGivenName(), currentlyLoggedInUser.getFamilyName())); + if (null != toDate) { + headerBuilder.append(String.format("Assignment status as at %s\n\n", timestampFormat.format(toDate))); + } + List headerRow = Lists.newArrayList(Arrays.asList("", "")); if (includeUserIDs) { headerRow.add(""); From 7292a2994b71574b266b0e646157cd1aafc1c151 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 5 May 2026 14:04:26 +0100 Subject: [PATCH 4/6] Avoid NPE --- .../java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java index 5be579c8f2..f70de4ee2f 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java @@ -738,7 +738,11 @@ public Response getGroupAssignmentsProgressDownloadCSV(@Context final HttpServle List questionPageIds = gameboardItems.stream().map(GameboardItem::getId).collect(Collectors.toList()); Map>>> questionAttempts; try { - questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, new Date(toDate)); + if (null != toDate) { + questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, new Date(toDate)); + } else { + questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds); + } } catch (IllegalArgumentException e) { questionAttempts = new HashMap<>(); } From 5e3e2f7ac6401dae69bfa35607c77791a8e132ff Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Tue, 5 May 2026 14:17:09 +0100 Subject: [PATCH 5/6] Note date cutoff in progress CSV --- .../java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java index f70de4ee2f..f6bc333517 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java @@ -944,9 +944,13 @@ public Response getGroupAssignmentsProgressDownloadCSV(@Context final HttpServle headerBuilder.append(String.format("Assignments for '%s' (%s)\nDownloaded on %s\nGenerated by: %s %s\n\n", group.getGroupName(), group.getId(), timestampFormat.format(Date.from(Instant.now(clock))), currentlyLoggedInUser.getGivenName(), currentlyLoggedInUser.getFamilyName())) - .append(stringWriter.toString()) + .append(stringWriter) .append("\n\nN.B.\n\"The percentages are for question parts completed, not question pages.\"\n"); + if (null != toDate) { + headerBuilder.append(String.format("Assignment status as at %s\n\n", timestampFormat.format(toDate))); + } + this.getLogManager().logEvent(currentlyLoggedInUser, request, IsaacServerLogType.DOWNLOAD_GROUP_PROGRESS_CSV, ImmutableMap.of("groupId", groupId)); From fedd78899c9c43508da5646fcd81f7597b6a4441 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Fri, 8 May 2026 16:41:05 +0100 Subject: [PATCH 6/6] Simplify control flow for null to_date --- .../java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java index f6bc333517..b3c48a1760 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java @@ -503,11 +503,11 @@ public Response getAssignmentProgressDownloadCSV(@Context final HttpServletReque } Map>>> questionAttempts; + Date cutoffDate = null; if (null != toDate) { - questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, new Date(toDate)); - } else { - questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds); + cutoffDate = new Date(toDate); } + questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, cutoffDate); Map>>> questionAttemptsForAllUsersOfInterest = new HashMap<>();