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

SAK-50907 Assignments add option to replicate the self-report rubric to the grading rubric #13229

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

st-manu
Copy link
Contributor

@st-manu st-manu commented Jan 23, 2025

https://sakaiproject.atlassian.net/browse/SAK-50907

Add the option to replicate the self-report rubric to the grading rubric.
clasic
new

@ern ern changed the title SAK-50907 Assignments: New option to replicate the self-report rubric to the grading rubric SAK-50907 Assignments add option to replicate the self-report rubric to the grading rubric Jan 28, 2025
Copy link
Contributor

@adrianfish adrianfish left a comment

Choose a reason for hiding this comment

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

I actually think that this should all happen on the server. It should be a service call on the rubrics service. All you are doing, effectively, is copying an evaluation to another and it would be way more reliable to do this server side, with some unit tests. Calling into the rubric web component like you have will probably break at some point in the future.

@@ -1006,6 +1006,92 @@ ASN.disableTimesheetSetupSection = function()
ASN.toggleAutoAnnounceEstimate(false);
}

ASN.autocompleteRubricWithSelfReport = function(event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is a brand new function, you should probably use a two space indentation. It's what we use in all the webcomponents and what we specify in our eslint config.

Comment on lines +1024 to +1033
const evaluationDetails = [];
const criterionRows = studentRubric.querySelectorAll(".criterion-row");
criterionRows.forEach(row => {
const criterionId = row.id.split("_").pop();
const selectedRating = row.querySelector(".rating-item.selected");
if (selectedRating) {
const selectedRatingId = selectedRating.id.split("-").pop();
evaluationDetails.push({ criterionId, selectedRatingId });
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const evaluationDetails = [];
const criterionRows = studentRubric.querySelectorAll(".criterion-row");
criterionRows.forEach(row => {
const criterionId = row.id.split("_").pop();
const selectedRating = row.querySelector(".rating-item.selected");
if (selectedRating) {
const selectedRatingId = selectedRating.id.split("-").pop();
evaluationDetails.push({ criterionId, selectedRatingId });
}
});
const evaluationDetails = [...studentRubric.querySelectorAll(".criterion-row")].reduce(acc, row) => {
const selectedRatingId = row.querySelector(".rating-item.selected")?.id.split("-").pop();
selectedRatingId && acc.push({ criterionId: row.id.split("_").pop(), selectedRatingId });
return acc;
}, []);

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm spreading the NodeList into an array and then reducing the array. I'm using optional chaining to remove some of the variables you had introduced.

Comment on lines +1047 to +1064
evaluationDetails.forEach(detail => {
const criterionRow = gradingRubric.querySelector(`#criterion_row_${detail.criterionId}`);
if (criterionRow) {
const ratingItem = criterionRow.querySelector(`.rating-item[data-rating-id="${detail.selectedRatingId}"]`);
if (ratingItem) {
ratingItem.click();
const pointsElement = ratingItem.querySelector(".points");
const pointsText = pointsElement.textContent.trim();
let points = parseFloat(pointsText.replace(",", "."));
const pointsInParentheses = pointsElement.querySelector("b");
if (pointsInParentheses) {
const pointsInParenthesesText = pointsInParentheses.textContent.trim().replace(/[()]/g, '');
points = parseFloat(pointsInParenthesesText.replace(",", "."));
}
totalPoints += points;
}
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole block should go into the rubric code somewhere. It seems a bit fragile, especially selecting the bold tag to find the points in parentheses. You could move the code into rubrics and add a unit test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants