-
-
Notifications
You must be signed in to change notification settings - Fork 949
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
SAK-40851 Assignments: Allow Students to use Rubrics for the Peer #8822
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1057,6 +1057,10 @@ public class AssignmentAction extends PagedResourceActionII { | |
* Site property for forcing anonymous grading in a site | ||
*/ | ||
private static final String SAK_PROP_FORCE_ANON_GRADING = "assignment.anon.grading.forced"; | ||
/** | ||
* submitter, the student who do a selfreport | ||
*/ | ||
private static final String STUDENT_WITH_SELFREPORT = "submitter"; | ||
|
||
private static final String FLAG_ON = "on"; | ||
private static final String FLAG_TRUE = "true"; | ||
|
@@ -1763,6 +1767,8 @@ private String build_student_view_submission_context(VelocityPortlet portlet, Co | |
canViewAssignmentIntoContext(context, assignment, s); | ||
|
||
addAdditionalNotesToContext(submitter, context, state); | ||
|
||
context.put("rubricSelfReport", assignmentToolUtils.hasRubricSelfReview(assignment.getId())); | ||
} | ||
|
||
if (taggingManager.isTaggable() && assignment != null) { | ||
|
@@ -2287,6 +2293,11 @@ protected String build_student_confirm_submission_context(VelocityPortlet portle | |
} | ||
} | ||
|
||
context.put(STUDENT_WITH_SELFREPORT, | ||
Optional.ofNullable((User) state.getAttribute("student")).orElse(user)); | ||
|
||
context.put("rubricSelfReport", assignmentToolUtils.hasRubricSelfReview(assignment.getId())); | ||
|
||
context.put("text", state.getAttribute(PREVIEW_SUBMISSION_TEXT)); | ||
Map<String, Reference> submissionAttachmentReferences = new HashMap<>(); | ||
stripInvisibleAttachments(state.getAttribute(PREVIEW_SUBMISSION_ATTACHMENTS)).forEach(r -> submissionAttachmentReferences.put(r.getId(), r)); | ||
|
@@ -3600,6 +3611,19 @@ protected String build_instructor_grade_submission_context(VelocityPortlet portl | |
} | ||
context.put("value_grades", grades); | ||
} | ||
|
||
// Check if the assignment has a rubric associated or not | ||
context.put(RubricsConstants.RBCS_HAS_ASSOCIATED_RUBRIC, rubricsService.hasAssociatedRubric(RubricsConstants.RBCS_TOOL_ASSIGNMENT, a.getId())); | ||
if (assignmentToolUtils.hasRubricSelfReview(a.getId())) { | ||
s.getSubmitters().stream().findAny().ifPresent(u -> context.put(STUDENT_WITH_SELFREPORT, u.getSubmitter())); | ||
} | ||
// Only one peer review for an assignment submission per student. | ||
if (a.getAllowPeerAssessment()) { | ||
List<PeerAssessmentItem> reviews = assignmentPeerAssessmentService.getPeerAssessmentItems(s.getId(), a.getScaleFactor()); | ||
if(reviews.size() > 1) { log.warn("More than one peer review were found for Rubric assignment"); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There could be reviews for multiple submissions, it's possible. If not now, in future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The limitation to 1 peer-review per assignment is for two reasons: our client asked us that way and the second reason so that the teacher's screen can be clearly qualified and because in the repository only 1 evaluator allows (already either by submission or by student id). If in the future someone wants something else, it is already outside the scope of this ticket, it would be a change like any other. |
||
reviews.stream().findFirst().ifPresent(pai -> context.put(STUDENT_WITH_SELFREPORT, pai.getId().getAssessorUserId())); | ||
} | ||
|
||
} | ||
|
||
// show alert if student is working on a draft | ||
|
@@ -3844,6 +3868,10 @@ protected String build_instructor_grade_submission_context(VelocityPortlet portl | |
// Check if the assignment has a rubric associated or not | ||
context.put("hasAssociatedRubric", assignment.isPresent() && rubricsService.hasAssociatedRubric(RubricsConstants.RBCS_TOOL_ASSIGNMENT, assignment.get().getId())); | ||
|
||
if (state.getAttribute(RUBRIC_STATE_DETAILS) != null) { | ||
context.put(RUBRIC_STATE_DETAILS, state.getAttribute(RUBRIC_STATE_DETAILS)); | ||
} | ||
|
||
String siteId = (String) state.getAttribute(STATE_CONTEXT_STRING); | ||
String toolId = toolManager.getCurrentPlacement().getId(); | ||
|
||
|
@@ -4859,6 +4887,9 @@ private String build_student_review_edit_context(VelocityPortlet portlet, Contex | |
securityService.popAdvisor(secAdv); | ||
} | ||
} | ||
|
||
context.put(RubricsConstants.RBCS_HAS_ASSOCIATED_RUBRIC, rubricsService.hasAssociatedRubric(RubricsConstants.RBCS_TOOL_ASSIGNMENT, assignment.getId())); | ||
|
||
if (s != null) { | ||
submissionId = s.getId(); | ||
context.put("submission", s); | ||
|
@@ -4919,7 +4950,7 @@ private String build_student_review_edit_context(VelocityPortlet portlet, Contex | |
} else { | ||
context.put("view_only", false); | ||
} | ||
|
||
context.put(RubricsConstants.RBCS_ASSESSOR_ID, peerAssessmentItem.getId().getAssessorUserId()); | ||
// get attachments for peer review item | ||
List<PeerAssessmentAttachment> attachments = assignmentPeerAssessmentService.getPeerAssessmentAttachments(peerAssessmentItem.getId().getSubmissionId(), peerAssessmentItem.getId().getAssessorUserId()); | ||
List<Reference> attachmentRefList = new ArrayList<>(); | ||
|
@@ -7035,6 +7066,8 @@ private void setNewAssignmentParameters(RunData data, boolean validify) { | |
} catch (Exception e) { | ||
addAlert(state, rb.getString("peerassessment.invalidNumReview")); | ||
} | ||
} else if (params.get(RubricsConstants.RBCS_ASSOCIATE).equals("1")) { | ||
state.setAttribute(NEW_ASSIGNMENT_PEER_ASSESSMENT_NUM_REVIEWS, 1); | ||
} else { | ||
addAlert(state, rb.getString("peerassessment.specifyNumReview")); | ||
} | ||
|
@@ -9285,7 +9318,7 @@ public void doView_assignment_as_student(RunData data) { | |
public void doView_submissionReviews(RunData data) { | ||
String submissionId = data.getParameters().getString("submissionId"); | ||
SessionState state = ((JetspeedRunData) data).getPortletSessionState(((JetspeedRunData) data).getJs_peid()); | ||
String assessorId = data.getParameters().getString("assessorId"); | ||
String assessorId = data.getParameters().getString(RubricsConstants.RBCS_ASSESSOR_ID); | ||
String assignmentId = StringUtils.trimToNull(data.getParameters().getString("assignmentId")); | ||
Assignment a = getAssignment(assignmentId, "doEdit_assignment", state); | ||
if (submissionId != null && !"".equals(submissionId) && a != null) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -717,11 +717,11 @@ ASN.enableSubmitUnlessNoFile = function(checkForFile) | |
|
||
if (doEnable) | ||
{ | ||
btnPost.removeAttribute('disabled'); | ||
btnPost.disabled = false; | ||
} | ||
else | ||
{ | ||
btnPost.setAttribute('disabled', 'disabled'); | ||
btnPost.disabled = true; | ||
} | ||
}; | ||
|
||
|
@@ -962,6 +962,34 @@ ASN.changeVisibleDate = function() | |
|
||
$(document).ready(() => { | ||
|
||
//peer-review and self-report | ||
$('body').on('rubric-association-loaded', e => { | ||
if($('#dont-associate-radio').length) { | ||
const dontAssociateCheck = document.getElementById("dont-associate-radio"); | ||
const doAssociateCheck = document.getElementById("do-associate-radio"); | ||
const numberReviews = document.getElementById("new_assignment_peer_assessment_num_reviews"); | ||
|
||
//page-loading | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Identation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
if (dontAssociateCheck.checked) { | ||
numberReviews.disabled = false; | ||
} | ||
|
||
if (doAssociateCheck.checked) { | ||
numberReviews.disabled = true; | ||
numberReviews.value = 1; | ||
} | ||
|
||
//event manage | ||
dontAssociateCheck.addEventListener("change", function() { | ||
numberReviews.disabled = false; | ||
}); | ||
doAssociateCheck.addEventListener("change", function() { | ||
numberReviews.disabled = true; | ||
numberReviews.value = 1; | ||
}); | ||
} | ||
}); | ||
|
||
ottenhoff marked this conversation as resolved.
Show resolved
Hide resolved
|
||
$("#infoImg").popover({html : true}); | ||
|
||
const saveRubric = e => { | ||
|
@@ -985,4 +1013,9 @@ $(document).ready(() => { | |
[...document.getElementsByTagName("sakai-rubric-student-button")].forEach(b => promises.push(b.releaseEvaluation())); | ||
Promise.all(promises).then(() => ASN.submitForm('viewForm', 'releaseGrades', null, null)); | ||
}); | ||
|
||
const confirmButton = document.getElementById("confirm"); | ||
confirmButton && confirmButton.addEventListener("click", saveRubric); | ||
const postButton = document.getElementById("post"); | ||
postButton && postButton.addEventListener("click", saveRubric); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same block of code :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasRubricSelfReport is a bit strange maybe hasRubricSelfReview?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no group logic here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Earle, trying to answer some of these questions as the original design and work was done by myself. Support for grouped submissions wasn't a requirement for these new features, so we believe they should be addressed in SAK-41604. Is this what you mean? Thanks.