Skip to content
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.

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

feat(Peer Group): Add teacher endpoint to get PeerGroup info for activity #58

Conversation

hirokiterashima
Copy link
Member

@hirokiterashima hirokiterashima commented Nov 16, 2021

Changes

  • Added new teacher endpoint /api/teacher/peer-group-info/RUN_ID/NODE_ID/COMPONENT_ID that will return PeerGroups and Workgroups that aren't in PeerGroups for the specified activity
  • Requires database change: add periodId bigint field to peer_groups table

Test Prep

  • add periodId column to peer_groups table: alter table peer_groups add column periodId bigint;

Test

  • Log in as a teacher
  • Make request to /api/teacher/peer-group-info/RUN_ID/NODE_ID/COMPONENT_ID
    • If valid runId/nodeId/componentId combination and teacher does not have permissions, should see AccessDenied
    • If invalid runId/nodeId/componentId combination, should throw error
    • If valid runId/nodeId/componentId combination and teacher has permissions, should return a map containing two objects:
      • "peerGroups": list of PeerGroups for the activity in the run
      • "workgroupsNotInPeerGroup": list of Workgroups that are not in a PeerGroup for the activity

Closes #57

Requires database change: add periodId bigint field to peer_groups table

Closes #57
- url: /api/teacher/peer-group-info returns a Map of PeerGroups and
workgroups that don't belong in PeerGroups for the specified
PeerGroupActivity
@hirokiterashima hirokiterashima changed the title feat(Peer Group): Add teacher endpoint to get PeerGroups for activity feat(Peer Group): Add teacher endpoint to get PeerGroup info for activity Nov 18, 2021
@geoffreykwan
Copy link
Member

Should we be sending the peerGroupActivity object with every peerGroup? It's duplicated for every peerGroup. Do we even need the peerGroupActivity information in the response?

{
	"peerGroups": [{
		"id": 39,
		"peerGroupActivity": {
			"id": 9,
			"nodeId": "node51",
			"componentId": "70ewf4mmov",
			"logic": "[{\"componentId\":\"sneu5vdrcr\",\"name\":\"maximizeSimilarIdeas\",\"nodeId\":\"node50\"}]",
			"logicThresholdCount": 3,
			"logicThresholdPercent": 100,
			"maxMembershipCount": 2,
			"logicNodeId": "node50",
			"logicComponentId": "sneu5vdrcr"
		},
		"members": [{
			"id": 235,
			"periodId": 306
		}, {
			"id": 234,
			"periodId": 306
		}],
		"periodId": 306
	}, {
		"id": 40,
		"peerGroupActivity": {
			"id": 9,
			"nodeId": "node51",
			"componentId": "70ewf4mmov",
			"logic": "[{\"componentId\":\"sneu5vdrcr\",\"name\":\"maximizeSimilarIdeas\",\"nodeId\":\"node50\"}]",
			"logicThresholdCount": 3,
			"logicThresholdPercent": 100,
			"maxMembershipCount": 2,
			"logicNodeId": "node50",
			"logicComponentId": "sneu5vdrcr"
		},
		"members": [{
			"id": 238,
			"periodId": 307
		}, {
			"id": 239,
			"periodId": 307
		}],
		"periodId": 307
	}],
	"workgroupsNotInPeerGroup": [{
		"id": 236,
		"periodId": 306
	}, {
		"id": 237,
		"periodId": 306
	}, {
		"id": 240,
		"periodId": 307
	}]
}

@hirokiterashima
Copy link
Member Author

This is the default serialization output.

We can write a custom JSON serializer for PeerGroup where we remove the PeerGroupActivity field altogether. This serialization would apply to all GET requests to PeerGroup. Or should we keep just the PeerGroupActivity.id field in cases where we'd request PeerGroups for different PeerGroupActivities (e.g. "get all PeerGroups for run")?

{
  "id": 39,
  "peerGroupActivity": {
    "id": 9			
  },
 "members": [{
   "id": 235,
   ...
  }]
}   

@hirokiterashima
Copy link
Member Author

@geoffreykwan as discussed, I removed PeerGroupActivity from PeerGroup serialization

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.
Copy link
Member

