SAK-41058: assignments > can circumvent gradebook category with drop/…

…keep uniform point value validation
susowrc · Dec 18, 2018 · adc13f1 · adc13f1
1 parent 6707e97
commit adc13f1
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 2 deletions.
diff --git a/assignment/bundles/resources/assignment.properties b/assignment/bundles/resources/assignment.properties
@@ -369,6 +369,8 @@ plesuse3 = Please use a positive number or zero for the grade field.
 
 plesuse4 = {0} is greater than the maximum value allowed by the system. Please enter a value no greater than {1}.
 
+pleasee6 = Points must match assignments in the selected Gradebook category ({0})
+
 points = Points
 
 prevassig.hidass     = Hide assignment

diff --git a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java
@@ -7006,6 +7006,7 @@ private void setNewAssignmentParameters(RunData data, boolean validify) {
         // only when choose to associate with assignment in Gradebook
         String associateAssignment = params.getString(PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT);
 
+        Double droppedCategoryPoints = -1D;
         if (grading != null) {
             if (grading.equals(GRADEBOOK_INTEGRATION_ASSOCIATE)) {
                 state.setAttribute(PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT, associateAssignment);
@@ -7022,6 +7023,33 @@ private void setNewAssignmentParameters(RunData data, boolean validify) {
                 // if chosen as "associate", have to choose one assignment from Gradebook
                 if (grading.equals(GRADEBOOK_INTEGRATION_ASSOCIATE) && StringUtils.trimToNull(associateAssignment) == null) {
                     addAlert(state, rb.getString("grading.associate.alert"));
+                } else {
+                    Long thisCatRef = -1L;
+                    String gradebookUid = toolManager.getCurrentPlacement().getContext();
+                    List<CategoryDefinition> categoryDefinitions = gradebookService.getCategoryDefinitions(gradebookUid);
+                    if (catInt != -1) {
+                      thisCatRef = catInt;
+                    } else if (assignmentRef.isEmpty()) {
+                      thisCatRef = catInt;
+                    } else {
+                        for (CategoryDefinition thisCategoryDefinition : categoryDefinitions) {
+                            if (thisCategoryDefinition.isAssignmentInThisCategory(assignmentRef)) {
+                                thisCatRef = thisCategoryDefinition.getId();
+                            }
+                        }
+                    }
+                    if (thisCatRef != -1) {
+                        for(CategoryDefinition thisCategoryDefinition : categoryDefinitions) {
+                            if (Objects.equals(thisCategoryDefinition.getId(), thisCatRef)) {
+                                if (thisCategoryDefinition.getIsDropped()) {
+                                    Double thisCategoryPoints = thisCategoryDefinition.getPointsForCategory();
+                                    if (thisCategoryPoints != null) {
+                                        droppedCategoryPoints = thisCategoryPoints;
+                                    }
+                                }
+                            }
+                        }
+                    }
                 }
 
                 // check if chosen a previously associated object
@@ -7165,6 +7193,13 @@ private void setNewAssignmentParameters(RunData data, boolean validify) {
                     // when scale is points, grade must be integer and less than maximum value
                     gradePoints = scalePointGrade(state, gradePoints, scaleFactor);
                     state.setAttribute(NEW_ASSIGNMENT_GRADE_POINTS, gradePoints);
+                    if (droppedCategoryPoints != -1) {
+                        int factor = assignmentService.getScaleFactor();
+                        Double enteredPoints = new Double(displayGrade(state, gradePoints, factor));
+                        if (enteredPoints != droppedCategoryPoints) {
+                          addAlert(state, rb.getFormattedMessage("pleasee6", new Object[] {droppedCategoryPoints.toString()}));
+                        }
+                    }
                 }
             }
         }
@@ -12479,7 +12514,8 @@ private void alertInvalidPoint(SessionState state, String grade, int factor) {
     }
 
     private String displayGrade(SessionState state, String grade, Integer factor) {
-        if (state.getAttribute(STATE_MESSAGE) == null) {
+        String currentStateMessage = (String) state.getAttribute(STATE_MESSAGE);
+        if (currentStateMessage == null || currentStateMessage.startsWith(rb.getString("pleasee6"))) {
             if (StringUtils.isNotBlank(grade)) {
                 grade = assignmentService.getGradeDisplay(grade, Assignment.GradeType.SCORE_GRADE_TYPE, factor);
             } else {

diff --git a/...ok-service/api/src/java/org/sakaiproject/service/gradebook/shared/CategoryDefinition.java b/...ok-service/api/src/java/org/sakaiproject/service/gradebook/shared/CategoryDefinition.java
@@ -41,6 +41,7 @@ public class CategoryDefinition implements Serializable {
     private Integer keepHighest;
     private Boolean extraCredit;
     private Integer categoryOrder;
+    private Boolean isDropped;
 
     public static Comparator<CategoryDefinition> orderComparator;
 
@@ -205,4 +206,37 @@ public Double getTotalPoints() {
 		}
 		return totalPoints.doubleValue();
 	}
+
+	public Boolean getIsDropped() {
+		return isDropped;
+	}
+
+	public void setIsDropped(Boolean isDropped) {
+		this.isDropped = isDropped;
+	}
+
+	public boolean isAssignmentInThisCategory(String assignmentId) {
+		for (Assignment thisAssignment : this.assignmentList) {
+			if (thisAssignment.getExternalId() == null) {
+				continue;
+			}
+			if (thisAssignment.getExternalId().equalsIgnoreCase(assignmentId)) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
+	public Double getPointsForCategory() {
+		if (!this.isDropped) {
+			return null;
+		}
+
+		for (Assignment thisAssignment : this.assignmentList) {
+			return thisAssignment.getPoints();
+		}
+
+		return null;
+	}
 }
diff --git a/...src/java/org/sakaiproject/component/gradebook/GradebookExternalAssessmentServiceImpl.java b/...src/java/org/sakaiproject/component/gradebook/GradebookExternalAssessmentServiceImpl.java
@@ -26,6 +26,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -716,6 +717,16 @@ public synchronized void addExternalAssessment(final String gradebookUid, final
 			Category persistedCategory = null;
 			if (categoryId != null) {
 				persistedCategory = getCategory(categoryId);
+				if (persistedCategory.isDropScores()) {
+					List<GradebookAssignment> thisCategoryAssignments = getAssignmentsForCategory(categoryId);
+					for (GradebookAssignment thisAssignment : thisCategoryAssignments) {
+						if (!Objects.equals(thisAssignment.getPointsPossible(), points)) {
+							String errorMessage = "Assignment points mismatch the selected Gradebook Category ("
+								+ thisAssignment.getPointsPossible().toString() + ") and cannot be added to Gradebook )";
+							throw new InvalidCategoryException(errorMessage);
+						}
+					}
+				}
 				if (persistedCategory == null || persistedCategory.isRemoved() ||
 						!persistedCategory.getGradebook().getId().equals(gradebook.getId())) {
 					throw new InvalidCategoryException("The category with id " + categoryId +

diff --git a/...ice/impl/src/java/org/sakaiproject/component/gradebook/GradebookServiceHibernateImpl.java b/...ice/impl/src/java/org/sakaiproject/component/gradebook/GradebookServiceHibernateImpl.java
@@ -49,7 +49,6 @@
 import org.sakaiproject.authz.cover.SecurityService;
 import org.sakaiproject.component.cover.ServerConfigurationService;
 import org.sakaiproject.hibernate.HibernateCriterionUtils;
-import org.sakaiproject.rubrics.logic.RubricsConstants;
 import org.sakaiproject.rubrics.logic.RubricsService;
 import org.sakaiproject.section.api.coursemanagement.CourseSection;
 import org.sakaiproject.section.api.coursemanagement.EnrollmentRecord;
@@ -2503,6 +2502,7 @@ private CategoryDefinition getCategoryDefinition(final Category category) {
 			categoryDef.setDropHighest(category.getDropHighest());
 			categoryDef.setKeepHighest(category.getKeepHighest());
 			categoryDef.setAssignmentList(getAssignments(category.getGradebook().getUid(), category.getName()));
+			categoryDef.setIsDropped(category.isDropScores());
 			categoryDef.setExtraCredit(category.isExtraCredit());
 			categoryDef.setCategoryOrder(category.getCategoryOrder());
 		}