From f88229dbff467a24feab493b1d98fe6a6bb102df Mon Sep 17 00:00:00 2001 From: Jaycie Brown Date: Fri, 12 Jun 2026 09:53:36 +0100 Subject: [PATCH 1/2] Allow using user bookmarks list as question source --- .../ac/cam/cl/dtg/isaac/api/PagesFacade.java | 35 +++++++--- .../cam/cl/dtg/isaac/api/PagesFacadeIT.java | 68 +++++++++---------- 2 files changed, 58 insertions(+), 45 deletions(-) diff --git a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java index 0e4d355946..dfd1cd4a97 100644 --- a/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java +++ b/src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java @@ -27,6 +27,7 @@ import uk.ac.cam.cl.dtg.isaac.api.managers.GameManager; import uk.ac.cam.cl.dtg.isaac.api.managers.URIManager; import uk.ac.cam.cl.dtg.isaac.api.managers.UserAttemptManager; +import uk.ac.cam.cl.dtg.isaac.dos.BookmarkDO; import uk.ac.cam.cl.dtg.isaac.dos.IsaacTopicSummaryPage; import uk.ac.cam.cl.dtg.isaac.dos.LightweightQuestionValidationResponse; import uk.ac.cam.cl.dtg.isaac.dos.QuestionValidationResponse; @@ -319,8 +320,6 @@ public final Response getConcept(@Context final Request request, @Context final /** * REST end point to provide a list of questions. * - * @param ids - * - the ids of the concepts to request. * @param searchString * - an optional search string to allow finding of questions by title. * @param tags @@ -334,6 +333,8 @@ public final Response getConcept(@Context final Request request, @Context final * - a string value to be converted into an integer which represents the start index of the results * @param paramLimit * - a string value to be converted into an integer that represents the number of results to return. + * @param searchBookmarks + * - whether the user's bookmarks should be used as the source of questions. * @return A response object which contains a list of questions or an empty list. */ @GET @@ -342,7 +343,7 @@ public final Response getConcept(@Context final Request request, @Context final @GZIP @Operation(summary = "List all question page objects matching the provided criteria.") public final Response getQuestionList(@Context final HttpServletRequest httpServletRequest, - @QueryParam("ids") final String ids, @QueryParam("searchString") final String searchString, + @QueryParam("searchString") final String searchString, @QueryParam("tags") final String tags, @QueryParam("levels") final String level, @QueryParam("subjects") final String subjects, @QueryParam("fields") final String fields, @QueryParam("topics") final String topics, @@ -353,7 +354,8 @@ public final Response getQuestionList(@Context final HttpServletRequest httpServ @DefaultValue("false") @QueryParam("fasttrack") final Boolean fasttrack, @DefaultValue(DEFAULT_START_INDEX_AS_STRING) @QueryParam("startIndex") final Integer paramStartIndex, @DefaultValue(DEFAULT_RESULTS_LIMIT_AS_STRING) @QueryParam("limit") final Integer paramLimit, - @QueryParam("randomSeed") final Long randomSeed, @QueryParam("querySource") final String querySource) { + @QueryParam("randomSeed") final Long randomSeed, @QueryParam("querySource") final String querySource, + @DefaultValue("false") @QueryParam("bookmarks") final Boolean searchBookmarks) { Map> fieldsToMatch = Maps.newHashMap(); Set filterByStatuses; AbstractSegueUserDTO user; @@ -390,10 +392,18 @@ public final Response getQuestionList(@Context final HttpServletRequest httpServ startIndex = paramStartIndex; } - if (ids != null && !ids.isEmpty()) { - Set idsList = Set.of(ids.split(",")); - fieldsToMatch.put(ID_FIELDNAME, idsList); - limit = idsList.size(); + List bookmarks = null; + if (searchBookmarks) { + if (user instanceof RegisteredUserDTO registeredUser) { + bookmarks = bookmarksManager.getBookmarksForUser(registeredUser.getId(), "isaacQuestionPage"); + Set bookmarkIds = bookmarks.stream() + .map(BookmarkDO::contentId) + .collect(Collectors.toSet()); + fieldsToMatch.put(ID_FIELDNAME, bookmarkIds); + limit = bookmarkIds.size(); + } else { + return SegueErrorResponse.getBadRequestResponse("Anonymous users cannot search bookmarks."); + } } if (null == querySource || !QUESTION_SEARCH_LOG_SOURCE_IGNORES.contains(querySource)) { @@ -410,19 +420,18 @@ public final Response getQuestionList(@Context final HttpServletRequest httpServ logEntry.put(TAGS_FIELDNAME, csvParamToLogValue(tags)); logEntry.put(QUESTION_STATUSES_FIELDNAME, csvParamToLogValue(statuses)); logEntry.put("levels", csvParamToLogValue(level)); - logEntry.put("questionIds", csvParamToLogValue(ids)); logEntry.put(START_INDEX_FIELDNAME, String.valueOf(startIndex)); logEntry.put(LIMIT_FIELDNAME, String.valueOf(limit)); logEntry.put(SEARCH_STRING_FIELDNAME, !Objects.equals(searchString, "") ? searchString : null); logEntry.put("fasttrack", Objects.equals(fasttrack, true) ? String.valueOf(fasttrack) : null); logEntry.put("randomSeed", null != randomSeed ? String.valueOf(randomSeed) : null); logEntry.put("querySource", querySource); + logEntry.put("searchBookmarks", searchBookmarks); this.getLogManager().logEvent(user, httpServletRequest, IsaacServerLogType.QUESTION_FINDER_SEARCH, logEntry); } Map fieldNameToValues = new HashMap<>(); - fieldNameToValues.put(ID_FIELDNAME, ids); fieldNameToValues.put(TAGS_FIELDNAME, tags); fieldNameToValues.put(SUBJECTS_FIELDNAME, subjects); fieldNameToValues.put(FIELDS_FIELDNAME, fields); @@ -515,7 +524,11 @@ public final Response getQuestionList(@Context final HttpServletRequest httpServ } if (user instanceof RegisteredUserDTO registeredUser) { - summarizedResults = bookmarksManager.augmentContentSummaryListWithBookmarkInformation(registeredUser.getId(), summarizedResults); + if (bookmarks == null) { + summarizedResults = bookmarksManager.augmentContentSummaryListWithBookmarkInformation(registeredUser.getId(), summarizedResults); + } else { + summarizedResults = bookmarksManager.augmentContentSummaryListWithBookmarkInformation(bookmarks, summarizedResults); + } } if (limit < 0 || combinedResults.size() + summarizedResults.size() <= limit) { diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java index 7d6875f30b..4f37db4a1a 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java @@ -58,7 +58,7 @@ public void getQuestionList_searchSpecificIDAsStudent_returnsOnlyQuestionWithID( // Act // make request Response searchResponse = pagesFacade.getQuestionList(searchRequest, - ITConstants.REGRESSION_TEST_PAGE_ID, "", "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null); + ITConstants.REGRESSION_TEST_PAGE_ID, "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null, false); // Assert // check status code is OK @@ -84,7 +84,7 @@ public void getQuestionList_searchByIDOfNonQuestionPageAsStudent_doesNotReturnPa // Act // make request Response searchResponse = pagesFacade.getQuestionList(searchRequest, - ITConstants.SEARCH_TEST_CONCEPT_ID, "", "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null); + ITConstants.SEARCH_TEST_CONCEPT_ID, "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null, false); // Assert // check status code is OK @@ -110,7 +110,7 @@ public void getQuestionList_searchByIDOfNonFastTrackQuestionPageAsStudentWhenFas // Act // make request Response searchResponse = pagesFacade.getQuestionList(searchRequest, - ITConstants.REGRESSION_TEST_PAGE_ID, "", "", "", "", "", "", "", "", "", "", "", "", true, 0, MAX_SEARCH_RESULT_LIMIT, null, null); + ITConstants.REGRESSION_TEST_PAGE_ID, "", "", "", "", "", "", "", "", "", "", "", true, 0, MAX_SEARCH_RESULT_LIMIT, null, null, false); // Assert // check status code is OK @@ -124,32 +124,32 @@ public void getQuestionList_searchByIDOfNonFastTrackQuestionPageAsStudentWhenFas assertEquals(Set.of(), questionIDs); } - @Test - public void getQuestionList_searchSpecificIDsAsStudent_returnsOnlyQuestionsWithIDs() throws Exception { - // Arrange - // log in as Student, create request - LoginResult studentLogin = loginAs(httpSession, ITConstants.TEST_STUDENT_EMAIL, - ITConstants.TEST_STUDENT_PASSWORD); - HttpServletRequest searchRequest = createRequestWithCookies(new Cookie[]{studentLogin.cookie}); - replay(searchRequest); - - // Act - // make request - Response searchResponse = pagesFacade.getQuestionList(searchRequest, - String.format("%s,%s", ITConstants.REGRESSION_TEST_PAGE_ID, ITConstants.ASSIGNMENT_TEST_PAGE_ID), "", - "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null); - - // Assert - // check status code is OK - assertEquals(Response.Status.OK.getStatusCode(), searchResponse.getStatus()); - - // check the search returned the expected content summary - @SuppressWarnings("unchecked") ResultsWrapper responseBody = - (ResultsWrapper) searchResponse.getEntity(); - - Set questionIDs = responseBody.getResults().stream().map(ContentSummaryDTO::getId).collect(Collectors.toSet()); - assertEquals(Set.of(ITConstants.REGRESSION_TEST_PAGE_ID, ITConstants.ASSIGNMENT_TEST_PAGE_ID), questionIDs); - } +// @Test +// public void getQuestionList_searchSpecificIDsAsStudent_returnsOnlyQuestionsWithIDs() throws Exception { +// // Arrange +// // log in as Student, create request +// LoginResult studentLogin = loginAs(httpSession, ITConstants.TEST_STUDENT_EMAIL, +// ITConstants.TEST_STUDENT_PASSWORD); +// HttpServletRequest searchRequest = createRequestWithCookies(new Cookie[]{studentLogin.cookie}); +// replay(searchRequest); +// +// // Act +// // make request +// Response searchResponse = pagesFacade.getQuestionList(searchRequest, +// String.format("%s,%s", ITConstants.REGRESSION_TEST_PAGE_ID, ITConstants.ASSIGNMENT_TEST_PAGE_ID), "", +// "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null); +// +// // Assert +// // check status code is OK +// assertEquals(Response.Status.OK.getStatusCode(), searchResponse.getStatus()); +// +// // check the search returned the expected content summary +// @SuppressWarnings("unchecked") ResultsWrapper responseBody = +// (ResultsWrapper) searchResponse.getEntity(); +// +// Set questionIDs = responseBody.getResults().stream().map(ContentSummaryDTO::getId).collect(Collectors.toSet()); +// assertEquals(Set.of(ITConstants.REGRESSION_TEST_PAGE_ID, ITConstants.ASSIGNMENT_TEST_PAGE_ID), questionIDs); +// } @Test public void getQuestionList_searchByStringAsStudent_returnsQuestionsWithSimilarTitlesInOrder() throws Exception { @@ -163,7 +163,7 @@ public void getQuestionList_searchByStringAsStudent_returnsQuestionsWithSimilarT // Act // make request Response searchResponse = pagesFacade.getQuestionList(searchRequest, - "", "Regression Test Page", "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null); + "Regression Test Page", "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null, false); // Assert // check status code is OK @@ -198,7 +198,7 @@ public void getQuestionList_limitedSearchByStringAsStudent_returnsLimitedNumberO // Act // make request Response searchResponse = pagesFacade.getQuestionList(searchRequest, - "", "Regression Test Page", "", "", "", "", "", "", "", "", "", "", "", false, 0, 1, null, null); + "Regression Test Page", "", "", "", "", "", "", "", "", "", "", "", false, 0, 1, null, null, false); // Assert // check status code is OK @@ -229,7 +229,7 @@ public void getQuestionList_searchForPhrase_returnExactlyMatchesPhrase() throws // Act // make request Response searchResponse = pagesFacade.getQuestionList(searchRequest, - "", "Canary", "", "", "", "", "", "", "", "", "", "", "", false, 0, 1, null, null); + "Canary", "", "", "", "", "", "", "", "", "", "", "", false, 0, 1, null, null, false); // Assert // check status code is OK @@ -259,7 +259,7 @@ public void getQuestionList_searchForSupersededPageAsStudent_returnsPageThatSupe // Act // make request Response searchResponse = pagesFacade.getQuestionList(searchRequest, - "", "Convival", "", "", "", "", "", "", "", "", "", "", "", false, 0, 1, null, null); + "Convival", "", "", "", "", "", "", "", "", "", "", "", false, 0, 1, null, null, false); // Assert // check status code is OK @@ -285,7 +285,7 @@ public void getQuestionList_searchForSupersededPageAsTeacher_returnsBoth() throw // Act // make request Response searchResponse = pagesFacade.getQuestionList(searchRequest, - "", "Convival", "", "", "", "", "", "", "", "", "", "", "", false, 0, 1, null, null); + "Convival", "", "", "", "", "", "", "", "", "", "", "", false, 0, 1, null, null, false); // Assert // check status code is OK From f1cf09eb69992c75571424f0005e17364853ddb0 Mon Sep 17 00:00:00 2001 From: Alex Lewin Date: Wed, 17 Jun 2026 15:59:54 +0100 Subject: [PATCH 2/2] Replace search by IDs test with search by bookmarks test to reflect changes to this endpoint --- .../cam/cl/dtg/isaac/api/PagesFacadeIT.java | 57 ++++++++++--------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java index 4f37db4a1a..8e085c600b 100644 --- a/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java +++ b/src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java @@ -19,6 +19,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import uk.ac.cam.cl.dtg.isaac.api.managers.URIManager; +import uk.ac.cam.cl.dtg.isaac.dos.BookmarkDO; import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper; import uk.ac.cam.cl.dtg.isaac.dto.content.ContentSummaryDTO; import uk.ac.cam.cl.dtg.segue.api.services.ContentService; @@ -26,12 +27,14 @@ import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; import jakarta.ws.rs.core.Response; +import java.util.Date; import java.util.List; import java.util.Set; import java.util.stream.Collectors; import static org.easymock.EasyMock.replay; import static org.junit.jupiter.api.Assertions.assertEquals; +import static uk.ac.cam.cl.dtg.isaac.api.Constants.*; import static uk.ac.cam.cl.dtg.segue.api.Constants.*; @@ -124,32 +127,34 @@ public void getQuestionList_searchByIDOfNonFastTrackQuestionPageAsStudentWhenFas assertEquals(Set.of(), questionIDs); } -// @Test -// public void getQuestionList_searchSpecificIDsAsStudent_returnsOnlyQuestionsWithIDs() throws Exception { -// // Arrange -// // log in as Student, create request -// LoginResult studentLogin = loginAs(httpSession, ITConstants.TEST_STUDENT_EMAIL, -// ITConstants.TEST_STUDENT_PASSWORD); -// HttpServletRequest searchRequest = createRequestWithCookies(new Cookie[]{studentLogin.cookie}); -// replay(searchRequest); -// -// // Act -// // make request -// Response searchResponse = pagesFacade.getQuestionList(searchRequest, -// String.format("%s,%s", ITConstants.REGRESSION_TEST_PAGE_ID, ITConstants.ASSIGNMENT_TEST_PAGE_ID), "", -// "", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null); -// -// // Assert -// // check status code is OK -// assertEquals(Response.Status.OK.getStatusCode(), searchResponse.getStatus()); -// -// // check the search returned the expected content summary -// @SuppressWarnings("unchecked") ResultsWrapper responseBody = -// (ResultsWrapper) searchResponse.getEntity(); -// -// Set questionIDs = responseBody.getResults().stream().map(ContentSummaryDTO::getId).collect(Collectors.toSet()); -// assertEquals(Set.of(ITConstants.REGRESSION_TEST_PAGE_ID, ITConstants.ASSIGNMENT_TEST_PAGE_ID), questionIDs); -// } + @Test + public void getQuestionList_searchBookmarksAsStudent_returnsOnlyBookmarks() throws Exception { + // Arrange + // log in as Student, create request + LoginResult studentLogin = loginAs(httpSession, ITConstants.TEST_STUDENT_EMAIL, + ITConstants.TEST_STUDENT_PASSWORD); + HttpServletRequest searchRequest = createRequestWithCookies(new Cookie[]{studentLogin.cookie}); + replay(searchRequest); + // add bookmark that should be included in search results + BookmarkDO bookmark = new BookmarkDO(ITConstants.TEST_STUDENT_ID, ITConstants.REGRESSION_TEST_PAGE_ID, QUESTION_TYPE, new Date()); + bookmarksManager.addBookmarkForUser(bookmark); + + // Act + // make request + Response searchResponse = pagesFacade.getQuestionList(searchRequest, "", "", "", "", "", "", "", "", "", "", + "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null, true); + + // Assert + // check status code is OK + assertEquals(Response.Status.OK.getStatusCode(), searchResponse.getStatus()); + + // check the search returned only the bookmarked content + @SuppressWarnings("unchecked") ResultsWrapper responseBody = + (ResultsWrapper) searchResponse.getEntity(); + + Set questionIDs = responseBody.getResults().stream().map(ContentSummaryDTO::getId).collect(Collectors.toSet()); + assertEquals(Set.of(ITConstants.REGRESSION_TEST_PAGE_ID), questionIDs); + } @Test public void getQuestionList_searchByStringAsStudent_returnsQuestionsWithSimilarTitlesInOrder() throws Exception {