Skip to content

Commit

Permalink
refactor(PeerGroup): Pass in domain objects to controllers (#99)
Browse files Browse the repository at this point in the history
  • Loading branch information
@hirokiterashima
hirokiterashima authored Feb 17, 2022
1 parent 9071fa3 commit 77dfa39
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 119 deletions.
21 changes: 8 additions & 13 deletions ...n/java/org/wise/portal/presentation/web/controllers/peergroup/PeerGroupAPIController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.wise.portal.dao.ObjectNotFoundException;
import org.wise.portal.domain.peergroup.PeerGroup;
import org.wise.portal.domain.peergroupactivity.PeerGroupActivity;
import org.wise.portal.domain.run.Run;
import org.wise.portal.domain.run.impl.RunImpl;
import org.wise.portal.domain.user.User;
import org.wise.portal.domain.workgroup.Workgroup;
import org.wise.portal.domain.workgroup.impl.WorkgroupImpl;
import org.wise.portal.service.peergroup.PeerGroupActivityThresholdNotSatisfiedException;
import org.wise.portal.service.peergroup.PeerGroupCreationException;
import org.wise.portal.service.peergroup.PeerGroupService;
import org.wise.portal.service.peergroupactivity.PeerGroupActivityNotFoundException;
import org.wise.portal.service.peergroupactivity.PeerGroupActivityService;
import org.wise.portal.service.run.RunService;
import org.wise.portal.service.user.UserService;
import org.wise.portal.service.workgroup.WorkgroupService;

Expand All @@ -52,9 +52,6 @@
@RequestMapping("/api/peer-group")
public class PeerGroupAPIController {

@Autowired
private RunService runService;

@Autowired
private UserService userService;

Expand All @@ -68,12 +65,11 @@ public class PeerGroupAPIController {
private PeerGroupActivityService peerGroupActivityService;

@GetMapping("/{runId}/{workgroupId}/{peerGroupActivityTag}")
PeerGroup getPeerGroup(@PathVariable Long runId, @PathVariable Long workgroupId,
PeerGroup getPeerGroup(@PathVariable("runId") RunImpl run,
@PathVariable("workgroupId") WorkgroupImpl workgroup,
@PathVariable String peerGroupActivityTag, Authentication auth)
throws JSONException, ObjectNotFoundException, PeerGroupActivityNotFoundException,
throws JSONException, PeerGroupActivityNotFoundException,
PeerGroupCreationException, PeerGroupActivityThresholdNotSatisfiedException {
Run run = runService.retrieveById(runId);
Workgroup workgroup = workgroupService.retrieveById(workgroupId);
User user = userService.retrieveUserByUsername(auth.getName());
if (workgroupService.isUserInWorkgroupForRun(user, run, workgroup)) {
return getPeerGroup(run, peerGroupActivityTag, workgroup);
Expand All @@ -83,12 +79,11 @@ PeerGroup getPeerGroup(@PathVariable Long runId, @PathVariable Long workgroupId,
}

@GetMapping("/{runId}/{workgroupId}/{nodeId}/{componentId}")
PeerGroup getPeerGroup(@PathVariable Long runId, @PathVariable Long workgroupId,
PeerGroup getPeerGroup(@PathVariable("runId") RunImpl run,
@PathVariable ("workgroupId") WorkgroupImpl workgroup,
@PathVariable String nodeId, @PathVariable String componentId, Authentication auth)
throws JSONException, ObjectNotFoundException, PeerGroupActivityNotFoundException,
throws JSONException, PeerGroupActivityNotFoundException,
PeerGroupCreationException, PeerGroupActivityThresholdNotSatisfiedException {
Run run = runService.retrieveById(runId);
Workgroup workgroup = workgroupService.retrieveById(workgroupId);
User user = userService.retrieveUserByUsername(auth.getName());
if (workgroupService.isUserInWorkgroupForRun(user, run, workgroup)) {
return getPeerGroup(run, nodeId, componentId, workgroup);
Expand Down
22 changes: 8 additions & 14 deletions ...va/org/wise/portal/presentation/web/controllers/peergroup/PeerGroupWorkAPIController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,19 @@
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.wise.portal.dao.ObjectNotFoundException;
import org.wise.portal.domain.peergroup.PeerGroup;
import org.wise.portal.domain.peergroup.impl.PeerGroupImpl;
import org.wise.portal.domain.peergroupactivity.PeerGroupActivity;
import org.wise.portal.domain.run.Run;
import org.wise.portal.domain.run.impl.RunImpl;
import org.wise.portal.domain.user.User;
import org.wise.portal.domain.workgroup.Workgroup;
import org.wise.portal.domain.workgroup.impl.WorkgroupImpl;
import org.wise.portal.service.peergroup.PeerGroupActivityThresholdNotSatisfiedException;
import org.wise.portal.service.peergroup.PeerGroupCreationException;
import org.wise.portal.service.peergroup.PeerGroupService;
import org.wise.portal.service.peergroupactivity.PeerGroupActivityNotFoundException;
import org.wise.portal.service.peergroupactivity.PeerGroupActivityService;
import org.wise.portal.service.run.RunService;
import org.wise.portal.service.user.UserService;
import org.wise.portal.service.workgroup.WorkgroupService;
import org.wise.vle.domain.work.StudentWork;

@RestController
Expand All @@ -44,13 +43,9 @@ public class PeerGroupWorkAPIController {
@Autowired
private UserService userService;

@Autowired
private WorkgroupService workgroupService;

@GetMapping("/{peerGroupId}/student-work")
List<StudentWork> getPeerGroupWork(@PathVariable Long peerGroupId, Authentication auth)
throws ObjectNotFoundException {
PeerGroup peerGroup = peerGroupService.getById(peerGroupId);
List<StudentWork> getPeerGroupWork(@PathVariable("peerGroupId") PeerGroupImpl peerGroup,
Authentication auth) {
if (isUserInPeerGroup(peerGroup, auth)) {
return peerGroupService.getStudentWork(peerGroup);
} else {
Expand All @@ -65,16 +60,15 @@ private boolean isUserInPeerGroup(PeerGroup peerGroup, Authentication auth) {

@Secured("ROLE_TEACHER")
@GetMapping("/{runId}/{workgroupId}/{nodeId}/{componentId}/student-work")
List<StudentWork> getPeerGroupWork(@PathVariable Long runId, @PathVariable Long workgroupId,
List<StudentWork> getPeerGroupWork(@PathVariable("runId") RunImpl run,
@PathVariable("workgroupId") WorkgroupImpl workgroup,
@PathVariable String nodeId, @PathVariable String componentId, Authentication auth)
throws JSONException, ObjectNotFoundException, PeerGroupActivityNotFoundException,
throws JSONException, PeerGroupActivityNotFoundException,
PeerGroupActivityThresholdNotSatisfiedException, PeerGroupCreationException {
Run run = runService.retrieveById(runId);
User user = userService.retrieveUserByUsername(auth.getName());
if (runService.isAllowedToViewStudentWork(run, user)) {
PeerGroupActivity activity = peerGroupActivityService.getByComponent(run, nodeId,
componentId);
Workgroup workgroup = workgroupService.retrieveById(workgroupId);
PeerGroup peerGroup = peerGroupService.getPeerGroup(workgroup, activity);
return peerGroupService.getStudentWork(peerGroup);
} else {
Expand Down
5 changes: 3 additions & 2 deletions ...in/java/org/wise/portal/presentation/web/controllers/student/PeerGroupWorkController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.wise.portal.dao.ObjectNotFoundException;
import org.wise.portal.domain.authentication.impl.StudentUserDetails;
import org.wise.portal.domain.peergroup.PeerGroup;
import org.wise.portal.domain.peergroup.impl.PeerGroupImpl;
import org.wise.portal.domain.project.impl.ProjectComponent;
import org.wise.portal.domain.run.Run;
import org.wise.portal.domain.user.User;
Expand All @@ -33,10 +34,10 @@ public class PeerGroupWorkController extends ClassmateDataController {

@GetMapping("{peerGroupId}/{showPeerGroupWorkNodeId}/{showPeerGroupWorkComponentId}/{showWorkNodeId}/{showWorkComponentId}")
public List<StudentWork> getPeerGroupWork(Authentication auth,
@PathVariable Long peerGroupId, @PathVariable String showPeerGroupWorkNodeId, @PathVariable String showPeerGroupWorkComponentId,
@PathVariable("peerGroupId") PeerGroupImpl peerGroup,
@PathVariable String showPeerGroupWorkNodeId, @PathVariable String showPeerGroupWorkComponentId,
@PathVariable String showWorkNodeId, @PathVariable String showWorkComponentId)
throws ObjectNotFoundException, JSONException, IOException {
PeerGroup peerGroup = peerGroupService.getById(peerGroupId);
if (isUserInPeerGroup(auth, peerGroup)) {
Run run = peerGroup.getPeerGroupActivity().getRun();
if (isValidShowPeerGroupWorkComponent(run, showPeerGroupWorkNodeId, showPeerGroupWorkComponentId, showWorkNodeId, showWorkComponentId)) {
Expand Down
12 changes: 5 additions & 7 deletions ...va/org/wise/portal/presentation/web/controllers/peergroup/PeerGroupAPIControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public void getPeerGroupByComponent_WorkgroupNotAssociatedWithRun_AccessDenied()
expectWorkgroupAssociatedWithRun(false);
replayAll();
try {
controller.getPeerGroup(runId1, workgroup1Id, run1Node1Id, run1Component1Id, studentAuth);
controller.getPeerGroup(run1, workgroup1, run1Node1Id, run1Component1Id, studentAuth);
fail("Expected AccessDeniedException, but was not thrown");
} catch (AccessDeniedException e) {
}
Expand All @@ -36,7 +36,7 @@ public void getPeerGroupByComponent_PeerGroupActivityNotFound_ThrowException() t
expectPeerGroupActivityNotFound();
replayAll();
try {
controller.getPeerGroup(runId1, workgroup1Id, run1Node1Id, run1Component1Id, studentAuth);
controller.getPeerGroup(run1, workgroup1, run1Node1Id, run1Component1Id, studentAuth);
fail("Expected PeerGroupActivityNotFoundException, but was not thrown");
} catch (PeerGroupActivityNotFoundException e) {
}
Expand All @@ -49,7 +49,7 @@ public void getPeerGroupByComponent_PeerGroupThresholdNotMet_ThrowException() th
expectPeerGroupThresholdNotSatisifed();
replayAll();
try {
controller.getPeerGroup(runId1, workgroup1Id, run1Node1Id, run1Component1Id, studentAuth);
controller.getPeerGroup(run1, workgroup1, run1Node1Id, run1Component1Id, studentAuth);
fail("Expected PeerGroupCreationException, but was not thrown");
} catch (PeerGroupActivityThresholdNotSatisfiedException e) {
}
Expand All @@ -62,7 +62,7 @@ public void getPeerGroupByComponent_ErrorCreatingPeerGroup_ThrowException() thro
expectPeerGroupCreationException();
replayAll();
try {
controller.getPeerGroup(runId1, workgroup1Id, run1Node1Id, run1Component1Id, studentAuth);
controller.getPeerGroup(run1, workgroup1, run1Node1Id, run1Component1Id, studentAuth);
fail("Expected PeerGroupCreationException, but was not thrown");
} catch (PeerGroupCreationException e) {
}
Expand All @@ -74,7 +74,7 @@ public void getPeerGroupByComponent_FoundExistingGroupOrGroupCreated_ReturnGroup
expectWorkgroupAssociatedWithRunAndActivityFound();
expectPeerGroupCreated();
replayAll();
assertNotNull(controller.getPeerGroup(runId1, workgroup1Id, run1Node1Id, run1Component1Id,
assertNotNull(controller.getPeerGroup(run1, workgroup1, run1Node1Id, run1Component1Id,
studentAuth));
verifyAll();
}
Expand All @@ -86,8 +86,6 @@ private void expectWorkgroupAssociatedWithRunAndActivityFound() throws Exception
}

private void expectWorkgroupAssociatedWithRun(boolean isAssociated) throws Exception {
expect(runService.retrieveById(runId1)).andReturn(run1);
expect(workgroupService.retrieveById(workgroup1Id)).andReturn(workgroup1);
expect(userService.retrieveUserByUsername(studentAuth.getName())).andReturn(student1);
expect(workgroupService.isUserInWorkgroupForRun(student1, run1, workgroup1))
.andReturn(isAssociated);
Expand Down
89 changes: 6 additions & 83 deletions ...rg/wise/portal/presentation/web/controllers/peergroup/PeerGroupWorkAPIControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
import org.junit.runner.RunWith;
import org.springframework.security.access.AccessDeniedException;
import org.wise.portal.dao.ObjectNotFoundException;
import org.wise.portal.domain.peergroup.PeerGroup;
import org.wise.portal.domain.run.impl.RunImpl;
import org.wise.portal.domain.workgroup.impl.WorkgroupImpl;
import org.wise.portal.service.peergroup.PeerGroupCreationException;
import org.wise.portal.service.peergroupactivity.PeerGroupActivityNotFoundException;
import org.wise.vle.domain.work.StudentWork;
Expand All @@ -28,27 +25,13 @@ public class PeerGroupWorkAPIControllerTest extends AbstractPeerGroupAPIControll

private List<StudentWork> peerGroup1StudentWork = new ArrayList<StudentWork>();

@Test
public void getPeerGroupWork_NonExistingPeerGroupIdSpecifiedByPeerGroupId_ThrowObjectNotFound()
throws ObjectNotFoundException {
expectPeerGroupIdNotExist();
replayAll();
try {
controller.getPeerGroupWork(peerGroup1Id, studentAuth);
fail("Expected ObjectNotFoundException, but was not thrown");
} catch (ObjectNotFoundException e) {
}
verifyAll();
}

@Test
public void getPeerGroupWork_UserNotInPeerGroupSpecifiedByPeerGroupId_ThrowAccessDenied()
throws ObjectNotFoundException {
expectPeerGroup();
expectUserNotInPeerGroup();
replayAll();
try {
controller.getPeerGroupWork(peerGroup1Id, studentAuth);
controller.getPeerGroupWork(peerGroup1, studentAuth);
fail("Expected AccessDeniedException, but was not thrown");
} catch (AccessDeniedException e) {
}
Expand All @@ -58,36 +41,21 @@ public void getPeerGroupWork_UserNotInPeerGroupSpecifiedByPeerGroupId_ThrowAcces
@Test
public void getPeerGroupWork_UserInPeerGroupSpecifiedByPeerGroupId_ReturnStudentWork()
throws ObjectNotFoundException {
expectPeerGroup();
expectUserInPeerGroup();
expectGetStudentWork();
replayAll();
assertNotNull(controller.getPeerGroupWork(peerGroup1Id, studentAuth));
verifyAll();
}

@Test
public void getPeerGroupWork_NonExistingRunIdSpecifiedByComponent_ThrowObjectNotFound()
throws Exception {
expectRunNotExists();
replayAll();
try {
controller.getPeerGroupWork(runId1, workgroup1Id, run1Node1Id, run1Component1Id, teacherAuth);
fail("Expected ObjectNotFoundException, but was not thrown");
} catch (ObjectNotFoundException e) {
}
assertNotNull(controller.getPeerGroupWork(peerGroup1, studentAuth));
verifyAll();
}

@Test
public void getPeerGroupWork_TeacherNotAllowedToViewRunTeacherSpecifiedByComponent_ThrowAccessDenied()
throws Exception {
expectRunExists();
expectRetrieveTeacherUser();
expectUserNotAllowedToViewStudentWorkForRun();
replayAll();
try {
controller.getPeerGroupWork(runId1, workgroup1Id, run1Node1Id, run1Component1Id, teacherAuth);
controller.getPeerGroupWork(run1, workgroup1, run1Node1Id, run1Component1Id, teacherAuth);
fail("Expected AccessDeniedException, but was not thrown");
} catch (AccessDeniedException e) {
}
Expand All @@ -101,38 +69,22 @@ public void getPeerGroupWork_PeerGroupActivityNotFoundSpecifiedByComponent_Throw
expectPeerGroupActivityNotFound();
replayAll();
try {
controller.getPeerGroupWork(runId1, workgroup1Id, run1Node1Id, run1Component1Id,
controller.getPeerGroupWork(run1, workgroup1, run1Node1Id, run1Component1Id,
teacherAuth);
fail("PeerGroupActivityNotFoundException expected, but was not thrown");
} catch (PeerGroupActivityNotFoundException e) {}
verifyAll();
}

@Test
public void getPeerGroupWork_WorkgroupNotFoundSpecifiedByComponent_ThrowObjectNotFound()
throws Exception {
expectUserAllowedToViewStudentWorkForRun();
expectPeerGroupActivityFound();
expectWorkgroupNotFound();
replayAll();
try {
controller.getPeerGroupWork(runId1, workgroup1Id, run1Node1Id, run1Component1Id,
teacherAuth);
fail("ObjectNotFoundException expected, but was not thrown");
} catch (ObjectNotFoundException e) {}
verifyAll();
}

@Test
public void getPeerGroupWork_PeerGroupNotFoundSpecifiedByComponent_ThrowException()
throws Exception {
expectUserAllowedToViewStudentWorkForRun();
expectPeerGroupActivityFound();
expectWorkgroupFound();
expectPeerGroupCreationException();
replayAll();
try {
controller.getPeerGroupWork(runId1, workgroup1Id, run1Node1Id, run1Component1Id,
controller.getPeerGroupWork(run1, workgroup1, run1Node1Id, run1Component1Id,
teacherAuth);
fail("PeerGroupCreationException expected, but was not thrown");
} catch (PeerGroupCreationException e) {}
Expand All @@ -144,11 +96,10 @@ public void getPeerGroupWork_PeerGroupFoundSpecifiedByComponent_ReturnStudentWor
throws Exception {
expectUserAllowedToViewStudentWorkForRun();
expectPeerGroupActivityFound();
expectWorkgroupFound();
expectPeerGroupFound();
expectGetStudentWork();
replayAll();
assertNotNull(controller.getPeerGroupWork(runId1, workgroup1Id, run1Node1Id, run1Component1Id,
assertNotNull(controller.getPeerGroupWork(run1, workgroup1, run1Node1Id, run1Component1Id,
teacherAuth));
verifyAll();
}
Expand All @@ -158,30 +109,11 @@ private void expectRetrieveTeacherUser() {
teacher1);
}

private void expectRunNotExists() throws ObjectNotFoundException {
expect(runService.retrieveById(runId1)).andThrow(new ObjectNotFoundException(runId1,
RunImpl.class));
}

private void expectRunExists() throws ObjectNotFoundException {
expect(runService.retrieveById(runId1)).andReturn(run1);
}

private void expectPeerGroupFound() throws Exception {
expect(peerGroupService.getPeerGroup(workgroup1, peerGroupActivity)).andReturn(peerGroup1);
}

private void expectWorkgroupFound() throws ObjectNotFoundException {
expect(workgroupService.retrieveById(workgroup1Id)).andReturn(workgroup1);
}

private void expectWorkgroupNotFound() throws ObjectNotFoundException {
expect(workgroupService.retrieveById(workgroup1Id)).andThrow(
new ObjectNotFoundException(workgroup1Id, WorkgroupImpl.class));
}

private void expectUserAllowedToViewStudentWorkForRun() throws ObjectNotFoundException {
expectRunExists();
expectRetrieveTeacherUser();
expect(runService.isAllowedToViewStudentWork(run1, teacher1)).andReturn(true);
}
Expand All @@ -201,13 +133,4 @@ private void expectUserNotInPeerGroup() {
private void expectUserInPeerGroup() {
expect(userService.retrieveUserByUsername(studentAuth.getName())).andReturn(student1);
}

private void expectPeerGroup() throws ObjectNotFoundException {
expect(peerGroupService.getById(peerGroup1Id)).andReturn(peerGroup1);
}

private void expectPeerGroupIdNotExist() throws ObjectNotFoundException {
expect(peerGroupService.getById(peerGroup1Id)).andThrow(new ObjectNotFoundException(
peerGroup1Id, PeerGroup.class));
}
}

0 comments on commit 77dfa39

Please sign in to comment.