Skip to content

Allow using user bookmarks list as question source #799

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
jacbn merged 2 commits into from
Jun 18, 2026
Merged

Allow using user bookmarks list as question source #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f88229d
Allow using user bookmarks list as question source
jacbn Jun 12, 2026
f1cf09e
Replace search by IDs test with search by bookmarks test
axlewin Jun 17, 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
35 changes: 24 additions & 11 deletions src/main/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacade.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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<String, Set<String>> fieldsToMatch = Maps.newHashMap();
Set<CompletionState> filterByStatuses;
AbstractSegueUserDTO user;
Expand Down Expand Up @@ -390,10 +392,18 @@ public final Response getQuestionList(@Context final HttpServletRequest httpServ
startIndex = paramStartIndex;
}

if (ids != null && !ids.isEmpty()) {
Set<String> idsList = Set.of(ids.split(","));
fieldsToMatch.put(ID_FIELDNAME, idsList);
limit = idsList.size();
List<BookmarkDO> bookmarks = null;
if (searchBookmarks) {
if (user instanceof RegisteredUserDTO registeredUser) {
bookmarks = bookmarksManager.getBookmarksForUser(registeredUser.getId(), "isaacQuestionPage");
Set<String> 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)) {
Expand All @@ -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<String, String> fieldNameToValues = new HashMap<>();
fieldNameToValues.put(ID_FIELDNAME, ids);
fieldNameToValues.put(TAGS_FIELDNAME, tags);
fieldNameToValues.put(SUBJECTS_FIELDNAME, subjects);
fieldNameToValues.put(FIELDS_FIELDNAME, fields);
Expand Down Expand Up @@ -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) {
Expand Down
33 changes: 19 additions & 14 deletions src/test/java/uk/ac/cam/cl/dtg/isaac/api/PagesFacadeIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,22 @@
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;

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.*;


Expand All @@ -58,7 +61,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
Expand All @@ -84,7 +87,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
Expand All @@ -110,7 +113,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
Expand All @@ -125,30 +128,32 @@ public void getQuestionList_searchByIDOfNonFastTrackQuestionPageAsStudentWhenFas
}

@Test
public void getQuestionList_searchSpecificIDsAsStudent_returnsOnlyQuestionsWithIDs() throws Exception {
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,
String.format("%s,%s", ITConstants.REGRESSION_TEST_PAGE_ID, ITConstants.ASSIGNMENT_TEST_PAGE_ID), "",
"", "", "", "", "", "", "", "", "", "", "", false, 0, MAX_SEARCH_RESULT_LIMIT, null, null);
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 the expected content summary
// check the search returned only the bookmarked content
@SuppressWarnings("unchecked") ResultsWrapper<ContentSummaryDTO> responseBody =
(ResultsWrapper<ContentSummaryDTO>) searchResponse.getEntity();

Set<String> questionIDs = responseBody.getResults().stream().map(ContentSummaryDTO::getId).collect(Collectors.toSet());
assertEquals(Set.of(ITConstants.REGRESSION_TEST_PAGE_ID, ITConstants.ASSIGNMENT_TEST_PAGE_ID), questionIDs);
assertEquals(Set.of(ITConstants.REGRESSION_TEST_PAGE_ID), questionIDs);
}

@Test
Expand All @@ -163,7 +168,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
Expand Down Expand Up @@ -198,7 +203,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
Expand Down Expand Up @@ -229,7 +234,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
Expand Down Expand Up @@ -259,7 +264,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
Expand All @@ -285,7 +290,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
Expand Down
Loading