SAK-40851 Assignments: Allow Students to use Rubrics for the Peer #8822

MRutea · 2020-11-18T13:04:41Z

SAK-40851 Assignments: Allow Students to use Rubrics for the Peer

assignment/api/src/resources/assignment.properties

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-association.js

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-grading.js

adrianfish

Couple more changes. Keep going :)

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentToolUtils.java

assignment/tool/src/webapp/js/assignments.js

assignment/tool/src/webapp/vm/assignment/chef_assignments_student_view_submission.vm

rubrics/api/src/main/java/org/sakaiproject/rubrics/logic/repository/EvaluationRepository.java

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-association.js

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-grading.js

assignment/tool/src/webapp/vm/assignment/chef_assignments_student_review_edit.vm

assignment/tool/src/webapp/vm/assignment/chef_assignments_student_view_grade.vm

MRutea · 2020-11-23T13:00:16Z

Hi;
First of all, thank you very much for your comments.
I have changed a lot of code following your comments.
Last Friday they were still provisional changes, not definitive ones.
Apologies if they may have caused confusion.

Among today's changes is an effort to decouple rubrics from assignments.
I still need to review some point.
Thanks again.

@adrianfish
@ern

adrianfish · 2020-11-27T15:10:17Z

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java

+                // 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 could be reviews for multiple submissions, it's possible. If not now, in future.

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.

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-association.js

MRutea · 2020-12-01T08:05:59Z

Greetings;

I have been working on all the suggestions you have made to this pull request. Above all, I have made enough effort to separate the logic of Assignment and Rubrics.

