sakai
sakai copied to clipboard
sakaiproject
SAK-40851 Assignments: Allow Students to use Rubrics for the Peer
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
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
@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?
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.
@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?
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?
Hi, Adrian; @adrianfish This new feature is not compatible at the moment with the new grader. Thank you.
@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).
@adrianfish Hi, I think the T&L approved it but you said you wanted to add more renaming recommendations, is this right? Thanks.
Shall we remove the draft status and mark it ready? As far as I can tell all the comments have been addressed?
Thank you, @adrianfish, for your new comments and the commit. I am doing new tests. As soon as possible I tell you the test results. @bgarciaentornos
Hi;
I've been doing tests after the last changes. And now everything works fine.
I've added some light changes like adding an event (or reused) to set total points when the students do their peer review of other student.
@adrianfish @bgarciaentornos
This is being replaced by the work done in S2U-34, see https://github.com/sakaiproject/sakai/pull/11950 and the original commits for the s2u-22x branch.