From eebb860aa6aaba0825529b7d9047c88e43ef6aca Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Tue, 16 Nov 2021 11:48:52 -0800 Subject: [PATCH 1/4] feat(Peer Group): Add teacher endpoint to get PeerGroups for activity Requires database change: add periodId bigint field to peer_groups table Closes #57 --- .../portal/dao/peergroup/PeerGroupDao.java | 2 + .../peergroup/impl/HibernatePeerGroupDao.java | 6 ++ .../domain/peergroup/impl/PeerGroupImpl.java | 19 ++++- .../TeacherPeerGroupAPIController.java | 52 ++++++++++++ .../service/peergroup/PeerGroupService.java | 7 ++ .../peergroup/impl/PeerGroupServiceImpl.java | 8 +- src/main/resources/wise_db_init.sql | 1 + .../impl/HibernatePeerGroupDaoTest.java | 6 ++ .../web/controllers/APIControllerTest.java | 4 + .../AbstractPeerGroupAPIControllerTest.java | 17 ++-- .../TeacherPeerGroupAPIControllerTest.java | 84 +++++++++++++++++++ .../impl/PeerGroupServiceImplTest.java | 17 +++- .../impl/PeerGroupServiceTestHelper.java | 7 +- 13 files changed, 221 insertions(+), 9 deletions(-) create mode 100644 src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIController.java create mode 100644 src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIControllerTest.java diff --git a/src/main/java/org/wise/portal/dao/peergroup/PeerGroupDao.java b/src/main/java/org/wise/portal/dao/peergroup/PeerGroupDao.java index 85f05f8e3..a19d4c93b 100644 --- a/src/main/java/org/wise/portal/dao/peergroup/PeerGroupDao.java +++ b/src/main/java/org/wise/portal/dao/peergroup/PeerGroupDao.java @@ -38,6 +38,8 @@ public interface PeerGroupDao extends SimpleDao { PeerGroup getByWorkgroupAndActivity(Workgroup workgroup, PeerGroupActivity activity); + List getListByActivity(PeerGroupActivity activity); + List getListByRun(Run run); List getListByComponent(Run run, String nodeId, String componentId); diff --git a/src/main/java/org/wise/portal/dao/peergroup/impl/HibernatePeerGroupDao.java b/src/main/java/org/wise/portal/dao/peergroup/impl/HibernatePeerGroupDao.java index 0568e718b..27b6522bc 100644 --- a/src/main/java/org/wise/portal/dao/peergroup/impl/HibernatePeerGroupDao.java +++ b/src/main/java/org/wise/portal/dao/peergroup/impl/HibernatePeerGroupDao.java @@ -84,6 +84,12 @@ public PeerGroup getByWorkgroupAndActivity(Workgroup workgroup, PeerGroupActivit return (PeerGroup) query.getResultStream().findFirst().orElse(null); } + + @Override + public List getListByActivity(PeerGroupActivity activity) { + return getListByComponent(activity.getRun(), activity.getNodeId(), activity.getComponentId()); + } + @Override public List getListByRun(Run run) { return getListByComponent(run, null, null); diff --git a/src/main/java/org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java b/src/main/java/org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java index 3ba2ab02e..28975c6cb 100644 --- a/src/main/java/org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java +++ b/src/main/java/org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java @@ -36,8 +36,14 @@ import javax.persistence.JoinTable; import javax.persistence.ManyToMany; import javax.persistence.ManyToOne; +import javax.persistence.OneToOne; import javax.persistence.Table; +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; + +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.group.impl.PersistentGroup; import org.wise.portal.domain.peergroup.PeerGroup; import org.wise.portal.domain.peergroupactivity.PeerGroupActivity; import org.wise.portal.domain.peergroupactivity.impl.PeerGroupActivityImpl; @@ -73,10 +79,16 @@ public class PeerGroupImpl implements PeerGroup { inverseJoinColumns = @JoinColumn(name = "workgroup_fk", nullable = false)) private Set members = new HashSet(); + @OneToOne(targetEntity = PersistentGroup.class, fetch = FetchType.LAZY) + @JoinColumn(name = "periodId") + @JsonIgnore + private Group period; + public PeerGroupImpl() {} - public PeerGroupImpl(PeerGroupActivity activity, Set members) { + public PeerGroupImpl(PeerGroupActivity activity, Group period, Set members) { this.peerGroupActivity = activity; + this.period = period; this.members = members; } @@ -93,4 +105,9 @@ public boolean isMember(User user) { } return false; } + + @JsonProperty + public Long getPeriodId() { + return period.getId(); + } } diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIController.java b/src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIController.java new file mode 100644 index 000000000..ab80d4b8f --- /dev/null +++ b/src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIController.java @@ -0,0 +1,52 @@ +package org.wise.portal.presentation.web.controllers.peergroup; + +import java.util.List; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.access.annotation.Secured; +import org.springframework.security.core.Authentication; +import org.springframework.web.bind.annotation.GetMapping; +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.service.peergroup.PeerGroupService; +import org.wise.portal.service.peergroupactivity.PeerGroupActivityNotFoundException; +import org.wise.portal.service.peergroupactivity.PeerGroupActivityService; +import org.wise.portal.service.run.RunService; + +/** + * @author Hiroki Terashima + */ +@RestController +@Secured("ROLE_TEACHER") +@RequestMapping("/api/teacher/peer-group") +public class TeacherPeerGroupAPIController { + + @Autowired + private PeerGroupActivityService peerGroupActivityService; + + @Autowired + private PeerGroupService peerGroupService; + + @Autowired + private RunService runService; + + @GetMapping("/{runId}/{nodeId}/{componentId}") + List getPeerGroups(@PathVariable Long runId, @PathVariable String nodeId, + @PathVariable String componentId, Authentication auth) + throws ObjectNotFoundException, PeerGroupActivityNotFoundException { + Run run = runService.retrieveById(runId); + if (runService.hasReadPermission(auth, run)) { + PeerGroupActivity activity = peerGroupActivityService.getByComponent(run, nodeId, + componentId); + return peerGroupService.getPeerGroups(activity); + } else { + throw new AccessDeniedException("Not permitted"); + } + } +} diff --git a/src/main/java/org/wise/portal/service/peergroup/PeerGroupService.java b/src/main/java/org/wise/portal/service/peergroup/PeerGroupService.java index 7a8b59a9f..bd65c59ae 100644 --- a/src/main/java/org/wise/portal/service/peergroup/PeerGroupService.java +++ b/src/main/java/org/wise/portal/service/peergroup/PeerGroupService.java @@ -59,6 +59,13 @@ public interface PeerGroupService { PeerGroup getPeerGroup(Workgroup workgroup, PeerGroupActivity activity) throws PeerGroupActivityThresholdNotSatisfiedException, PeerGroupCreationException; + /** + * Gets all the PeerGroups for the specified PeerGroupActivity + * @param activity PeerGroupActivity the PeerGroups works on + * @return PeerGroups that work on the specified activity + */ + List getPeerGroups(PeerGroupActivity activity); + /** * Gets StudentWork for the PeerGroup's activity from all the members in the PeerGroup * @param peerGroup group of workgroups in the PeerGroup diff --git a/src/main/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceImpl.java b/src/main/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceImpl.java index 67de7b892..b48697d97 100644 --- a/src/main/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceImpl.java +++ b/src/main/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceImpl.java @@ -84,7 +84,8 @@ private PeerGroup createPeerGroup(Workgroup workgroup, PeerGroupActivity activit } else if (!peerGroupThresholdService.canCreatePeerGroup(activity, workgroup.getPeriod())) { throw new PeerGroupCreationException(); } else { - PeerGroup peerGroup = new PeerGroupImpl(activity, getPeerGroupMembers(workgroup, activity)); + PeerGroup peerGroup = new PeerGroupImpl(activity, workgroup.getPeriod(), + getPeerGroupMembers(workgroup, activity)); this.peerGroupDao.save(peerGroup); return peerGroup; } @@ -181,6 +182,11 @@ private List getLogicComponentStudentWorkForPeriod(PeerGroupActivit return studentWorkUniqueWorkgroups; } + @Override + public List getPeerGroups(PeerGroupActivity activity) { + return peerGroupDao.getListByActivity(activity); + } + @Override public List getStudentWork(PeerGroup peerGroup) { return studentWorkDao.getWorkForComponentByWorkgroups(peerGroup.getMembers(), diff --git a/src/main/resources/wise_db_init.sql b/src/main/resources/wise_db_init.sql index 274494ed1..de647696c 100644 --- a/src/main/resources/wise_db_init.sql +++ b/src/main/resources/wise_db_init.sql @@ -227,6 +227,7 @@ create table peer_group_activities ( create table peer_groups ( id bigint not null auto_increment, peerGroupActivityId bigint not null, + periodId bigint, OPTLOCK integer, constraint peerGroupActivityIdFK foreign key (peerGroupActivityId) references peer_group_activities (id), primary key (id) diff --git a/src/test/java/org/wise/portal/dao/peergroup/impl/HibernatePeerGroupDaoTest.java b/src/test/java/org/wise/portal/dao/peergroup/impl/HibernatePeerGroupDaoTest.java index 548a5b46a..25c713e9a 100644 --- a/src/test/java/org/wise/portal/dao/peergroup/impl/HibernatePeerGroupDaoTest.java +++ b/src/test/java/org/wise/portal/dao/peergroup/impl/HibernatePeerGroupDaoTest.java @@ -96,6 +96,12 @@ public void getByWorkgroupAndActivity_WorkgroupNotInPeerGroupForActivity_ReturnN assertNull(peerGroupDao.getByWorkgroupAndActivity(workgroup2, activity1)); } + @Test + public void getListByActivity_ReturnListByActivity() { + assertEquals(1, peerGroupDao.getListByActivity(activity1).size()); + assertEquals(1, peerGroupDao.getListByActivity(activity2).size()); + } + @Test public void getListByRun_ReturnListByRun() { assertEquals(2, peerGroupDao.getListByRun(run1).size()); diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/APIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/APIControllerTest.java index 78c0e4a53..405da2567 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/APIControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/APIControllerTest.java @@ -111,6 +111,10 @@ public class APIControllerTest { protected Group run1Period1, run1Period2; + protected String run1Node1Id = "run1Node1"; + + protected String run1Component1Id = "run1Component1"; + @Mock protected HttpServletRequest request; diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/AbstractPeerGroupAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/AbstractPeerGroupAPIControllerTest.java index 8d67a514e..e9f4967f7 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/AbstractPeerGroupAPIControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/AbstractPeerGroupAPIControllerTest.java @@ -2,6 +2,9 @@ import static org.easymock.EasyMock.*; +import java.util.ArrayList; +import java.util.List; + import org.easymock.Mock; import org.junit.Before; import org.wise.portal.domain.peergroup.PeerGroup; @@ -22,22 +25,26 @@ public abstract class AbstractPeerGroupAPIControllerTest extends APIControllerTe @Mock protected PeerGroupActivityService peerGroupActivityService; - protected String run1Node1Id = "run1Node1"; - - protected String run1Component1Id = "run1Component1"; - protected PeerGroupActivity peerGroupActivity; - protected PeerGroup peerGroup1; + protected PeerGroup peerGroup1, peerGroup2; protected Long peerGroup1Id = 1L; + protected Long peerGroup2Id = 2L; + + protected List peerGroups = new ArrayList(); + @Before public void setUp() { super.setUp(); peerGroupActivity = new PeerGroupActivityImpl(); peerGroup1 = new PeerGroupImpl(); peerGroup1.addMember(workgroup1); + peerGroups.add(peerGroup1); + peerGroup2 = new PeerGroupImpl(); + peerGroup2.addMember(workgroup2); + peerGroups.add(peerGroup2); } protected void expectPeerGroupActivityFound() throws PeerGroupActivityNotFoundException { diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIControllerTest.java new file mode 100644 index 000000000..fcf6d9073 --- /dev/null +++ b/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIControllerTest.java @@ -0,0 +1,84 @@ +package org.wise.portal.presentation.web.controllers.peergroup; + +import static org.easymock.EasyMock.*; +import static org.junit.Assert.*; + +import java.util.List; + +import org.easymock.EasyMockRunner; +import org.easymock.TestSubject; +import org.junit.Before; +import org.junit.Test; +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.service.peergroupactivity.PeerGroupActivityNotFoundException; + +/** + * @author Hiroki Terashima + */ +@RunWith(EasyMockRunner.class) +public class TeacherPeerGroupAPIControllerTest extends AbstractPeerGroupAPIControllerTest { + + @TestSubject + private TeacherPeerGroupAPIController controller = new TeacherPeerGroupAPIController(); + + @Before + public void setUp() { + super.setUp(); + expectRunExists(); + } + + @Test + public void getPeerGroups_NoPermissions_AccessDenied() throws Exception { + expectTeacherHasAccessToRun(false); + replayAll(); + try { + controller.getPeerGroups(runId1, run1Node1Id, run1Component1Id, teacherAuth); + fail("Expected AccessDeniedException, but was not thrown"); + } catch (AccessDeniedException e) { + } + verifyAll(); + } + + @Test + public void getPeerGroups_PeerGroupActivityNotFound_ThrowException() throws Exception { + expectTeacherHasAccessToRun(true); + expectPeerGroupActivityNotFound(); + replayAll(); + try { + controller.getPeerGroups(runId1, run1Node1Id, run1Component1Id, teacherAuth); + fail("Expected PeerGroupActivityNotFoundException, but was not thrown"); + } catch (PeerGroupActivityNotFoundException e) { + } + verifyAll(); + } + + @Test + public void getPeerGroups_ActivityFound_ReturnList() throws Exception { + expectTeacherHasAccessToRun(true); + expectPeerGroupActivityFound(); + expectPeerGroups(); + replayAll(); + List peerGroups = + controller.getPeerGroups(runId1, run1Node1Id, run1Component1Id, teacherAuth); + assertEquals(2, peerGroups.size()); + verifyAll(); + } + + private void expectPeerGroups() { + expect(peerGroupService.getPeerGroups(peerGroupActivity)).andReturn(peerGroups); + } + + private void expectRunExists() { + try { + expect(runService.retrieveById(runId1)).andReturn(run1); + } catch (ObjectNotFoundException e) { + } + } + + private void expectTeacherHasAccessToRun(boolean hasAccess) { + expect(runService.hasReadPermission(teacherAuth, run1)).andReturn(hasAccess); + } +} diff --git a/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceImplTest.java b/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceImplTest.java index 1bb6732be..979fe38f5 100644 --- a/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceImplTest.java +++ b/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceImplTest.java @@ -82,12 +82,15 @@ public class PeerGroupServiceImplTest extends WISEServiceTest { PeerGroup peerGroup; + List peerGroups; + @Before public void setUp() throws Exception { super.setUp(); PeerGroupServiceTestHelper testHelper = new PeerGroupServiceTestHelper(run1, run1Component2); activity = testHelper.activity; peerGroup = testHelper.peerGroup1; + peerGroups = testHelper.peerGroups; } @Test @@ -177,10 +180,22 @@ public void getPeerGroup_AllThresholdsSatisfiedMoreThan3WorkgroupsLeft_Create2Wo public void getStudentWork_PeerGroupExist_ReturnStudentWorkList() { expectGetWorkForComponentByWorkgroups(); replayAll(); - assertEquals(service.getStudentWork(peerGroup).size(), 3); + assertEquals(3, service.getStudentWork(peerGroup).size()); + verifyAll(); + } + + @Test + public void getPeerGroups_ReturnPeerGroupList() { + expectGetPeerGroupsByActivity(); + replayAll(); + assertEquals(1, service.getPeerGroups(activity).size()); verifyAll(); } + private void expectGetPeerGroupsByActivity() { + expect(peerGroupDao.getListByActivity(activity)).andReturn(peerGroups); + } + private void expectAllThresholdsSatisfied() throws JSONException { expectPeerGroupFromDB(null); expectWorkForComponentByWorkgroup(Arrays.asList(componentWorkSubmit1)); diff --git a/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceTestHelper.java b/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceTestHelper.java index 0b64e1fd1..e3e52e14c 100644 --- a/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceTestHelper.java +++ b/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupServiceTestHelper.java @@ -23,7 +23,9 @@ */ package org.wise.portal.service.peergroup.impl; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; import org.json.JSONObject; @@ -62,6 +64,8 @@ public class PeerGroupServiceTestHelper extends WISEServiceTest { PeerGroup peerGroup1, peerGroup2, peerGroup3; + List peerGroups = new ArrayList(); + public PeerGroupServiceTestHelper(Run run, Component component) throws Exception { super.setUp(); activity = new PeerGroupActivityImpl(run, component.nodeId, @@ -69,6 +73,7 @@ public PeerGroupServiceTestHelper(Run run, Component component) throws Exception Set members = new HashSet(); members.add(run1Workgroup1); members.add(run1Workgroup2); - peerGroup1 = new PeerGroupImpl(activity, members); + peerGroup1 = new PeerGroupImpl(activity, run1Period1, members); + peerGroups.add(peerGroup1); } } From 37c30948552ffd705bc408170f492473f28aa5a4 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Thu, 18 Nov 2021 10:13:18 -0800 Subject: [PATCH 2/4] feat(PeerGroup): Add teacher endpoint to get PeerGroup info - url: /api/teacher/peer-group-info returns a Map of PeerGroups and workgroups that don't belong in PeerGroups for the specified PeerGroupActivity --- ...=> TeacherPeerGroupInfoAPIController.java} | 21 ++-- .../peergroup/PeerGroupInfoService.java | 21 ++++ .../impl/PeerGroupInfoServiceImpl.java | 69 +++++++++++++ .../AbstractPeerGroupAPIControllerTest.java | 13 ++- ...eacherPeerGroupInfoAPIControllerTest.java} | 34 ++++--- .../wise/portal/service/WISEServiceTest.java | 3 + .../impl/PeerGroupInfoServiceImplTest.java | 98 +++++++++++++++++++ 7 files changed, 232 insertions(+), 27 deletions(-) rename src/main/java/org/wise/portal/presentation/web/controllers/peergroup/{TeacherPeerGroupAPIController.java => TeacherPeerGroupInfoAPIController.java} (67%) create mode 100644 src/main/java/org/wise/portal/service/peergroup/PeerGroupInfoService.java create mode 100644 src/main/java/org/wise/portal/service/peergroup/impl/PeerGroupInfoServiceImpl.java rename src/test/java/org/wise/portal/presentation/web/controllers/peergroup/{TeacherPeerGroupAPIControllerTest.java => TeacherPeerGroupInfoAPIControllerTest.java} (53%) create mode 100644 src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupInfoServiceImplTest.java diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIController.java b/src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupInfoAPIController.java similarity index 67% rename from src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIController.java rename to src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupInfoAPIController.java index ab80d4b8f..a4aa2ed7c 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupInfoAPIController.java @@ -1,6 +1,6 @@ package org.wise.portal.presentation.web.controllers.peergroup; -import java.util.List; +import java.util.Map; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.access.AccessDeniedException; @@ -11,10 +11,8 @@ 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.service.peergroup.PeerGroupService; +import org.wise.portal.service.peergroup.PeerGroupInfoService; import org.wise.portal.service.peergroupactivity.PeerGroupActivityNotFoundException; import org.wise.portal.service.peergroupactivity.PeerGroupActivityService; import org.wise.portal.service.run.RunService; @@ -24,27 +22,26 @@ */ @RestController @Secured("ROLE_TEACHER") -@RequestMapping("/api/teacher/peer-group") -public class TeacherPeerGroupAPIController { +@RequestMapping("/api/teacher/peer-group-info") +public class TeacherPeerGroupInfoAPIController { @Autowired private PeerGroupActivityService peerGroupActivityService; @Autowired - private PeerGroupService peerGroupService; + private PeerGroupInfoService peerGroupInfoService; @Autowired private RunService runService; @GetMapping("/{runId}/{nodeId}/{componentId}") - List getPeerGroups(@PathVariable Long runId, @PathVariable String nodeId, - @PathVariable String componentId, Authentication auth) + public Map getPeerGroupsInfo(@PathVariable Long runId, + @PathVariable String nodeId, @PathVariable String componentId, Authentication auth) throws ObjectNotFoundException, PeerGroupActivityNotFoundException { Run run = runService.retrieveById(runId); if (runService.hasReadPermission(auth, run)) { - PeerGroupActivity activity = peerGroupActivityService.getByComponent(run, nodeId, - componentId); - return peerGroupService.getPeerGroups(activity); + return peerGroupInfoService.getPeerGroupInfo(peerGroupActivityService.getByComponent(run, + nodeId, componentId)); } else { throw new AccessDeniedException("Not permitted"); } diff --git a/src/main/java/org/wise/portal/service/peergroup/PeerGroupInfoService.java b/src/main/java/org/wise/portal/service/peergroup/PeerGroupInfoService.java new file mode 100644 index 000000000..ff7c98135 --- /dev/null +++ b/src/main/java/org/wise/portal/service/peergroup/PeerGroupInfoService.java @@ -0,0 +1,21 @@ +package org.wise.portal.service.peergroup; + +import java.util.Map; + +import org.wise.portal.domain.peergroupactivity.PeerGroupActivity; + +/** + * @author Hiroki Terashima + */ +public interface PeerGroupInfoService { + + /** + * Returns a map with 2 elements: + * 1. "peerGroups": PeerGroups in the specified activity + * 2. "workgroupsNotInPeerGroups": Workgroups that have not been paired into a + * PeerGroup for the specified activity + * @param activity PeerGroupActivity to get the info for + * @return a Map containing information about PeerGroupings for the specified activity + */ + Map getPeerGroupInfo(PeerGroupActivity activity); +} diff --git a/src/main/java/org/wise/portal/service/peergroup/impl/PeerGroupInfoServiceImpl.java b/src/main/java/org/wise/portal/service/peergroup/impl/PeerGroupInfoServiceImpl.java new file mode 100644 index 000000000..84875f546 --- /dev/null +++ b/src/main/java/org/wise/portal/service/peergroup/impl/PeerGroupInfoServiceImpl.java @@ -0,0 +1,69 @@ +package org.wise.portal.service.peergroup.impl; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; +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.workgroup.Workgroup; +import org.wise.portal.service.peergroup.PeerGroupInfoService; +import org.wise.portal.service.peergroup.PeerGroupService; +import org.wise.portal.service.run.RunService; + +/** + * @author Hiroki Terashima + */ +@Service +public class PeerGroupInfoServiceImpl implements PeerGroupInfoService { + + @Autowired + private PeerGroupService peerGroupService; + + @Autowired + private RunService runService; + + @Override + public Map getPeerGroupInfo(PeerGroupActivity activity) { + Map peerGroupInfo = new HashMap(); + List peerGroups = peerGroupService.getPeerGroups(activity); + peerGroupInfo.put("peerGroups", peerGroups); + peerGroupInfo.put("workgroupsNotInPeerGroup", getWorkgroupsNotInPeerGroup(activity.getRun(), + peerGroups)); + return peerGroupInfo; + } + + private List getWorkgroupsNotInPeerGroup(Run run, List peerGroups) { + Set workgroupsInPeerGroup = getWorkgroupsInPeerGroup(peerGroups); + List workgroupsNotInPeerGroups = new ArrayList(); + try { + for (Workgroup workgroupInRun : runService.getWorkgroups(run.getId())) { + if (isActiveStudentWorkgroup(workgroupInRun) && + !workgroupsInPeerGroup.contains(workgroupInRun)) { + workgroupsNotInPeerGroups.add(workgroupInRun); + } + } + } catch (ObjectNotFoundException e) { + } + return workgroupsNotInPeerGroups; + } + + private Set getWorkgroupsInPeerGroup(List peerGroups) { + Set workgroupsInPeerGroup = new HashSet(); + for (PeerGroup peerGroup : peerGroups) { + workgroupsInPeerGroup.addAll(peerGroup.getMembers()); + } + return workgroupsInPeerGroup; + } + + private boolean isActiveStudentWorkgroup(Workgroup workgroup) { + return workgroup.getPeriod() != null && workgroup.isStudentWorkgroup(); + } +} diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/AbstractPeerGroupAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/AbstractPeerGroupAPIControllerTest.java index e9f4967f7..ba3f33d81 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/AbstractPeerGroupAPIControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/AbstractPeerGroupAPIControllerTest.java @@ -11,8 +11,10 @@ import org.wise.portal.domain.peergroup.impl.PeerGroupImpl; import org.wise.portal.domain.peergroupactivity.PeerGroupActivity; import org.wise.portal.domain.peergroupactivity.impl.PeerGroupActivityImpl; +import org.wise.portal.domain.workgroup.Workgroup; import org.wise.portal.presentation.web.controllers.APIControllerTest; import org.wise.portal.service.peergroup.PeerGroupCreationException; +import org.wise.portal.service.peergroup.PeerGroupInfoService; import org.wise.portal.service.peergroup.PeerGroupService; import org.wise.portal.service.peergroupactivity.PeerGroupActivityNotFoundException; import org.wise.portal.service.peergroupactivity.PeerGroupActivityService; @@ -25,6 +27,9 @@ public abstract class AbstractPeerGroupAPIControllerTest extends APIControllerTe @Mock protected PeerGroupActivityService peerGroupActivityService; + @Mock + protected PeerGroupInfoService peerGroupInfoService; + protected PeerGroupActivity peerGroupActivity; protected PeerGroup peerGroup1, peerGroup2; @@ -35,6 +40,8 @@ public abstract class AbstractPeerGroupAPIControllerTest extends APIControllerTe protected List peerGroups = new ArrayList(); + protected List workgroupsNotInPeerGroups = new ArrayList(); + @Before public void setUp() { super.setUp(); @@ -63,10 +70,12 @@ protected void expectPeerGroupCreationException() throws Exception { } protected void verifyAll() { - verify(peerGroupActivityService, peerGroupService, runService, userService, workgroupService); + verify(peerGroupActivityService, peerGroupInfoService, peerGroupService, runService, + userService, workgroupService); } protected void replayAll() { - replay(peerGroupActivityService, peerGroupService, runService, userService, workgroupService); + replay(peerGroupActivityService, peerGroupInfoService, peerGroupService, runService, + userService, workgroupService); } } diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupInfoAPIControllerTest.java similarity index 53% rename from src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIControllerTest.java rename to src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupInfoAPIControllerTest.java index fcf6d9073..9055b5ba4 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupAPIControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/peergroup/TeacherPeerGroupInfoAPIControllerTest.java @@ -3,7 +3,9 @@ import static org.easymock.EasyMock.*; import static org.junit.Assert.*; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.easymock.EasyMockRunner; import org.easymock.TestSubject; @@ -19,10 +21,10 @@ * @author Hiroki Terashima */ @RunWith(EasyMockRunner.class) -public class TeacherPeerGroupAPIControllerTest extends AbstractPeerGroupAPIControllerTest { +public class TeacherPeerGroupInfoAPIControllerTest extends AbstractPeerGroupAPIControllerTest { @TestSubject - private TeacherPeerGroupAPIController controller = new TeacherPeerGroupAPIController(); + private TeacherPeerGroupInfoAPIController controller = new TeacherPeerGroupInfoAPIController(); @Before public void setUp() { @@ -31,11 +33,11 @@ public void setUp() { } @Test - public void getPeerGroups_NoPermissions_AccessDenied() throws Exception { + public void getPeerGroupsInfo_NoPermissions_AccessDenied() throws Exception { expectTeacherHasAccessToRun(false); replayAll(); try { - controller.getPeerGroups(runId1, run1Node1Id, run1Component1Id, teacherAuth); + controller.getPeerGroupsInfo(runId1, run1Node1Id, run1Component1Id, teacherAuth); fail("Expected AccessDeniedException, but was not thrown"); } catch (AccessDeniedException e) { } @@ -43,32 +45,38 @@ public void getPeerGroups_NoPermissions_AccessDenied() throws Exception { } @Test - public void getPeerGroups_PeerGroupActivityNotFound_ThrowException() throws Exception { + public void getPeerGroupsInfo_PeerGroupActivityNotFound_ThrowException() throws Exception { expectTeacherHasAccessToRun(true); expectPeerGroupActivityNotFound(); replayAll(); try { - controller.getPeerGroups(runId1, run1Node1Id, run1Component1Id, teacherAuth); + controller.getPeerGroupsInfo(runId1, run1Node1Id, run1Component1Id, teacherAuth); fail("Expected PeerGroupActivityNotFoundException, but was not thrown"); } catch (PeerGroupActivityNotFoundException e) { } verifyAll(); } + @SuppressWarnings("unchecked") @Test - public void getPeerGroups_ActivityFound_ReturnList() throws Exception { + public void getPeerGroupsInfo_ActivityFound_ReturnInfo() throws Exception { expectTeacherHasAccessToRun(true); expectPeerGroupActivityFound(); - expectPeerGroups(); + expectPeerGroupInfo(); replayAll(); - List peerGroups = - controller.getPeerGroups(runId1, run1Node1Id, run1Component1Id, teacherAuth); - assertEquals(2, peerGroups.size()); + Map peerGroupsInfo = controller.getPeerGroupsInfo(runId1, run1Node1Id, + run1Component1Id, teacherAuth); + assertEquals(2, peerGroupsInfo.size()); + assertEquals(2, ((List) peerGroupsInfo.get("peerGroups")).size()); + assertEquals(0, ((List) peerGroupsInfo.get("workgroupsNotInPeerGroups")).size()); verifyAll(); } - private void expectPeerGroups() { - expect(peerGroupService.getPeerGroups(peerGroupActivity)).andReturn(peerGroups); + private void expectPeerGroupInfo() { + Map peerGroupInfo = new HashMap(); + peerGroupInfo.put("peerGroups", peerGroups); + peerGroupInfo.put("workgroupsNotInPeerGroups", workgroupsNotInPeerGroups); + expect(peerGroupInfoService.getPeerGroupInfo(peerGroupActivity)).andReturn(peerGroupInfo); } private void expectRunExists() { diff --git a/src/test/java/org/wise/portal/service/WISEServiceTest.java b/src/test/java/org/wise/portal/service/WISEServiceTest.java index ae0fbb549..78d0f8496 100644 --- a/src/test/java/org/wise/portal/service/WISEServiceTest.java +++ b/src/test/java/org/wise/portal/service/WISEServiceTest.java @@ -53,6 +53,8 @@ public class WISEServiceTest { protected Workgroup run1Workgroup1, run1Workgroup2, run1Workgroup3, run1Workgroup4, run1Workgroup5; + protected List run1Workgroups = new ArrayList(); + protected Component run1Component1, run1Component2; protected String run1Node1Id = "run1Node1"; @@ -97,6 +99,7 @@ private Workgroup createWorkgroupInPeriod(Run run, Long id, Group period, User m members.add(member); workgroup.setMembers(members); workgroup.getGroup().setName("Group " + id); + run1Workgroups.add(workgroup); return workgroup; } diff --git a/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupInfoServiceImplTest.java b/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupInfoServiceImplTest.java new file mode 100644 index 000000000..17e204528 --- /dev/null +++ b/src/test/java/org/wise/portal/service/peergroup/impl/PeerGroupInfoServiceImplTest.java @@ -0,0 +1,98 @@ +/** + * Copyright (c) 2008-2021 Regents of the University of California (Regents). + * Created by WISE, Graduate School of Education, University of California, Berkeley. + * + * This software is distributed under the GNU General Public License, v3, + * or (at your option) any later version. + * + * Permission is hereby granted, without written agreement and without license + * or royalty fees, to use, copy, modify, and distribute this software and its + * documentation for any purpose, provided that the above copyright notice and + * the following two paragraphs appear in all copies of this software. + * + * REGENTS SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE. THE SOFTWARE AND ACCOMPANYING DOCUMENTATION, IF ANY, PROVIDED + * HEREUNDER IS PROVIDED "AS IS". REGENTS HAS NO OBLIGATION TO PROVIDE + * MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. + * + * IN NO EVENT SHALL REGENTS BE LIABLE TO ANY PARTY FOR DIRECT, INDIRECT, + * SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING LOST PROFITS, + * ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS DOCUMENTATION, EVEN IF + * REGENTS HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.wise.portal.service.peergroup.impl; + +import static org.junit.Assert.*; +import static org.easymock.EasyMock.*; + +import java.util.List; +import java.util.Map; + +import org.easymock.EasyMockRunner; +import org.easymock.Mock; +import org.easymock.TestSubject; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +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.workgroup.Workgroup; +import org.wise.portal.service.WISEServiceTest; +import org.wise.portal.service.peergroup.PeerGroupService; +import org.wise.portal.service.run.RunService; + +/** + * @author Hiroki Terashima + */ +@SuppressWarnings("unchecked") +@RunWith(EasyMockRunner.class) +public class PeerGroupInfoServiceImplTest extends WISEServiceTest { + + @TestSubject + private PeerGroupInfoServiceImpl service = new PeerGroupInfoServiceImpl(); + + @Mock + private PeerGroupService peerGroupService; + + @Mock + private RunService runService; + + PeerGroupActivity activity; + + PeerGroup peerGroup; + + List peerGroups; + + @Before + public void setUp() throws Exception { + super.setUp(); + PeerGroupServiceTestHelper testHelper = new PeerGroupServiceTestHelper(run1, run1Component2); + activity = testHelper.activity; + peerGroup = testHelper.peerGroup1; + peerGroups = testHelper.peerGroups; + } + + @Test + public void getPeerGroupInfo_ReturnPeerGroupInfo() { + expectGetPeerGroups(); + expectGetWorkgroups(); + replay(peerGroupService, runService); + Map peerGroupInfo = service.getPeerGroupInfo(activity); + assertEquals(1, ((List) peerGroupInfo.get("peerGroups")).size()); + assertEquals(5, ((List) peerGroupInfo.get("workgroupsNotInPeerGroup")).size()); + verify(peerGroupService, runService); + } + + private void expectGetPeerGroups() { + expect(peerGroupService.getPeerGroups(activity)).andReturn(peerGroups); + } + + private void expectGetWorkgroups() { + try { + expect(runService.getWorkgroups(run1Id)).andReturn(run1Workgroups); + } catch (ObjectNotFoundException e) { + } + } +} From 605d264d130f836906b0d9e31659a145a48c29db Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Mon, 22 Nov 2021 10:40:44 -0800 Subject: [PATCH 3/4] Remove PeerGroupActivity from PeerGroup serialization #57 --- .../org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java b/src/main/java/org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java index 28975c6cb..d42ce9669 100644 --- a/src/main/java/org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java +++ b/src/main/java/org/wise/portal/domain/peergroup/impl/PeerGroupImpl.java @@ -71,6 +71,7 @@ public class PeerGroupImpl implements PeerGroup { @ManyToOne(targetEntity = PeerGroupActivityImpl.class, cascade = { CascadeType.PERSIST }, fetch = FetchType.LAZY) @JoinColumn(name = "peerGroupActivityId", nullable = false) + @JsonIgnore private PeerGroupActivity peerGroupActivity; @ManyToMany(targetEntity = WorkgroupImpl.class) From fc9d78ddbea67feeaab5e221d0f60414e82450b2 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Mon, 22 Nov 2021 10:58:11 -0800 Subject: [PATCH 4/4] Exclude HibernateStudentAttendaceDao unit test This test has been known to be finicky, so we decided to exclude from running until we can find time to look into the root cause. --- .../attendance/HibernateStudentAttendanceDaoTest.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/wise/portal/dao/attendance/HibernateStudentAttendanceDaoTest.java b/src/test/java/org/wise/portal/dao/attendance/HibernateStudentAttendanceDaoTest.java index f5c49b256..518bd7cee 100644 --- a/src/test/java/org/wise/portal/dao/attendance/HibernateStudentAttendanceDaoTest.java +++ b/src/test/java/org/wise/portal/dao/attendance/HibernateStudentAttendanceDaoTest.java @@ -32,6 +32,7 @@ import java.util.TreeSet; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -92,16 +93,17 @@ public void setUp() throws Exception { @Test public void getStudentAttendanceByRunId_RunWithNoStudentAttendance_ShouldReturnNone() { - List studentAttendance = + List studentAttendance = studentAttendanceDao.getStudentAttendanceByRunId(run.getId()); assertEquals(0, studentAttendance.size()); } + @Ignore @Test public void getAchievementsByParams_RunWithAchievements_ShouldReturnAchievements() { createStudentAttendance(run.getId(), workgroup1.getId(), getDateXDaysFromNow(1), "[1]", "[]"); createStudentAttendance(run.getId(), workgroup2.getId(), getDateXDaysFromNow(1), "[2]", "[]"); - List studentAttendance = + List studentAttendance = studentAttendanceDao.getStudentAttendanceByRunId(run.getId()); assertEquals(2, studentAttendance.size()); assertEquals("[2]", studentAttendance.get(0).getPresentUserIds()); @@ -112,7 +114,7 @@ public void getAchievementsByParams_RunWithAchievements_ShouldReturnAchievements public void getStudentAttendanceByRunIdAndPeriod_RunWithAchievements_ShouldReturnAchievements() { createStudentAttendance(run.getId(), workgroup1.getId(), getDateXDaysFromNow(-1), "[1]", "[]"); createStudentAttendance(run.getId(), workgroup2.getId(), getDateXDaysFromNow(-10), "[2]", "[]"); - List studentAttendance = + List studentAttendance = studentAttendanceDao.getStudentAttendanceByRunIdAndPeriod(run.getId(), 7); assertEquals(1, studentAttendance.size()); assertEquals("[1]", studentAttendance.get(0).getPresentUserIds()); @@ -129,4 +131,4 @@ public StudentAttendance createStudentAttendance(Long runId, Long workgroupId, studentAttendanceDao.save(studentAttendance); return studentAttendance; } -} \ No newline at end of file +}