From e89fa2cfc007de92528f531aabc05dacb5f76df9 Mon Sep 17 00:00:00 2001 From: Matthew Trew Date: Thu, 29 May 2025 16:36:22 +0100 Subject: [PATCH 1/2] Add question part status to AssignmentProgressDTO --- .../cl/dtg/isaac/api/AssignmentFacade.java | 10 +++++--- .../dtg/isaac/dto/AssignmentProgressDTO.java | 25 +++++++++++++------ .../cl/dtg/isaac/api/AssignmentFacadeIT.java | 6 ++--- .../dtg/isaac/api/AuthorisationFacadeIT.java | 2 +- 4 files changed, 28 insertions(+), 15 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 1cfc5e747f..fdb92c4192 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 @@ -405,11 +405,13 @@ public Response getAssignmentProgress(@Context final HttpServletRequest request, // can the user access the data? if (userSummary.isAuthorisedFullAccess()) { - ArrayList states = Lists.newArrayList(); + ArrayList questionStates = Lists.newArrayList(); + ArrayList> questionPartStates = Lists.newArrayList(); ArrayList correctQuestionParts = Lists.newArrayList(); ArrayList incorrectQuestionParts = Lists.newArrayList(); for (GameboardItem questionResult : userGameboardItems.getRight()) { - states.add(questionResult.getState()); + questionStates.add(questionResult.getState()); + questionPartStates.add(questionResult.getQuestionPartStates()); correctQuestionParts.add(questionResult.getQuestionPartsCorrect()); incorrectQuestionParts.add(questionResult.getQuestionPartsIncorrect()); } @@ -417,13 +419,15 @@ public Response getAssignmentProgress(@Context final HttpServletRequest request, userSummary, correctQuestionParts, incorrectQuestionParts, - states + questionStates, + questionPartStates )); } else { result.add(new AssignmentProgressDTO( userSummary, Collections.emptyList(), Collections.emptyList(), + Collections.emptyList(), Collections.emptyList() )); } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AssignmentProgressDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AssignmentProgressDTO.java index ac1424d674..0a6f1712e8 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AssignmentProgressDTO.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AssignmentProgressDTO.java @@ -12,7 +12,8 @@ public class AssignmentProgressDTO { public UserSummaryDTO user; public List correctPartResults; public List incorrectPartResults; - public List results; + public List questionResults; + public List> questionPartResults; /** * Complete AssignmentProgressDTO constructor with all dependencies. @@ -25,11 +26,12 @@ public class AssignmentProgressDTO { * @param results * - Array of results. */ - public AssignmentProgressDTO(UserSummaryDTO user, List correctPartResults, List incorrectPartResults, List results) { + public AssignmentProgressDTO(UserSummaryDTO user, List correctPartResults, List incorrectPartResults, List results, List> partResults) { this.user = user; this.correctPartResults = correctPartResults; this.incorrectPartResults = incorrectPartResults; - this.results = results; + this.questionResults = results; + this.questionPartResults = partResults; } public AssignmentProgressDTO() { @@ -59,11 +61,20 @@ public void setIncorrectPartResults(List incorrectPartResults) { this.incorrectPartResults = incorrectPartResults; } - public List getResults() { - return results; + public List getQuestionResults() { + return questionResults; } - public void setResults(List results) { - this.results = results; + public void setQuestionResults(List questionResults) { + this.questionResults = questionResults; + } + + + public List> getQuestionPartResults() { + return questionPartResults; + } + + public void setQuestionPartResults(List> questionPartResults) { + this.questionPartResults = questionPartResults; } } 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 e7ab11d81f..0e7079198a 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 @@ -28,7 +28,6 @@ import uk.ac.cam.cl.dtg.isaac.dto.AssignmentStatusDTO; import uk.ac.cam.cl.dtg.isaac.dto.SegueErrorResponse; import uk.ac.cam.cl.dtg.isaac.dto.UserGroupDTO; -import uk.ac.cam.cl.dtg.isaac.dto.users.UserSummaryDTO; import uk.ac.cam.cl.dtg.segue.dao.SegueDatabaseException; import jakarta.servlet.http.Cookie; @@ -45,7 +44,6 @@ import java.util.ArrayList; import java.util.Calendar; import java.util.Collections; -import java.util.Map; import static org.easymock.EasyMock.replay; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -506,7 +504,7 @@ public void getAssignmentProgressEndpoint_getProgressAsGroupOwner_succeeds() thr // check the progress response contains the right fields AssignmentProgressDTO progressDTO = responseData.get(0); Assertions.assertNotNull(progressDTO.getUser()); - Assertions.assertNotNull(progressDTO.getResults()); + Assertions.assertNotNull(progressDTO.getQuestionResults()); Assertions.assertNotNull(progressDTO.getCorrectPartResults()); Assertions.assertNotNull(progressDTO.getIncorrectPartResults()); } @@ -540,7 +538,7 @@ public void getAssignmentProgressEndpoint_getProgressAsAdditionalManager_succeed // check the progress response contains the right fields AssignmentProgressDTO progressDTO = responseData.get(0); Assertions.assertNotNull(progressDTO.getUser()); - Assertions.assertNotNull(progressDTO.getResults()); + Assertions.assertNotNull(progressDTO.getQuestionResults()); Assertions.assertNotNull(progressDTO.getCorrectPartResults()); Assertions.assertNotNull(progressDTO.getIncorrectPartResults()); } diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AuthorisationFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AuthorisationFacadeIT.java index 21d03af6af..dc9d4169ad 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AuthorisationFacadeIT.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/AuthorisationFacadeIT.java @@ -276,7 +276,7 @@ public void revokeOwnerAssociation_studentRevokesTeacherConnection_studentNotInM UserSummaryDTO userSummary = studentResults.getUser(); assert userSummary != null; if (userSummary.getId() == ALICE_STUDENT_ID) { - List results = studentResults.getResults(); + List results = studentResults.getQuestionResults(); assertTrue(results != null && results.isEmpty()); break; } From 4d42777c9f294ed8cb13d05e107fb2e51badb9b2 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Wed, 4 Jun 2025 12:27:09 +0100 Subject: [PATCH 2/2] Remove GameboardItemStates; replace with CompletionStates --- .../cl/dtg/isaac/api/AssignmentFacade.java | 2 +- .../uk/ac/cam/cl/dtg/isaac/api/Constants.java | 9 +---- .../dtg/isaac/api/managers/GameManager.java | 37 ++++++++++++------- .../api/managers/UserAttemptManager.java | 34 ++++++++++------- .../dtg/isaac/dto/AssignmentProgressDTO.java | 8 ++-- .../cam/cl/dtg/isaac/dto/GameboardItem.java | 7 ++-- .../dtg/segue/api/managers/GroupManager.java | 4 +- .../dtg/isaac/api/AuthorisationFacadeIT.java | 2 +- 8 files changed, 56 insertions(+), 47 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 1cfc5e747f..3e26b12822 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 @@ -405,7 +405,7 @@ public Response getAssignmentProgress(@Context final HttpServletRequest request, // can the user access the data? if (userSummary.isAuthorisedFullAccess()) { - ArrayList states = Lists.newArrayList(); + ArrayList states = Lists.newArrayList(); ArrayList correctQuestionParts = Lists.newArrayList(); ArrayList incorrectQuestionParts = Lists.newArrayList(); for (GameboardItem questionResult : userGameboardItems.getRight()) { diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/Constants.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/Constants.java index 3ee9b2404e..8a1122a0b1 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/Constants.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/Constants.java @@ -70,15 +70,8 @@ public final class Constants { */ public static final int GAME_BOARD_TARGET_SIZE = 10; - /** - * GameboardItemState Represents the potential states of a gameboard item. - */ - public enum GameboardItemState { - PERFECT, PASSED, IN_PROGRESS, NOT_ATTEMPTED, FAILED; - } - public enum CompletionState { - ALL_CORRECT, IN_PROGRESS, NOT_ATTEMPTED; + ALL_CORRECT, ALL_ATTEMPTED, ALL_INCORRECT, IN_PROGRESS, NOT_ATTEMPTED; private static final Set allStates = Set.of(CompletionState.values()); diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java index 804d8d57d7..fc641d8c81 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/GameManager.java @@ -818,7 +818,7 @@ private GameboardDTO augmentGameboardWithQuestionAttemptInformation(final Gamebo continue; } - if (!gameboardStarted && !gameItem.getState().equals(Constants.GameboardItemState.NOT_ATTEMPTED)) { + if (!gameboardStarted && !gameItem.getState().equals(CompletionState.NOT_ATTEMPTED)) { gameboardStarted = true; gameboardDTO.setStartedQuestion(gameboardStarted); } @@ -1009,7 +1009,7 @@ private List getSelectedGameboardQuestions(final GameFilter gameF // choose the gameboard questions to include. while (gameboardReadyQuestions.size() < GAME_BOARD_TARGET_SIZE && !selectionOfGameboardQuestions.isEmpty()) { for (GameboardItem gameboardItem : selectionOfGameboardQuestions) { - GameboardItemState questionState; + CompletionState questionState; try { this.augmentGameItemWithAttemptInformation(gameboardItem, usersQuestionAttempts); questionState = gameboardItem.getState(); @@ -1019,8 +1019,8 @@ private List getSelectedGameboardQuestions(final GameFilter gameF + "should only show available content."); } - if (questionState.equals(GameboardItemState.PASSED) - || questionState.equals(GameboardItemState.PERFECT)) { + if (questionState.equals(CompletionState.ALL_ATTEMPTED) + || questionState.equals(CompletionState.ALL_CORRECT)) { completedQuestions.add(gameboardItem); } else { gameboardReadyQuestions.add(gameboardItem); @@ -1192,21 +1192,30 @@ private void augmentGameItemWithAttemptInformation( gameItem.setQuestionPartStates(questionPartStates); int questionPartsTotal = questionPartsCorrect + questionPartsIncorrect + questionPartsNotAttempted; gameItem.setQuestionPartsTotal(questionPartsTotal); - float percentCorrect = 100f * questionPartsCorrect / questionPartsTotal; - float percentIncorrect = 100f * questionPartsIncorrect / questionPartsTotal; +// float percentCorrect = 100f * questionPartsCorrect / questionPartsTotal; +// float percentIncorrect = 100f * questionPartsIncorrect / questionPartsTotal; - GameboardItemState state; + CompletionState state; if (questionPartsCorrect == questionPartsTotal) { - state = GameboardItemState.PERFECT; + state = CompletionState.ALL_CORRECT; + } else if (questionPartsIncorrect == questionPartsTotal) { + state = CompletionState.ALL_INCORRECT; } else if (questionPartsNotAttempted == questionPartsTotal) { - state = GameboardItemState.NOT_ATTEMPTED; - } else if (percentCorrect >= gameItem.getPassMark()) { - state = GameboardItemState.PASSED; - } else if (percentIncorrect > (100 - gameItem.getPassMark())) { - state = GameboardItemState.FAILED; + state = CompletionState.NOT_ATTEMPTED; + } else if (questionPartsNotAttempted > 0) { + state = CompletionState.IN_PROGRESS; } else { - state = GameboardItemState.IN_PROGRESS; + state = CompletionState.ALL_ATTEMPTED; } + + +// } else if (percentCorrect >= gameItem.getPassMark()) { +// state = GameboardItemState.PASSED; +// } else if (percentIncorrect > (100 - gameItem.getPassMark())) { +// state = GameboardItemState.FAILED; +// } else { +// state = GameboardItemState.IN_PROGRESS; +// } gameItem.setState(state); } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/UserAttemptManager.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/UserAttemptManager.java index 447b4b78d1..cf5a0934ed 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/UserAttemptManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/managers/UserAttemptManager.java @@ -71,34 +71,42 @@ private void augmentContentSummaryWithAttemptInformation( String questionId = contentSummary.getId(); Map> questionAttempts = usersQuestionAttempts.get(questionId); boolean questionAnsweredCorrectly = false; - boolean attempted = false; + int questionPartsCorrect = 0; + int questionPartsIncorrect = 0; + int questionPartsTotal = contentSummary.getQuestionPartIds().size(); if (questionAttempts != null) { for (String relatedQuestionPartId : contentSummary.getQuestionPartIds()) { - questionAnsweredCorrectly = false; List questionPartAttempts = questionAttempts.get(relatedQuestionPartId); if (questionPartAttempts != null) { - attempted = true; for (LightweightQuestionValidationResponse partAttempt : questionPartAttempts) { questionAnsweredCorrectly = partAttempt.isCorrect(); if (questionAnsweredCorrectly) { + questionPartsCorrect++; break; // exit on first correct attempt } } - } - if (!questionAnsweredCorrectly) { - break; // exit on first false question part + if (!questionAnsweredCorrectly) { + questionPartsIncorrect++; + } } } } - if (attempted) { - if (questionAnsweredCorrectly) { - contentSummary.setState(Constants.CompletionState.ALL_CORRECT); - } else { - contentSummary.setState(Constants.CompletionState.IN_PROGRESS); - } + int questionPartsNotAttempted = questionPartsTotal - (questionPartsCorrect + questionPartsIncorrect); + + // TODO: move to common function with GameManagerL1198 + CompletionState state; + if (questionPartsCorrect == questionPartsTotal) { + state = CompletionState.ALL_CORRECT; + } else if (questionPartsIncorrect == questionPartsTotal) { + state = CompletionState.ALL_INCORRECT; + } else if (questionPartsNotAttempted == questionPartsTotal) { + state = CompletionState.NOT_ATTEMPTED; + } else if (questionPartsNotAttempted > 0) { + state = CompletionState.IN_PROGRESS; } else { - contentSummary.setState(Constants.CompletionState.NOT_ATTEMPTED); + state = CompletionState.ALL_ATTEMPTED; } + contentSummary.setState(state); } /** diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AssignmentProgressDTO.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AssignmentProgressDTO.java index ac1424d674..7e1714b9bf 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AssignmentProgressDTO.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/AssignmentProgressDTO.java @@ -12,7 +12,7 @@ public class AssignmentProgressDTO { public UserSummaryDTO user; public List correctPartResults; public List incorrectPartResults; - public List results; + public List results; /** * Complete AssignmentProgressDTO constructor with all dependencies. @@ -25,7 +25,7 @@ public class AssignmentProgressDTO { * @param results * - Array of results. */ - public AssignmentProgressDTO(UserSummaryDTO user, List correctPartResults, List incorrectPartResults, List results) { + public AssignmentProgressDTO(UserSummaryDTO user, List correctPartResults, List incorrectPartResults, List results) { this.user = user; this.correctPartResults = correctPartResults; this.incorrectPartResults = incorrectPartResults; @@ -59,11 +59,11 @@ public void setIncorrectPartResults(List incorrectPartResults) { this.incorrectPartResults = incorrectPartResults; } - public List getResults() { + public List getResults() { return results; } - public void setResults(List results) { + public void setResults(List results) { this.results = results; } } diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/GameboardItem.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/GameboardItem.java index 8534c32b77..8b58a6624b 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/GameboardItem.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/dto/GameboardItem.java @@ -17,7 +17,6 @@ import com.google.common.collect.Lists; import uk.ac.cam.cl.dtg.isaac.api.Constants; -import uk.ac.cam.cl.dtg.isaac.api.Constants.GameboardItemState; import uk.ac.cam.cl.dtg.isaac.dos.AudienceContext; import uk.ac.cam.cl.dtg.isaac.dos.GameboardContentDescriptor; @@ -46,7 +45,7 @@ public class GameboardItem { private Integer questionPartsNotAttempted; private Integer questionPartsTotal; private Float passMark; - private GameboardItemState state; + private Constants.CompletionState state; private List questionPartStates = Lists.newArrayList(); // optional field if we want to use the gameboard item outside the context of a board. @@ -346,7 +345,7 @@ public final void setPassMark(final Float passMark) { * * @return the state */ - public final GameboardItemState getState() { + public final Constants.CompletionState getState() { return state; } @@ -356,7 +355,7 @@ public final GameboardItemState getState() { * @param state * the state to set */ - public final void setState(final GameboardItemState state) { + public final void setState(final Constants.CompletionState state) { this.state = state; } diff --git a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManager.java b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManager.java index 00e9d6ae93..57948c1a51 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManager.java +++ b/src/main/java/uk/ac/cam/cl/dtg/segue/api/managers/GroupManager.java @@ -703,8 +703,8 @@ public List getGroupProgressSummary(List results = studentResults.getResults(); + List results = studentResults.getResults(); assertTrue(results != null && results.isEmpty()); break; }