S2U-34 Assignments add rubric support for peer review #11950
Conversation
|
Thanks! 5 years since the original development and after more than a few updates, this might make it to master! |
ern
changed the title
| @Getter @Setter private ScheduledInvocationManager scheduledInvocationManager; | ||
| @Getter @Setter protected AssignmentService assignmentService; | ||
| @Getter @Setter private SecurityService securityService = null; | ||
| @Getter @Setter private SessionManager sessionManager; | ||
| @Getter @Setter private SiteService siteService; |
There was a problem hiding this comment.
most of the time you only need Setter for services
There was a problem hiding this comment.
Yes, I think we can remove the getters, right @bgarciaentornos?
There was a problem hiding this comment.
Hey Markus @stetsche, I think we can remove them, however, I'm not a big fan of changing code that is working and has been verified, even if they are simple and small changes. Let's take advantage of the effort spent on this, this is running in production for many years right now.
I prefer to not introduce differences between Master and the 22.x version of the feature, we did that in the past and that practices almost killed all of our rubrics features, this has been around for five years and we want to be consistent between versions.
Let's stick with what it is in 22.x please.
|
Does this need a conversion script for 24? |
|
It seems to work without, but I'll create a PR for it as well. |
stetsche
force-pushed
the
S2U-34-MASTER
branch
from
bd541d3 to
b6652b1
Compare
|
It does need a conversion script, forgot to mention it initially |
stetsche
force-pushed
the
S2U-34-MASTER
branch
from
b6652b1 to
0d675d4
Compare
stetsche
force-pushed
the
S2U-34-MASTER
branch
from
0d675d4 to
16d0950
Compare
| // If the assignment reference is not equal to the associated gradebook item, then a custom gb item is being used | ||
| if (!assignmentRef.equals(assignmentAssociateGradebook)) { | ||
| try { | ||
| org.sakaiproject.grading.api.Assignment gbAssignment = gradingService.getAssignment(gradebookUid, |
There was a problem hiding this comment.
Please put a comment about what differences are between Master and 22.x, what lines we replaced, etc. Thanks
There was a problem hiding this comment.
The logic between master and 22 here should work the same.
On 22.x an org.sakaiproject.service.gradebook.shared.Assignment object is retrieved from the GradebookService if it's null we don't set the context variables.
For the master an org.sakaiproject.service.gradebook.shared.Assignment object is retrieved from the GradingService, if an AssessmentNotFoundException (yes Assessment) thrown, the variables are not added to the context.
There was a problem hiding this comment.
But put a comment int the source! :D
|
Please put comments in the relevant parts where there are differences between 22.x and Master. |
There was a problem hiding this comment.
@mpellicer I added a few comments related to the migration.
webcomponents/tool/src/main/frontend/js/rubrics/sakai-rubrics-api-mixin.js
Show resolved
Hide resolved
|
|
||
| if (StringUtils.isBlank(id)) return false; | ||
|
|
||
| return associationRepository.findByToolIdAndItemId(tool, id).filter(canEvaluate.or(canEdit).or(isCreator)).isPresent(); | ||
| return getRubricAssociation(tool, id).isPresent(); |
There was a problem hiding this comment.
Like in the s2u22x version this will bypass the permission check
| // If the assignment reference is not equal to the associated gradebook item, then a custom gb item is being used | ||
| if (!assignmentRef.equals(assignmentAssociateGradebook)) { | ||
| try { | ||
| org.sakaiproject.grading.api.Assignment gbAssignment = gradingService.getAssignment(gradebookUid, |
There was a problem hiding this comment.
The logic between master and 22 here should work the same.
On 22.x an org.sakaiproject.service.gradebook.shared.Assignment object is retrieved from the GradebookService if it's null we don't set the context variables.
For the master an org.sakaiproject.service.gradebook.shared.Assignment object is retrieved from the GradingService, if an AssessmentNotFoundException (yes Assessment) thrown, the variables are not added to the context.
assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java
Show resolved
Hide resolved
rubrics/impl/src/main/java/org/sakaiproject/rubrics/impl/RubricsServiceImpl.java
Show resolved
Hide resolved
…csServiceImpl.java
…gnmentAction.java
| Long associateGradebookAssignmentId = gbAssignment.getId(); | ||
| context.put("associatedToGbItem", true); | ||
| context.put("associatedToGbEntityId", associateGradebookAssignmentId); | ||
| } catch (AssessmentNotFoundException e) { |
There was a problem hiding this comment.
AssessmentNotFoundException has been in edu-services forever. To the grading service, Assignment = Assessment. Go figure.
|
Terrific work guys, I want to thank you all involved in this piece of work for five years, it's been a long journey but it's definitely contributed. @bgarciaentornos @stetsche @MRutea @josecebe and Manu Fuster. |
https://sakaiproject.atlassian.net/browse/S2U-34
Conversion script: sakaiproject/sakai-reference#222