Answering some of your questions:

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 also because Evaluationrepository only allows 1 evaluator (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.

Thank you for all your comments.
Waiting for your answers.

@bgarciaentornos
@adrianfish
@ern

adrianfish · 2020-12-01T08:31:27Z

@MRutea
@bgarciaentornos
@ern

I don't know whether making this change limited to one submission at this stage is wise. So, not differentiating between different submissions from the student is limiting the functionality in the future, and probably enforcing the need fo a db conversion script when that functionality (peer rubrics evaluation per submission) inevitably gets asked for. Decisions we make now will have effects down the line. I know UPV may have asked for just one peer review per assignment, but that just seems a false restriction when you could work out an evaluated_item_id scheme to handle multiple submissions. Maybe I'm over thinking it - I'm just aware that we have been asked for full submission histories to be visible to instructors. Which submission would the peer review apply to? Could we ever find that out under your scheme?

bgarciaentornos · 2020-12-01T08:41:01Z

Hi Adrian. The way I see it we're only adding a self-contained new functionality (two, in fact). This means it doesn't affect what was already in Sakai but also it can't anticipate every future use case. Functionally we're just giving some users more useful tools and not removing anything. I feel this is the way contributions usually work in the Sakai project, the code has been adapted to match the standards as it should but going much further will only mean postponing it for something that might or might not happen. If we had a different approach that could be applied smoothly then I'd agree with you, but changing the design at this stage is not feasible. If this was the level for every other addition to the project then I'm pretty sure 21 would look like 12...

Of course you'll have the last word, just giving my 2 cents.

adrianfish · 2020-12-01T09:37:55Z

@bgarciaentornos Can't you just change the design to be able to differentiate between submissions? Use both the student's user id and the assignment submission id as the evaluated entity id? I think the feature's great, but I don't want to be in the position where we have to do a complicated db conversion because of a hasty decision now, just for the sake of what seems a small change to the way this is currently working. If we wanted to make peer or self assessments submission specific in the future we literally would not be able to do a conversion script - we'd just have to delete all the current peer evaluations. If you add this now, no need in future. This is a community decision though - I just know I wouldn't be happy if this was my design. I'd be worried about not tying it to an actual submission and making work in the future. The trouble with adding code that isn't future proof and that creates the need for a complicated db conversion later on, is that nobody wants to do that work later and we end up with more technical debt that nobody wants to touch.

Some Sakai school: "Can we show the students peer reviews for all the submissions? Not just their latest one?"
Me: "Sorry, it's not possible with the current design."
Some Sakai school: "Why not?"
Me: "Because the initial design asked for just the assignment to be peer reviewed."
Some sakai School: "But students can submit multiple times, shouldn't the submission be the thing that is being peer reviewed, not the actual assignment?"
Me: "Maybe. Would you like a quote for the work?"
Me, thinking: "Oh no, we'll have to do a db conversion for this, and it's going to lose data"

@ern What do you think? Am I over thinking this?

adrianfish · 2020-12-01T11:58:38Z

Does this work in the new grader? The new grader's been in since 20.0 and I don't see us maintaining two grading interfaces into the future. I see the new grader as being the grader, so I'd like this feature to work with grader when it goes in. Maybe this should be the subject of a T&L call on a Wednesday?

MRutea · 2020-12-01T12:23:54Z

Hi, Adrian;
@adrianfish
This new feature is not compatible at the moment with the new grader.
Thank you.

adrianfish · 2020-12-01T15:25:42Z

@MRutea @bgarciaentornos @ern
This has been added to the T&L call for tomorrow. If nobody from EDF can make it I'll talk about it. I think the feature is good, but people need to be aware that it is peer review per assignment, not submission, and that the feature won't work with the 20.0 grader (Sakai Grader).

bgarciaentornos · 2020-12-10T07:46:34Z

@adrianfish
Hi, I think the T&L approved it but you said you wanted to add more renaming recommendations, is this right? Thanks.

mpellicer · 2020-12-10T08:16:09Z

Shall we remove the draft status and mark it ready? As far as I can tell all the comments have been addressed?

josecebe · 2021-02-23T14:51:43Z

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java

@@ -203,6 +203,7 @@
 import org.sakaiproject.exception.ServerOverloadException;
 import org.sakaiproject.javax.PagingPosition;
 import org.sakaiproject.message.api.MessageHeader;
+import org.sakaiproject.rubrics.logic.model.ToolItemRubricAssociation;


Not used import

josecebe · 2021-02-23T14:57:23Z

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentToolUtils.java

+        try {
+            Optional<ToolItemRubricAssociation> rubricAssociation = rubricsService.getRubricAssociation(RubricsConstants.RBCS_TOOL_ASSIGNMENT, assignmentId);
+            if (rubricAssociation.isPresent() && rubricAssociation.get().getParameter(RubricsConstants.RBCS_STUDENT_SELF_REPORT)) {
+                context.put("rubricSelfReport", true);


I find a little strange that this method put a parameter inside the context. Maybe this method should only return true or false and then insert the returned value from the controller, instead of doing it from this utils method.

Good point. Thank you!

josecebe · 2021-02-23T14:59:11Z

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentToolUtils.java

@@ -46,8 +46,12 @@
 import org.sakaiproject.assignment.api.model.Assignment;
 import org.sakaiproject.assignment.api.model.AssignmentSubmission;
 import org.sakaiproject.assignment.api.model.AssignmentSubmissionSubmitter;
+import org.sakaiproject.cheftool.Context;


Not need of importin Context if "hasRubricSelfReview" method only returns a boolean value.

josecebe · 2021-02-23T15:04:37Z

assignment/tool/src/webapp/js/assignments.js

+          const doAssociateCheck = document.getElementById("do-associate-radio");
+          const numberReviews = document.getElementById("new_assignment_peer_assessment_num_reviews");
+
+      //page-loading


josecebe · 2021-02-23T15:04:46Z

assignment/tool/src/webapp/js/assignments.js

+    const confirmButton = document.getElementById("confirm");
+    confirmButton && confirmButton.addEventListener("click", saveRubric);
+    const postButton = document.getElementById("post");
+    postButton && postButton.addEventListener("click", saveRubric);


josecebe · 2021-02-23T15:05:17Z

assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_grading_submission.vm

+                        tool-id="sakai.assignment"
+                        entity-id="$assignment.Id"
+                        evaluated-item-id="$submitter"
+                    ></sakai-rubric-student>


josecebe · 2021-02-23T15:09:15Z

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-association.js

@@ -39,6 +39,8 @@ class SakaiRubricAssociation extends RubricsElement {
      associateValue: { attribute: "associate-value", type: Number },
      fineTunePoints: { attribute: "fine-tune-points", type: String },
      hideStudentPreview: { attribute: "hide-student-preview", type: String },
+      studentSelfReport: { attribute: "student-self-report", type: String },
+      showSelfReportCheck: { attribute: "show-self-report-check", type: String },


Both should be Boolean

ok.
but studentSelfReport is a string ->"Allow students to self-report their work"

josecebe · 2021-02-23T15:10:32Z

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-student.js

+              console.log(error);
+            });
+
+        }


Fix all this block identation

MRutea · 2021-02-24T08:26:58Z

@josecebe
Thank you for your comments.

SAK-40851 Assignments: Allow Students to use Rubrics for the Peer SAK-40851 Assignments: Allow Students to use Rubrics for the Peer SAK-40851 Assignments: Allow Students to use Rubrics for the Peer

bgarciaentornos · 2023-09-27T15:20:15Z

This is being replaced by the work done in S2U-34, see #11950 and the original commits for the s2u-22x branch.

MRutea force-pushed the SAK-40851 branch from 73c2d85 to 3792595 Compare November 18, 2020 13:35

adrianfish requested changes Nov 18, 2020

View reviewed changes

MRutea force-pushed the SAK-40851 branch from b88bdac to 5031f0e Compare November 19, 2020 11:31

adrianfish requested changes Nov 19, 2020

View reviewed changes

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java Outdated Show resolved Hide resolved

assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentToolUtils.java Outdated Show resolved Hide resolved

MRutea force-pushed the SAK-40851 branch 2 times, most recently from 59b6764 to bc5caf9 Compare November 20, 2020 09:45

adrianfish requested changes Nov 20, 2020

View reviewed changes

adrianfish marked this pull request as draft November 20, 2020 19:13

MRutea force-pushed the SAK-40851 branch 2 times, most recently from 502cf75 to 195543d Compare November 23, 2020 08:39

adrianfish requested changes Nov 23, 2020

View reviewed changes

MRutea force-pushed the SAK-40851 branch 2 times, most recently from de14165 to bc3cb6f Compare November 23, 2020 12:57

MRutea force-pushed the SAK-40851 branch 2 times, most recently from f39c66f to 5451956 Compare November 27, 2020 12:04

adrianfish reviewed Nov 27, 2020

View reviewed changes

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-association.js Outdated Show resolved Hide resolved

adrianfish reviewed Nov 27, 2020

View reviewed changes

webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubric-association.js Outdated Show resolved Hide resolved

MRutea force-pushed the SAK-40851 branch 2 times, most recently from c85f828 to 839ec15 Compare December 1, 2020 08:01

MRutea force-pushed the SAK-40851 branch 6 times, most recently from c2e7b97 to ed7f85b Compare December 18, 2020 08:23

bjones86 requested a review from adrianfish January 4, 2021 15:16

ern added the future label Jan 12, 2021

ern marked this pull request as draft January 22, 2021 19:08

MRutea force-pushed the SAK-40851 branch 10 times, most recently from 64f2445 to 3327021 Compare February 23, 2021 11:41

josecebe suggested changes Feb 23, 2021

View reviewed changes

MRutea force-pushed the SAK-40851 branch 3 times, most recently from a6a0b87 to 471f31f Compare February 24, 2021 08:26

MRutea force-pushed the SAK-40851 branch from 5d8f369 to 73cb1a2 Compare March 3, 2021 11:34

SAK-40851 Assignments: Allow Students to use Rubrics for the Peer

8cba2b5

SAK-40851 Assignments: Allow Students to use Rubrics for the Peer SAK-40851 Assignments: Allow Students to use Rubrics for the Peer SAK-40851 Assignments: Allow Students to use Rubrics for the Peer

MRutea force-pushed the SAK-40851 branch from 73cb1a2 to 8cba2b5 Compare March 3, 2021 11:35

mpellicer assigned josecebe Mar 30, 2021

bgarciaentornos closed this Sep 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAK-40851 Assignments: Allow Students to use Rubrics for the Peer #8822

SAK-40851 Assignments: Allow Students to use Rubrics for the Peer #8822

MRutea commented Nov 18, 2020 •

edited by jira bot

Loading

adrianfish left a comment

MRutea commented Nov 23, 2020

adrianfish Nov 27, 2020

MRutea Dec 1, 2020

MRutea commented Dec 1, 2020 •

edited

Loading

adrianfish commented Dec 1, 2020

bgarciaentornos commented Dec 1, 2020

adrianfish commented Dec 1, 2020 •

edited

Loading

adrianfish commented Dec 1, 2020

MRutea commented Dec 1, 2020

adrianfish commented Dec 1, 2020

bgarciaentornos commented Dec 10, 2020

mpellicer commented Dec 10, 2020

josecebe Feb 23, 2021

MRutea Feb 23, 2021

josecebe Feb 23, 2021

MRutea Feb 23, 2021

josecebe Feb 23, 2021

MRutea Feb 23, 2021

josecebe Feb 23, 2021

MRutea Feb 23, 2021

josecebe Feb 23, 2021

MRutea Feb 23, 2021

josecebe Feb 23, 2021

MRutea Feb 23, 2021

josecebe Feb 23, 2021

MRutea Feb 23, 2021

josecebe Feb 23, 2021

MRutea Feb 23, 2021

MRutea commented Feb 24, 2021

bgarciaentornos commented Sep 27, 2023

SAK-40851 Assignments: Allow Students to use Rubrics for the Peer #8822

SAK-40851 Assignments: Allow Students to use Rubrics for the Peer #8822

Conversation

MRutea commented Nov 18, 2020 • edited by jira bot Loading

adrianfish left a comment

Choose a reason for hiding this comment

MRutea commented Nov 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MRutea commented Dec 1, 2020 • edited Loading

adrianfish commented Dec 1, 2020

bgarciaentornos commented Dec 1, 2020

adrianfish commented Dec 1, 2020 • edited Loading

adrianfish commented Dec 1, 2020

MRutea commented Dec 1, 2020

adrianfish commented Dec 1, 2020

bgarciaentornos commented Dec 10, 2020

mpellicer commented Dec 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MRutea commented Feb 24, 2021

bgarciaentornos commented Sep 27, 2023

MRutea commented Nov 18, 2020 •

edited by jira bot

Loading

MRutea commented Dec 1, 2020 •

edited

Loading

adrianfish commented Dec 1, 2020 •

edited

Loading