SAK-49766 Assignments releasing submissions should work consistently

https://sakaiproject.atlassian.net/browse/SAK-49766 - this also removes the outdated option withGrade
sakaiproject · Feb 20, 2024 · b79e2d0 · b79e2d0
1 parent 6176591
commit b79e2d0
Show file tree

Hide file tree

Showing 23 changed files with 711 additions and 805 deletions.
diff --git a/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java b/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java
@@ -333,8 +333,6 @@ public enum IMSGradingProgress {
 
     public static final String STATE_CONTEXT_STRING = "Assignment.context_string";
 
-    public static final String WITH_GRADES = "with_grades";
-
     public static final String GRADE_SUBMISSION_SUBMISSION_ID = "grade_submission_submission_id";
     public static final String GRADE_SUBMISSION_ASSIGNMENT_ID = "grade_submission_assignment_id";
     public static final String GRADE_SUBMISSION_GRADE = "grade_submission_grade";

diff --git a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
@@ -2936,8 +2936,6 @@ public boolean isGradeOverridden(AssignmentSubmission submission, String submitt
      * Contains logic to consistently output a String based version of a grade
      * Interprets the grade using the scale for display
      *
-     * This should probably be moved to a static utility class - ern
-     *
      * @param grade
      * @param typeOfGrade
      * @param scaleFactor

diff --git a/...t/tool/src/java/org/sakaiproject/assignment/entityproviders/AssignmentEntityProvider.java b/...t/tool/src/java/org/sakaiproject/assignment/entityproviders/AssignmentEntityProvider.java
@@ -1187,7 +1187,6 @@ public ActionReturn setGrade(Map<String, Object> params) {
         options.put(GRADE_SUBMISSION_FEEDBACK_TEXT, feedbackText);
         options.put(GRADE_SUBMISSION_FEEDBACK_COMMENT, feedbackComment);
         options.put(GRADE_SUBMISSION_PRIVATE_NOTES, privateNotes);
-        options.put(WITH_GRADES, true);
         options.put(ALLOW_RESUBMIT_NUMBER, resubmitNumber);
 
         if (StringUtils.isNotBlank(resubmitDate)) {

diff --git a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java
diff --git a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentToolUtils.java b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentToolUtils.java
@@ -38,6 +38,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.apache.commons.lang3.BooleanUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.sakaiproject.assignment.api.AssignmentReferenceReckoner;
 import org.sakaiproject.assignment.api.AssignmentService;
@@ -275,63 +276,81 @@ public List<String> alertInvalidPoint(String grade, int factor) {
     public void gradeSubmission(AssignmentSubmission submission, String gradeOption, Map<String, Object> options, List<String> alerts) {
 
         if (submission != null) {
-            boolean withGrade = options.get(WITH_GRADES) != null && (Boolean) options.get(WITH_GRADES);
-            String grade = (String) options.get(GRADE_SUBMISSION_GRADE);
-            boolean gradeChanged = false;
-            if (!StringUtils.equals(StringUtils.trimToNull(submission.getGrade()), StringUtils.trimToNull(grade))) {
-                //one is null the other isn't
-                gradeChanged = true;
-            }
             Assignment a = submission.getAssignment();
-            if (!withGrade) {
-                // no grade input needed for the without-grade version of assignment tool
+            String grade = assignmentService.getGradeDisplay((String) options.get(GRADE_SUBMISSION_GRADE), a.getTypeOfGrade(), a.getScaleFactor());
+
+            boolean gradeChanged = !StringUtils.equals(StringUtils.trimToNull(submission.getGrade()), StringUtils.trimToNull(grade));
+
+            // the instructor feedback comment
+            String submittedfeedbackComment = StringUtils.trimToNull((String) options.get(GRADE_SUBMISSION_FEEDBACK_COMMENT));
+            if (submittedfeedbackComment != null) {
+                submission.setFeedbackComment(submittedfeedbackComment);
+            } else {
+                submission.setFeedbackComment(null);
+            }
+
+            // the instructor inline feedback
+            String submittedfeedbackText = StringUtils.trimToNull((String) options.get(GRADE_SUBMISSION_FEEDBACK_TEXT));
+            if (submittedfeedbackText != null) {
+                submission.setFeedbackText(submittedfeedbackText);
+            } else {
+                submission.setFeedbackText(null);
+            }
+
+            List<Reference> submittedfeedbackAttachments = (List<Reference>) options.get(GRADE_SUBMISSION_FEEDBACK_ATTACHMENT);
+            if (submittedfeedbackAttachments != null) {
+                // clear the old attachments first
+                Set<String> feedbackAttachments = submission.getFeedbackAttachments();
+                if (!BooleanUtils.toBoolean((Boolean) options.get(GRADE_SUBMISSION_DONT_CLEAR_CURRENT_ATTACHMENTS))) {
+                    feedbackAttachments.clear();
+                }
+                for (Reference attachment : submittedfeedbackAttachments) {
+                    feedbackAttachments.add(attachment.getReference());
+                }
+            }
+
+            String submittedInstructorPrivateNotes = StringUtils.trimToNull((String) options.get(GRADE_SUBMISSION_PRIVATE_NOTES));
+            if (submittedInstructorPrivateNotes != null) {
+                submission.setPrivateNotes(submittedInstructorPrivateNotes);
+            }
+
+            // determine if the submission is graded
+            if (StringUtils.isNotBlank(grade)) {
+                // if there is a grade then the submission is graded
                 submission.setGraded(true);
+                submission.setGrade(grade);
+
                 if (gradeChanged) {
                     submission.setGradedBy(userDirectoryService.getCurrentUser() == null ? null : userDirectoryService.getCurrentUser().getId());
                 }
-                if (SUBMISSION_OPTION_RETURN.equals(gradeOption) || SUBMISSION_OPTION_RELEASE.equals(gradeOption)) {
-                    submission.setGradeReleased(true);
-                }
+            } else if (submittedfeedbackComment != null) {
+                // if there is feedback by the instructor regardless of an entered grade
+                submission.setGraded(true);
             } else {
-                //If the grade is not blank
-                if (StringUtils.isNotBlank(grade)) {
-                    submission.setGrade(grade);
-                    submission.setGraded(true);
-                    if (gradeChanged) {
-                        submission.setGradedBy(userDirectoryService.getCurrentUser() == null ? null : userDirectoryService.getCurrentUser().getId());
-                    }
-                } else {
-                    submission.setGrade(null);
-                    submission.setGraded(false);
-                    if (gradeChanged) {
-                        submission.setGradedBy(null);
-                    }
+                // if no grade or feedback left then it is not graded
+                submission.setGrade(null);
+                submission.setGraded(false);
+                if (gradeChanged) {
+                    submission.setGradedBy(null);
                 }
             }
 
-            // iterate through submitters and look for grade overrides...
-            if (withGrade && a.getIsGroup()) {
+            if (a.getIsGroup()) {
+                // group project only set a grade override for submitters
                 for (AssignmentSubmissionSubmitter submitter : submission.getSubmitters()) {
-                    String g = (String) options.get(GRADE_SUBMISSION_GRADE + "_" + submitter.getSubmitter());
-                    if (g != submitter.getGrade()) submitter.setGrade(g);
+                    String submitterGradeOverride = StringUtils.trimToNull((String) options.get(GRADE_SUBMISSION_GRADE + "_" + submitter.getSubmitter()));
+                    if (!StringUtils.equals(submitterGradeOverride, submitter.getGrade())) {
+                        submitter.setGrade(submitterGradeOverride);
+                    }
                 }
             }
 
             if (SUBMISSION_OPTION_RELEASE.equals(gradeOption)) {
                 submission.setGradeReleased(true);
-                submission.setGraded(true);
-                if (gradeChanged) {
-                    submission.setGradedBy(userDirectoryService.getCurrentUser() == null ? null : userDirectoryService.getCurrentUser().getId());
-                }
-                // clear the returned flag
                 submission.setReturned(false);
                 submission.setDateReturned(null);
             } else if (SUBMISSION_OPTION_RETURN.equals(gradeOption)) {
                 submission.setGradeReleased(true);
-                submission.setGraded(true);
-                if (gradeChanged) {
-                    submission.setGradedBy(userDirectoryService.getCurrentUser() == null ? null : userDirectoryService.getCurrentUser().getId());
-                }
                 submission.setReturned(true);
                 submission.setDateReturned(Instant.now());
             } else if (SUBMISSION_OPTION_RETRACT.equals(gradeOption)) {
@@ -369,41 +388,6 @@ public void gradeSubmission(AssignmentSubmission submission, String gradeOption,
                 properties.remove(ALLOW_EXTENSION_CLOSETIME);
             }
 
-            // the instructor comment
-            String feedbackCommentString = StringUtils.trimToNull((String) options.get(GRADE_SUBMISSION_FEEDBACK_COMMENT));
-            if (feedbackCommentString != null) {
-                submission.setFeedbackComment(feedbackCommentString);
-            } else {
-                submission.setFeedbackComment("");
-            }
-
-            // the instructor inline feedback
-            String feedbackTextString = (String) options.get(GRADE_SUBMISSION_FEEDBACK_TEXT);
-            if (feedbackTextString != null) {
-                submission.setFeedbackText(feedbackTextString);
-            }
-
-            List<Reference> v = (List<Reference>) options.get(GRADE_SUBMISSION_FEEDBACK_ATTACHMENT);
-            if (v != null) {
-
-                // clear the old attachments first
-                Set<String> feedbackAttachments = submission.getFeedbackAttachments();
-
-                boolean clear = !(options.get(GRADE_SUBMISSION_DONT_CLEAR_CURRENT_ATTACHMENTS) != null
-                    && (Boolean) options.get(GRADE_SUBMISSION_DONT_CLEAR_CURRENT_ATTACHMENTS));
-                if (clear) {
-                    feedbackAttachments.clear();
-                }
-
-                for (Reference aV : v) {
-                    feedbackAttachments.add(aV.getReference());
-                }
-            }
-
-            if (options.get(GRADE_SUBMISSION_PRIVATE_NOTES) != null) {
-                submission.setPrivateNotes((String) options.get(GRADE_SUBMISSION_PRIVATE_NOTES));
-            }
-
             String sReference = AssignmentReferenceReckoner.reckoner().submission(submission).reckon().getReference();
 
             // save a timestamp for this grading process

diff --git a/assignment/tool/src/webapp/WEB-INF/tools/sakai.assignment.grades.xml b/assignment/tool/src/webapp/WEB-INF/tools/sakai.assignment.grades.xml
@@ -12,7 +12,6 @@
 		<category name="project" />
 
 		<configuration name="pagesize" value="200" />
-		<configuration name="withGrades" value="TRUE" type="final" />
 		<configuration name="help.id" value="sakai.assignment" type="final" />
 		<configuration name="showNumSubmissionColumn" value="TRUE" />
 

diff --git a/assignment/tool/src/webapp/vm/assignment/assignment_macros.vm b/assignment/tool/src/webapp/vm/assignment/assignment_macros.vm
@@ -92,7 +92,7 @@
 #end
 
 ## Macro for the top nav bar HREF and #toolLink/#toolLinkParam
-#macro( navBarHREF $allowAddAssignment $withGrade $allowGradeSubmission $allowAddAssignment $allowRecoverAssignment $allowAllGroups $assignmentscheck $allowUpdateSite $enableViewOption $view $current )
+#macro( navBarHREF $allowAddAssignment $allowGradeSubmission $allowAddAssignment $allowRecoverAssignment $allowAllGroups $assignmentscheck $allowUpdateSite $enableViewOption $view $current )
     <ul class="navIntraTool actionToolBar">
         #set( $prevAction = false )
         <li #if( $prevAction == false ) class="firstToolBarItem" #set( $prevAction = true ) #end>
@@ -122,7 +122,7 @@
             #end
         </li>
 
-        #if( $withGrade && $!allowGradeSubmission )
+        #if($!allowGradeSubmission )
             <li #if( $prevAction == false ) class="firstToolBarItem" #set( $prevAction = true ) #end>
                 #if( !$!view.equals( "grarep" ) )
                     <span>
@@ -225,7 +225,7 @@
 #end
 
 ## Macro for the top nav bar using onclick JavaScript
-#macro( navBarOnClick $allowAddAssignment $withGrade $allowGradeSubmission $allowAddAssignment $allowRecoverAssignment $allowAllGroups $assignmentscheck $allowUpdateSite $enableViewOption $view $formID $current )
+#macro( navBarOnClick $allowAddAssignment $allowGradeSubmission $allowAddAssignment $allowRecoverAssignment $allowAllGroups $assignmentscheck $allowUpdateSite $enableViewOption $view $formID $current )
     <ul class="navIntraTool actionToolBar">
         #set( $prevAction = false )
         #if( !$!view.equals( "lisofass1" ) )
@@ -277,7 +277,7 @@
                 <span class="disabled">$!tlang.getString( "lisofass2" )</span>
             </li>
         #end
-        #if( $withGrade && $!allowGradeSubmission )
+        #if( $!allowGradeSubmission )
             <li #if( $prevAction == false ) class="firstToolBarItem" #set( $prevAction = true ) #end>
                 #if( $current == "gradeReport" )
                     <span class="current">$!tlang.getString( "gen.grarep" )</span>