-
-
Notifications
You must be signed in to change notification settings - Fork 935
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-50187 Assignments Add visibility flag for gradebook item ui #12784
base: master
Are you sure you want to change the base?
Conversation
This is an enhanced version of the initial PR for SAK-50187. In this version I basically add transfer beans for 3 of the assignment hibernate objects, Assignment, AssignmentSubmission and AssignmentSubmissionSubmitter. This means that manipulations on the hibernate proxied JPA objects are more limited to the service layer, which makes for safer and less error prone code. Mods to the transfer beans will not result in db updates - they are stupid. I also refactored the sort comparators used in the frontend - they are now served from a factory. They should probably move into the service layer, but it was just draining getting this far. Anyway, whoever may end up reviewing this - enjoy :) |
@@ -24,6 +24,7 @@ | |||
|
|||
import org.sakaiproject.api.app.messageforums.OpenForum; | |||
import org.sakaiproject.assignment.api.model.Assignment; |
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.
Remove if it is not used now
@@ -733,15 +733,15 @@ public String createSubmission( | |||
//s.setUserId(user.getId()); | |||
//s.setUserEid(userId); | |||
|
|||
AssignmentSubmission ase = assignmentService.addSubmission(assignmentId, userDirectoryService.getUserId(userId)); | |||
SubmissionTransferBean ase = assignmentService.addSubmission(assignmentId, userDirectoryService.getUserId(userId)); |
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.
Spacing
|
||
AssignmentSubmissionSubmitter submitter = new AssignmentSubmissionSubmitter(); | ||
SubmitterTransferBean submitter = new SubmitterTransferBean(); |
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.
Spacing
submitter.setSubmitter(user.getId()); | ||
submitter.setSubmittee(true); | ||
ase.setSubmitted(true); | ||
|
||
Instant subTime = Instant.ofEpochMilli(time); | ||
log.info("Setting time to " + time); | ||
log.info("Setting time to {}", time); |
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.
Spacing
@@ -769,7 +769,7 @@ public String addSubmissionAttachment( | |||
try { | |||
// establish the session | |||
Session s = establishSession(sessionId); | |||
AssignmentSubmission sub = assignmentService.getSubmission(submissionId); | |||
SubmissionTransferBean sub = assignmentService.getSubmission(submissionId); |
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.
Spacing
@@ -28,6 +29,10 @@ public boolean getBoolean(String name, boolean dflt) { | |||
return false; | |||
} | |||
|
|||
public List<String> getStringList(String name) { |
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.
Spacing
if (a == null) { | ||
visible = false; | ||
} else if (Assignment.Access.GROUP.equals(a.getTypeOfAccess())) { | ||
ArrayList<String> azgList = new ArrayList<String>((Collection<String>) a.getGroups()); | ||
//ArrayList<String> azgList = new ArrayList<>((Collection<String>) a.getGroups()); |
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.
Remove
Map<String, List<String>> allExternals = new HashMap<String, List<String>>(); | ||
final Map<String, List<String>> allExternals | ||
= studentIds.stream().collect(Collectors.toMap(Function.identity(), s -> new ArrayList<>())); | ||
/* |
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.
Remove comment
Please hold on with merging this one. It's a very very big piece and it clashes heavily with a development we've been doing since 2023 (our initial attempt of contributing it was stopped and then extended and evolved the feature). So please at least let as many people as possible review this, identify potential issues and then we'll try to catch up with this all somehow. Thanks. |
+1 |
Please and thank you |
https://sakaiproject.atlassian.net/browse/SAK-50187