Skip to content

Commit 4903b9d

Browse files
jsharkey13jsharkey13
authored
Merge pull request #694 from isaacphysics/feature/anon-users-list-quizzes
Let anonymous users list student-visible quizzes
2 parents f5c01a1 + ec1f263 commit 4903b9d

File tree

4 files changed

+30
-18
lines changed

4 files changed

+30
-18
lines changed

‎src/main/java/uk/ac/cam/cl/dtg/isaac/api/QuizFacade.java

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import uk.ac.cam.cl.dtg.isaac.dos.QuizFeedbackMode;
3838
import uk.ac.cam.cl.dtg.isaac.dos.content.Content;
3939
import uk.ac.cam.cl.dtg.isaac.dos.content.Question;
40+
import uk.ac.cam.cl.dtg.isaac.dos.users.Role;
4041
import uk.ac.cam.cl.dtg.isaac.dto.AssignmentStatusDTO;
4142
import uk.ac.cam.cl.dtg.isaac.dto.IsaacQuestionBaseDTO;
4243
import uk.ac.cam.cl.dtg.isaac.dto.IsaacQuizDTO;
@@ -54,6 +55,7 @@
5455
import uk.ac.cam.cl.dtg.isaac.dto.content.ContentBaseDTO;
5556
import uk.ac.cam.cl.dtg.isaac.dto.content.ContentSummaryDTO;
5657
import uk.ac.cam.cl.dtg.isaac.dto.content.DetailedQuizSummaryDTO;
58+
import uk.ac.cam.cl.dtg.isaac.dto.users.AbstractSegueUserDTO;
5759
import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO;
5860
import uk.ac.cam.cl.dtg.isaac.dto.users.UserSummaryDTO;
5961
import uk.ac.cam.cl.dtg.segue.api.ErrorResponseWrapper;
@@ -161,7 +163,7 @@ public QuizFacade(final AbstractConfigLoader properties, final ILogManager logMa
161163
/**
162164
* Get quizzes visible to this user, starting from index 0.
163165
*
164-
* Anonymous users can't see quizzes.
166+
* See {@link #getAvailableQuizzes(Request, HttpServletRequest, Integer)}.
165167
*
166168
* @return a Response containing a list of ContentSummaryDTO for the visible quizzes.
167169
*/
@@ -176,7 +178,7 @@ public final Response getAvailableQuizzes(@Context final Request request, @Conte
176178
/**
177179
* Get quizzes visible to this user, starting from the specified index.
178180
*
179-
* Anonymous users can't see quizzes.
181+
* Anonymous users can't view or take quizzes, but can list STUDENT quizzes using this endpoint.
180182
*
181183
* @param request the Request needed for ETag checking.
182184
* @param httpServletRequest the Request needed for Cookies for the current user.
@@ -187,17 +189,21 @@ public final Response getAvailableQuizzes(@Context final Request request, @Conte
187189
@Path("/available/{startIndex}")
188190
@Produces(MediaType.APPLICATION_JSON)
189191
@GZIP
190-
@Operation(summary = "Get tests visible to this user, from the specified index.")
192+
@Operation(summary = "Get tests visible to this user, from the specified index.",
193+
description = "Anonymous users can list student quizzes, but cannot take them.")
191194
public final Response getAvailableQuizzes(@Context final Request request,
192195
@Context final HttpServletRequest httpServletRequest,
193196
@PathParam("startIndex") final Integer startIndex) {
194197
try {
195-
RegisteredUserDTO user = this.userManager.getCurrentRegisteredUser(httpServletRequest);
196198

197-
String userRoleString = user.getRole().name();
199+
String userRoleString = Role.STUDENT.name(); // Allow anonymous users to list STUDENT quizzes.
200+
AbstractSegueUserDTO currentUser = userManager.getCurrentUser(httpServletRequest);
201+
if (currentUser instanceof RegisteredUserDTO) {
202+
userRoleString = ((RegisteredUserDTO) currentUser).getRole().name();
203+
}
198204

199205
// Cache the list of quizzes based on current content version, user's role, and startIndex:
200-
int etagValue = this.contentManager.getCurrentContentSHA().hashCode() + user.getRole().hashCode();
206+
int etagValue = this.contentManager.getCurrentContentSHA().hashCode() + userRoleString.hashCode();
201207
if (null != startIndex) {
202208
etagValue += startIndex;
203209
}
@@ -220,8 +226,10 @@ public final Response getAvailableQuizzes(@Context final Request request,
220226
String message = "ContentManagerException whilst getting available tests";
221227
log.error(message, e);
222228
return new SegueErrorResponse(Status.INTERNAL_SERVER_ERROR, message).toResponse();
223-
} catch (NoUserLoggedInException e) {
224-
return SegueErrorResponse.getNotLoggedInResponse();
229+
} catch (SegueDatabaseException e) {
230+
String message = "Database error whilst getting available tests";
231+
log.error(message, e);
232+
return new SegueErrorResponse(Response.Status.INTERNAL_SERVER_ERROR, message).toResponse();
225233
}
226234
}
227235

@@ -256,7 +264,7 @@ public final Response getAssignedQuizzes(@Context final HttpServletRequest reque
256264
return Response.ok(assignments)
257265
.cacheControl(getCacheControl(NEVER_CACHE_WITHOUT_ETAG_CHECK, false)).build();
258266
} catch (SegueDatabaseException e) {
259-
String message = "SegueDatabaseException whilst getting available tests";
267+
String message = "Database error whilst getting available tests";
260268
log.error(message, e);
261269
return new SegueErrorResponse(Response.Status.INTERNAL_SERVER_ERROR, message).toResponse();
262270
} catch (NoUserLoggedInException e) {
@@ -287,7 +295,7 @@ public final Response getFreeAttempts(@Context final HttpServletRequest request)
287295
return Response.ok(attempts)
288296
.cacheControl(getCacheControl(NEVER_CACHE_WITHOUT_ETAG_CHECK, false)).build();
289297
} catch (SegueDatabaseException e) {
290-
String message = "SegueDatabaseException whilst getting available tests";
298+
String message = "Database error whilst getting available tests";
291299
log.error(message, e);
292300
return new SegueErrorResponse(Response.Status.INTERNAL_SERVER_ERROR, message).toResponse();
293301
} catch (NoUserLoggedInException e) {

‎src/test/java/uk/ac/cam/cl/dtg/isaac/IsaacTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import uk.ac.cam.cl.dtg.isaac.dto.ResultsWrapper;
3939
import uk.ac.cam.cl.dtg.isaac.dto.UserGroupDTO;
4040
import uk.ac.cam.cl.dtg.isaac.dto.content.QuizSummaryDTO;
41+
import uk.ac.cam.cl.dtg.isaac.dto.users.AnonymousUserDTO;
4142
import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO;
4243
import uk.ac.cam.cl.dtg.isaac.dto.users.UserSummaryWithEmailAddressDTO;
4344
import uk.ac.cam.cl.dtg.segue.api.managers.GroupManager;
@@ -86,6 +87,7 @@ public class IsaacTest {
8687
protected RegisteredUserDTO secondTeacher;
8788
protected RegisteredUserDTO otherTeacher;
8889
protected RegisteredUserDTO noone;
90+
protected AnonymousUserDTO anonUser;
8991
protected RegisteredUserDTO secondStudent;
9092
protected RegisteredUserDTO otherStudent;
9193
protected RegisteredUserDTO adminUser;
@@ -190,6 +192,7 @@ protected void initializeIsaacObjects() {
190192
otherTeacher.setId(++id);
191193

192194
noone = null;
195+
anonUser = new AnonymousUserDTO("fake-session-id");
193196

194197
secondStudent = new RegisteredUserDTO("Second", "Student", "second-student@test.com", EmailVerificationStatus.VERIFIED, somePastDate, Gender.FEMALE, somePastDate, "", null, false);
195198
secondStudent.setRole(Role.STUDENT);

‎src/test/java/uk/ac/cam/cl/dtg/isaac/api/AbstractFacadeTest.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,21 +17,21 @@
1717

1818
import com.google.api.client.util.Maps;
1919
import com.google.common.base.Joiner;
20-
import jakarta.ws.rs.core.Request;
2120
import org.junit.Before;
2221
import org.junit.runner.RunWith;
2322
import org.powermock.core.classloader.annotations.PowerMockIgnore;
2423
import org.powermock.core.classloader.annotations.PrepareForTest;
2524
import org.powermock.modules.junit4.PowerMockRunner;
2625
import uk.ac.cam.cl.dtg.isaac.IsaacTest;
27-
import uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager;
28-
import uk.ac.cam.cl.dtg.segue.auth.exceptions.NoUserLoggedInException;
2926
import uk.ac.cam.cl.dtg.isaac.dto.SegueErrorResponse;
3027
import uk.ac.cam.cl.dtg.isaac.dto.users.RegisteredUserDTO;
3128
import uk.ac.cam.cl.dtg.isaac.dto.users.UserSummaryDTO;
29+
import uk.ac.cam.cl.dtg.segue.api.managers.UserAccountManager;
30+
import uk.ac.cam.cl.dtg.segue.auth.exceptions.NoUserLoggedInException;
3231

3332
import jakarta.annotation.Nullable;
3433
import jakarta.servlet.http.HttpServletRequest;
34+
import jakarta.ws.rs.core.Request;
3535
import jakarta.ws.rs.core.Response;
3636
import jakarta.ws.rs.core.Response.Status;
3737
import java.util.ArrayList;
@@ -105,7 +105,7 @@ public void abstractFacadeTestSetup() {
105105
request = createNiceMock(Request.class); // We don't particularly care about what gets called on this.
106106
replay(request);
107107

108-
userManager = createPartialMock(UserAccountManager.class, "getCurrentRegisteredUser", "convertToUserSummaryObject", "getUserDTOById");
108+
userManager = createPartialMock(UserAccountManager.class, "getCurrentUser", "getCurrentRegisteredUser", "convertToUserSummaryObject", "getUserDTOById");
109109

110110
registerDefaultsFor(userManager, m -> {
111111
expect(m.convertToUserSummaryObject(anyObject(RegisteredUserDTO.class))).andStubAnswer(() -> {
@@ -428,9 +428,11 @@ private void runSteps(Endpoint endpoint) {
428428
private void runStepsAs(@Nullable RegisteredUserDTO user, Endpoint endpoint) {
429429
withMock(userManager, m -> {
430430
if (user == null) {
431-
expect(m.getCurrentRegisteredUser(httpServletRequest)).andThrow(new NoUserLoggedInException());
431+
expect(m.getCurrentRegisteredUser(httpServletRequest)).andThrow(new NoUserLoggedInException()).anyTimes();
432+
expect(m.getCurrentUser(httpServletRequest)).andReturn(anonUser).anyTimes();
432433
} else {
433-
expect(m.getCurrentRegisteredUser(httpServletRequest)).andReturn(user);
434+
expect(m.getCurrentRegisteredUser(httpServletRequest)).andReturn(user).anyTimes();
435+
expect(m.getCurrentUser(httpServletRequest)).andReturn(user).anyTimes();
434436
}
435437
});
436438
currentUser = user;

‎src/test/java/uk/ac/cam/cl/dtg/isaac/api/QuizFacadeTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,7 @@ public void setUp() throws ContentManagerException {
183183
@Test
184184
public void availableQuizzes() {
185185
forEndpoint(() -> quizFacade.getAvailableQuizzes(request, httpServletRequest),
186-
requiresLogin(),
187-
as(anyOf(student, secondStudent),
186+
as(anyOf(noone, student, secondStudent),
188187
check((response) ->
189188
assertEquals(Collections.singletonList(studentQuizSummary), extractResults(response)))
190189
),

0 commit comments

Comments
 (0)