@geoffreykwan geoffreykwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@geoffreykwan geoffreykwan merged commit 9cc356a into issue-37-peer-interaction Nov 23, 2021
@geoffreykwan geoffreykwan deleted the issue-57-add-teacher-endpoint-to-get-peer-groups-for-activity branch November 23, 2021 20:45
geoffreykwan pushed a commit that referenced this pull request May 13, 2022
* Create domain objects and dao's for PeerGroup #38 (#39)
Requires creating new tables in the database (see wise_db_init.sql).
Domain objects include PeerGroupActivity and PeerGroup.
Add unit tests.
* feat(Peer Interaction): Add service method to get PeerGroupActivity (#41)
- Add PeerGroupActivityService and getByComponent() method to return
PeerGroupActivity for a specific component.
- Add supporting classes (ProjectContent, ProjectNode, ProjectComponent)
to parse the project JSON.
- PeerGroupActivityNotFoundException is thrown if a PeerGroupActivity
for the component cannot be found in the database and curriculum
content.
* feat(Peer Group): Add service method to get/create PeerGroup #42 (#43)
- Assumes PeerGrouping logic looks at the same nodeId/componentId
- Extracted common hibernate/service test methods to parent.
* feat(Peer Group): Add endpoint for getting Peer Group (#45)
* feat(Peer Group): Add endpoint to get PeerGroup
- Ensure there are at least 2 submissions before pairing
* feat(Peer Group): Add endpoint to get PeerGroup
- Threshold not met now throws an exception instead of returning null.
- Throws PeerGroupCreationException when there are other workgroups
to wait for.
- Fix issue calculating completion percentage by using float instead
of int.
* feat(Peer Group): Add endpoint to get PeerGroup
- Add id equality test to WorkgroupImpl and PersistentGroup
- Changed getNumWorkgroupsInPeerGroup() to get PeerGroup for activity
component instead of logic component
- Return exception key in ResponseBody
* feat(Peer Group): Add endpoint to get StudentWork for PeerGroup (#48)
- Also change URL to get PeerGroup to
  /{runId}/{workgroupId}/{nodeId}/{componentId}
* feat(Peer Group): Add teacher endpoint to get work for peer group (#50)
* refactor(Peer Group): Move PeerGroupWork api to new class
* feat(Peer Group): Add teacher endpoint to get work for PeerGroup
- Extract common test methods to AbstractPeerGroupAPIControllerTest
* feat(Peer Group): Make PeerGroup with remaining students (#53)
- When maxMembershipCount + 1 students have not been pair yet, and
everybody else have completed the logic activity, we put them all
into the same peer group.
- Also updated code to ensure that we only look for workgroups
in the same period when pairing students into PeerGroups.
* feat(Peer Group): Add teacher endpoint to get PeerGroup info for activity (#58)
* feat(Peer Group): Add teacher endpoint to get PeerGroups for activity
Requires database change: add periodId bigint field to peer_groups table
Closes #57
* 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
* Remove PeerGroupActivity from PeerGroup serialization #57
* 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.
* feat(Get Classmate Work): Create endpoint for Peer Chat other work (#63)
The other work is the work from a previous component that is shown at the top of the Peer Chat component.
Closes #62
* feat(Peer-Chat): Add endpoint to get latest student work for other
component
* test(Peer-Chat): Fix PeerChatDataControllerTest
* feat(Peer-Group): Add teacher endpoint to create peer group
* feat(Group): Add GroupRepository
* feat(PeerGroup): Add teacher endpoint for creating PeerGroup
- Needed to edit APIControllerTest to use the PersistentGroup class type
  now that we are using DomainConverter to directly pass in objects
  instead of Long
* feat(PeerGroup): Add teacher endpoint for creating PeerGroup
- Ensure that requested period is in requested run.
- Modified test's period name to distinguish from other periods
* feat(Peer-Group): Add teacher endpoint to add member to peer group
* feat(Group): Add GroupRepository
* feat(PeerGroup): Add teacher endpoint for creating PeerGroup
- Needed to edit APIControllerTest to use the PersistentGroup class type
  now that we are using DomainConverter to directly pass in objects
  instead of Long
* feat(Peer-Chat): Add endpoint to add member to PeerGroup
- Added PeerGroupRepository to convert PeerGroupId PathVariables to
  PeerGroup objects and updated tests to use impl classes.
* feat(Peer-Chat): Add endpoint to add member to PeerGroup
- Don't allow addMember if requested workgroup does not belong in the
  run.
* fix(Test): Fix a test that was not compiling
* feat(Peer Group): Add ability to get manually assigned peer group (#77)
* feat(Peer Group): Add ability to get manually assigned peer group
If a Peer Activity uses auto assignment and a Peer Group is requested
and the Peer Group does not exist, it will automatically be created.
If a Peer Activity uses manual assignment and a Peer Group is requested
and the Peer Group does not exist, it will not be automatically created.
* refactor(PeerGroup): Change PeerGroupServiceHelperTest to extend WISEServiceTest
- Renamed PeerGroupServiceHelperTest to PeerGroupServiceTest
* feat(PeerGroup): Add teacher endpoint to remove member from PeerGroup
* Merge branch: 'get-peer-group-by-tag' into issue-37-peer-interaction
* feat(PeerGroup): Add endpoint to get by Tag
* feat(PeerGroup): Create PeerGroupActivity from tag in project
When a peerGroupActivityTag is added in the project in the authoring tool
or if a new run is created from the project that contains the tag,
PeerGroupActivities are created for each tag.
* refactor(PeerGroup): Use PeerGroupActivityTag to get PeerGroup (#96)
* refactor(PeerGroup): Use PeerGroupActivityTag to get PeerGroup
- Switch to using the new PeerGroupActivityTag to lookup the
PeerGroupActrivity instead of nodeId/componentId.
- Add tag column to db init sql
- Clean up code
* Remove unused exceptions
* feat(PeerChat): Group students randomly (#94)
* feat(PeerChat): Group students randomly
* Improve addMembersRandomly efficiency
Removed randomly chosen members from future consideration.
* Optimize addMembersRandomly method
* refactor(PeerGroup): Pass in domain objects to controllers (#99)
* feat(PeerChat): Allow teacher participation
- Add endpoint at /api/teacher/run/{runId}/work for teachers to POST work
- Add PeerGroupId to StudentWork (requires db update, see below)
* refactor(PeerGroupActivity): Remove nodeId and componentId fields
- Remove nodeId, componentId fields from PeerGroupActivity
- Set PeerGroupActivity.logic field's default value to [{"name":"manual"}]
- Remove PeerGroupActivityDao.getByComponent()
* feat(Peer Group Activity): Move settings to project level (#108)
* refactor(PeerGroupActivityDao): Remove getByComponent()
Layers above should now be using getByTag() to look up the
PeerGroupActivity.
* refactor(PeerGroupActivityImpl): Remove nodeId, componentId fields
Also set logic field's default value to [{"name":"manual"}]
* Remove nodeId/componentId fields from db init sql
* refactor(PeerGroupActivity): Change logic field from JSON array to string (#107)
Instead of expressing the logic entirely in the logic field, use the
logic field to only store a string from the following: "manual",
"random", "custom".
* feat(Peer Group Activity): Move settings to project-level
- Introduce new "peerGroupActivity" project level field, an array to store
PeerGroupActivities in the unit
* refactor(Peer Group API): Remove getPeerGroup by component (#110)
- This API endpoint has been replaced by getting PeerGroup by Tag
* feat(Peer Group Settings): Add endpoint to create and edit (#113)
* feat(ACL): Add support for parsing permission string
* feat(Peer Group Settings): Add endpoint to create/edit
* feat(Peer Group Settings): Remove scan for tags on project save
* chore(Peer Grouping): Rename Peer Group Activity to Peer Grouping (#115)
* fix(PeerGrouping): Random pairing (#120)
- Also remove completion requirement before pairing
- Fix tests
* fix(Peer Grouping): Create run with Peer Groupings with correct fields (#121)
* fix(Peer Grouping): Return bad request when creating duplicate tag
* fix(PeerGroup): Don't pair with empty workgroups
* fix(Peer Group): Don't count empty workgroups when check if can create
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(Peer Group): Add Teacher endpoint to get PeerGroups for an activity
3 participants