Skip to content

Add date filter to assignment progress CSV endpoints #787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jsharkey13 merged 6 commits into from
May 12, 2026
Merged

Add date filter to assignment progress CSV endpoints #787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e36a950
Add optional date parameter to assignment progress CSV endpoints
axlewin May 5, 2026
da03516
Avoid NPE
axlewin May 5, 2026
4bbb464
Note date cutoff in progress CSV
axlewin May 5, 2026
7292a29
Avoid NPE
axlewin May 5, 2026
5e3e2f7
Note date cutoff in progress CSV
axlewin May 5, 2026
fedd788
Simplify control flow for null to_date
axlewin May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions src/main/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacade.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -499,7 +502,12 @@ public Response getAssignmentProgressDownloadCSV(@Context final HttpServletReque
questionPageIds.add(questionPage.getId());
}
Map<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>> questionAttempts;
questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds);

Date cutoffDate = null;
if (null != toDate) {
cutoffDate = new Date(toDate);
}
questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds, cutoffDate);

Map<RegisteredUserDTO, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>>
questionAttemptsForAllUsersOfInterest = new HashMap<>();
Expand All @@ -519,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<String> headerRow = Lists.newArrayList(Arrays.asList("", ""));
if (includeUserIDs) {
headerRow.add("");
Expand Down Expand Up @@ -666,6 +678,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.
Expand All @@ -678,7 +692,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
Expand Down Expand Up @@ -723,7 +738,11 @@ public Response getGroupAssignmentsProgressDownloadCSV(@Context final HttpServle
List<String> questionPageIds = gameboardItems.stream().map(GameboardItem::getId).collect(Collectors.toList());
Map<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>> questionAttempts;
try {
questionAttempts = this.questionManager.getMatchingLightweightQuestionAttempts(groupMembers, questionPageIds);
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<>();
}
Expand Down Expand Up @@ -925,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));

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/IQuestionAttemptManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ Map<String, Map<String, List<QuestionValidationResponse>>> getQuestionAttempts(L
* - if a database error occurrs
*/
Map<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>>
getMatchingLightweightQuestionAttempts(List<Long> userIds, List<String> questionPage)
getMatchingLightweightQuestionAttempts(List<Long> userIds, List<String> questionPage, Date toDate)
throws SegueDatabaseException;

/**
Expand Down
46 changes: 34 additions & 12 deletions src/main/java/uk/ac/cam/cl/dtg/isaac/quiz/PgQuestionAttempts.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -260,15 +261,21 @@ public Map<String, Map<String, List<QuestionValidationResponse>>> getQuestionAtt
}
}

public Map<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>> getLightweightQuestionAttemptsByUsers(final List<Long> userIds)
throws SegueDatabaseException {
public Map<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>> getLightweightQuestionAttemptsByUsers(
final List<Long> 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";
Comment thread
jsharkey13 marked this conversation as resolved.
} else {
query = "SELECT id, user_id, question_id, correct, marks, timestamp FROM question_attempts"
+ " WHERE user_id = ANY(?) ORDER BY \"timestamp\" ASC";
}

Map<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>> mapToReturn
= userIds.stream().collect(Collectors.toMap(Function.identity(), k -> Maps.newLinkedHashMap()));
Expand All @@ -279,25 +286,29 @@ public Map<Long, Map<String, Map<String, List<LightweightQuestionValidationRespo
Array userIdArray = conn.createArrayOf("INTEGER", userIds.toArray());
pst.setArray(1, userIdArray);

if (null != toDate) {
pst.setTimestamp(2, new Timestamp(toDate.getTime()));
}

try (ResultSet results = pst.executeQuery()) {
augmentMapLightweightValidationResponseByUserPagePartWithResults(mapToReturn, results);
return mapToReturn;
} finally {
userIdArray.free();
}
} catch (SQLException e) {
} catch (final SQLException e) {
throw new SegueDatabaseException("Postgres exception", e);
}
}

@Override
public Map<String, Map<String, List<LightweightQuestionValidationResponse>>> getLightweightQuestionAttempts(Long userId) throws SegueDatabaseException {
return this.getLightweightQuestionAttemptsByUsers(Collections.singletonList(userId)).getOrDefault(userId, Collections.emptyMap());
public Map<String, Map<String, List<LightweightQuestionValidationResponse>>> getLightweightQuestionAttempts(final Long userId) throws SegueDatabaseException {
return this.getLightweightQuestionAttemptsByUsers(Collections.singletonList(userId), null).getOrDefault(userId, Collections.emptyMap());
}

@Override
public Map<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>>
getMatchingLightweightQuestionAttempts(final List<Long> userIds, final List<String> allQuestionPageIds)
getMatchingLightweightQuestionAttempts(final List<Long> userIds, final List<String> allQuestionPageIds, final Date toDate)
throws SegueDatabaseException {

if (allQuestionPageIds.isEmpty() || userIds.isEmpty()) {
Expand All @@ -307,16 +318,23 @@ public Map<String, Map<String, List<LightweightQuestionValidationResponse>>> get
List<String> 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<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>> 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 < ?"
Comment thread
jsharkey13 marked this conversation as resolved.
+ " 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)) {

Expand All @@ -325,6 +343,10 @@ public Map<String, Map<String, List<LightweightQuestionValidationResponse>>> 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;
Expand Down
22 changes: 20 additions & 2 deletions src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/QuestionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,17 +417,35 @@ public Map<String, Map<String, List<QuestionValidationResponse>>> 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<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>> getMatchingLightweightQuestionAttempts(
final List<RegisteredUserDTO> users, final List<String> questionPageIds) throws SegueDatabaseException {
final List<RegisteredUserDTO> users, final List<String> questionPageIds, final Date toDate)
throws SegueDatabaseException {
List<Long> 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<Long, Map<String, Map<String, List<LightweightQuestionValidationResponse>>>> getMatchingLightweightQuestionAttempts(
final List<RegisteredUserDTO> users, final List<String> questionPageIds)
throws SegueDatabaseException {
return getMatchingLightweightQuestionAttempts(users, questionPageIds, null);
Comment thread
jsharkey13 marked this conversation as resolved.
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/uk/ac/cam/cl/dtg/isaac/api/AssignmentFacadeIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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());
Expand All @@ -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
Expand All @@ -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());
Expand Down
Loading