SAK-33108: Provide visual indicators for dropped scores in categories

Author: plukasew

edu-services/gradebook-service/api/src/java/org/sakaiproject/service/gradebook/shared/CategoryScoreData.java

@@ -0,0 +1,20 @@
+package org.sakaiproject.service.gradebook.shared;
+
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Immutable category score and list of gradebook items dropped (highest/lowest) from the score calculation
+ * @author plukasew
+ */
+public final class CategoryScoreData
+{
+	public final double score;
+	public final List<Long> droppedItems;
+
+	public CategoryScoreData(double score, List<Long> droppedItems)
+	{
+		this.score = score;
+		this.droppedItems = Collections.unmodifiableList(droppedItems);
+	}
+}

edu-services/gradebook-service/api/src/java/org/sakaiproject/service/gradebook/shared/GradebookService.java

@@ -27,6 +27,7 @@
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 
 import org.apache.commons.lang3.StringUtils;
@@ -804,10 +805,10 @@ public void finalizeGrades(String gradebookUid)
 	 * @param gradebookId Id of the gradebook
 	 * @param studentUuid uuid of the student
 	 * @param categoryId id of category
-	 * @return percentage or null if no calculations were made
+	 * @return percentage and dropped items, or empty if no calculations were made
 	 *
 	 */
-	Double calculateCategoryScore(Long gradebookId, String studentUuid, Long categoryId);
+	Optional<CategoryScoreData> calculateCategoryScore(Long gradebookId, String studentUuid, Long categoryId);
 
 	/**
 	 * Calculate the category score for the given gradebook, category, assignments in the category and grade map. This doesn't do any
@@ -816,12 +817,12 @@ public void finalizeGrades(String gradebookUid)
 	 * @param gradebook the gradebook. As this method is called for every student at once, this is passed in to save additional lookups by
 	 *            id.
 	 * @param studentUuid uuid of the student
-	 * @param categoryId id of category
+	 * @param category the category
 	 * @param categoryAssignments list of assignments the student can view, and are in the category
 	 * @param gradeMap map of assignmentId to grade, to use for the calculations
-	 * @return percentage or null if no calculations were made
+	 * @return percentage and dropped items, or empty if no calculations were made
 	 */
-	Double calculateCategoryScore(Object gradebook, String studentUuid, CategoryDefinition category,
+	Optional<CategoryScoreData> calculateCategoryScore(Object gradebook, String studentUuid, CategoryDefinition category,
 			final List<Assignment> categoryAssignments, Map<Long, String> gradeMap);
 
 	/**

edu-services/gradebook-service/impl/src/java/org/sakaiproject/component/gradebook/GradebookServiceHibernateImpl.java

@@ -32,6 +32,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -58,6 +59,7 @@
 import org.sakaiproject.service.gradebook.shared.AssessmentNotFoundException;
 import org.sakaiproject.service.gradebook.shared.AssignmentHasIllegalPointsException;
 import org.sakaiproject.service.gradebook.shared.CategoryDefinition;
+import org.sakaiproject.service.gradebook.shared.CategoryScoreData;
 import org.sakaiproject.service.gradebook.shared.CommentDefinition;
 import org.sakaiproject.service.gradebook.shared.ConflictingAssignmentNameException;
 import org.sakaiproject.service.gradebook.shared.ConflictingCategoryNameException;
@@ -3132,7 +3134,7 @@ public List<GradingEvent> getGradingEvents(final String studentId, final long as
     }
 
 	@Override
-	public Double calculateCategoryScore(final Object gradebook, final String studentUuid, final CategoryDefinition category, final List<org.sakaiproject.service.gradebook.shared.Assignment> categoryAssignments, final Map<Long,String> gradeMap) {
+	public Optional<CategoryScoreData> calculateCategoryScore(final Object gradebook, final String studentUuid, final CategoryDefinition category, final List<org.sakaiproject.service.gradebook.shared.Assignment> categoryAssignments, final Map<Long,String> gradeMap) {
 
 		final Gradebook gb = (Gradebook) gradebook;
 
@@ -3176,6 +3178,7 @@ public Double calculateCategoryScore(final Object gradebook, final String studen
 			a.setRemoved(false); //shared.GradebookAssignment doesn't include removed so this will always be false
 			a.setGradebook(gb);
 			a.setCategory(c);
+			a.setId(assignment.getId());  // store the id so we can find out later which grades were dropped, if any
 
 			//create the AGR
 			final AssignmentGradeRecord gradeRecord = new AssignmentGradeRecord(a, studentUuid, grade);
@@ -3187,7 +3190,7 @@ public Double calculateCategoryScore(final Object gradebook, final String studen
 	}
 
 	@Override
-	public Double calculateCategoryScore(final Long gradebookId, final String studentUuid, final Long categoryId) {
+	public Optional<CategoryScoreData> calculateCategoryScore(final Long gradebookId, final String studentUuid, final Long categoryId) {
 
 		//get all grade records for the student
 		@SuppressWarnings({ "unchecked", "rawtypes"})
@@ -3208,22 +3211,22 @@ public Object doInHibernate(final Session session) throws HibernateException {
 	/**
 	 * Does the heavy lifting for the category calculations.
 	 * Requires the List of AssignmentGradeRecord so that we can applyDropScores.
-	 * @param studentUuid the studnet uuid
-	 * @param categoryId the cateogry id we are interested in
+	 * @param studentUuid the student uuid
+	 * @param categoryId the category id we are interested in
 	 * @param gradeRecords all grade records for the student
 	 * @return
 	 */
-	private Double calculateCategoryScore(final String studentUuid, final Long categoryId, final List<AssignmentGradeRecord> gradeRecords) {
+	private Optional<CategoryScoreData> calculateCategoryScore(final String studentUuid, final Long categoryId, final List<AssignmentGradeRecord> gradeRecords) {
 
 		//validate
 		if(gradeRecords == null) {
 			log.debug("No grade records for student: {}. Nothing to do.", studentUuid);
-			return null;
+			return Optional.empty();
 		}
 
 		if (categoryId == null) {
 			log.debug("No category supplied, nothing to do.");
-			return null;
+			return Optional.empty();
 		}
 
 		//setup
@@ -3235,6 +3238,13 @@ private Double calculateCategoryScore(final String studentUuid, final Long categ
 		// apply any drop/keep settings for this category
 		applyDropScores(gradeRecords);
 
+		// find the records marked as dropped (highest/lowest) before continuing,
+		// as gradeRecords will be modified in place after this and these records will be removed
+		List<Long> droppedItemIds = gradeRecords.stream()
+				.filter(AssignmentGradeRecord::getDroppedFromGrade)
+				.map(agr -> agr.getAssignment().getId())
+				.collect(Collectors.toList());
+
 		// Since all gradeRecords for the student are passed in, not just for this category,
 		// plus they may not meet the criteria for including in the calculation,
 		// this list is filtered down according to the following rules:
@@ -3271,7 +3281,7 @@ private Double calculateCategoryScore(final String studentUuid, final Long categ
 		// pre-calculation
 		// Rule 1. If category only has a single EC item, don't try to calculate category total.
 		if(gradeRecords.size() == 1 && gradeRecords.get(0).getAssignment().isExtraCredit()) {
-			return null;
+			return Optional.empty();
 		}
 
 		//iterate the filtered list and set the variables for the calculation
@@ -3295,11 +3305,11 @@ private Double calculateCategoryScore(final String studentUuid, final Long categ
 		}
 
 		if (numScored == 0 || numOfAssignments == 0 || totalPossible.doubleValue() == 0) {
-    		return null;
+    		return Optional.empty();
     	}
 
     	final BigDecimal mean = totalEarned.divide(new BigDecimal(numScored), GradebookService.MATH_CONTEXT).divide((totalPossible.divide(new BigDecimal(numOfAssignments), GradebookService.MATH_CONTEXT)), GradebookService.MATH_CONTEXT).multiply(new BigDecimal("100"));
-    	return mean.doubleValue();
+    	return Optional.of(new CategoryScoreData(mean.doubleValue(), droppedItemIds));
 	}
 
 	@Override

gradebookng/tool/src/java/org/sakaiproject/gradebookng/GradebookNgApplication.properties

@@ -29,6 +29,11 @@ label.noduedate=-
 label.percentage.valued={0}%
 label.percentage.plain=%
 
+label.category.drophighest=Drop Highest: {0}
+label.category.droplowest=Drop Lowest: {0}
+label.category.keephighest=Keep Highest: {0}
+label.category.dropSeparator=&
+
 label.coursegrade.released=Course grade has been released to students.
 label.coursegrade.notreleased=Course grade has not been released to students.
 label.coursegrade.nopermission=-

gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java

@@ -42,6 +42,8 @@
 import org.apache.commons.lang.StringUtils;
 
 import org.sakaiproject.authz.api.Member;
+import org.sakaiproject.authz.api.SecurityAdvisor;
+import org.sakaiproject.authz.api.SecurityAdvisor.SecurityAdvice;
 import org.sakaiproject.authz.api.SecurityService;
 import org.sakaiproject.component.cover.ComponentManager;
 import org.sakaiproject.coursemanagement.api.Membership;
@@ -67,6 +69,7 @@
 import org.sakaiproject.service.gradebook.shared.AssessmentNotFoundException;
 import org.sakaiproject.service.gradebook.shared.Assignment;
 import org.sakaiproject.service.gradebook.shared.CategoryDefinition;
+import org.sakaiproject.service.gradebook.shared.CategoryScoreData;
 import org.sakaiproject.service.gradebook.shared.CommentDefinition;
 import org.sakaiproject.service.gradebook.shared.CourseGrade;
 import org.sakaiproject.service.gradebook.shared.GradeDefinition;
@@ -515,6 +518,38 @@ public List<CategoryDefinition> getGradebookCategories(final String siteId) {
 		return rval;
 	}
 
+	/**
+	* Retrieve the categories visible to the given student.
+	*
+	* This should only be called if you are wanting to view the categories that a student would see (ie if you ARE a student, or if you
+	* are an instructor using the student review mode)
+	*
+	* @param studentUuid
+	* @return list of visible categories
+	*/
+	public List<CategoryDefinition> getGradebookCategoriesForStudent(String studentUuid) {
+		// find the categories that this student's visible assignments belong to
+		List<Assignment> viewableAssignments = getGradebookAssignmentsForStudent(studentUuid);
+		final List<Long> catIds = new ArrayList<>();
+		for (Assignment a : viewableAssignments) {
+			Long catId = a.getCategoryId();
+			if (catId != null && !catIds.contains(catId)) {
+				catIds.add(a.getCategoryId());
+			}
+		}
+
+		// get all the categories in the gradebook, use a security advisor in case the current user is the student
+		SecurityAdvisor gbAdvisor = (String userId, String function, String reference)
+						-> "gradebook.gradeAll".equals(function) ? SecurityAdvice.ALLOWED : SecurityAdvice.PASS;
+		securityService.pushAdvisor(gbAdvisor);
+		List<CategoryDefinition> catDefs = gradebookService.getCategoryDefinitions(getGradebook().getUid());
+		securityService.popAdvisor(gbAdvisor);
+
+		// filter out the categories that don't match the categories of the viewable assignments
+		return catDefs.stream().filter(def -> catIds.contains(def.getId())).collect(Collectors.toList());
+
+	}
+
 	/**
 	 * Get a map of course grades for all students in the given site.
 	 *
@@ -1110,10 +1145,17 @@ public void putAssignmentsAndCategoryItemsInMatrix(Map<String, GbStudentGradeInf
 						}
 					}
 
-					final Double categoryScore = this.gradebookService.calculateCategoryScore(gradebook, student.getUserUuid(), category, category.getAssignmentList(), gradeMap);
-
-					// add to GbStudentGradeInfo
-					sg.addCategoryAverage(category.getId(), categoryScore);
+					final Optional<CategoryScoreData> categoryScore = gradebookService.calculateCategoryScore(gradebook,
+							student.getUserUuid(), category, category.getAssignmentList(), gradeMap);
+					categoryScore.ifPresent(data -> {
+						for (Long item : gradeMap.keySet())	{
+							if (data.droppedItems.contains(item)) {
+								grades.get(item).setDroppedFromCategoryScore(true);
+							}
+						}
+						// add to GbStudentGradeInfo
+						sg.addCategoryAverage(category.getId(), data.score);
+					});
 
 					// TODO the TA permission check could reuse this iteration... check performance.
 				}
@@ -2094,14 +2136,14 @@ public Map<Long, GbGradeInfo> getGradesForStudent(final String studentUuid) {
 	 * @param studentUuid uuid of student
 	 * @return
 	 */
-	public Double getCategoryScoreForStudent(final Long categoryId, final String studentUuid) {
+	public Optional<CategoryScoreData> getCategoryScoreForStudent(final Long categoryId, final String studentUuid) {
 
 		final Gradebook gradebook = getGradebook();
 
-		final Double score = this.gradebookService.calculateCategoryScore(gradebook.getId(), studentUuid, categoryId);
-		log.info("Category score for category: {}, student: {}:{}", categoryId, studentUuid, score);
+		final Optional<CategoryScoreData> result = gradebookService.calculateCategoryScore(gradebook.getId(), studentUuid, categoryId);
+		log.info("Category score for category: {}, student: {}:{}", categoryId, studentUuid, result.map(r -> r.score).orElse(null));
 
-		return score;
+		return result;
 	}
 
 	/**

gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/model/GbGradeInfo.java

@@ -47,6 +47,13 @@ public class GbGradeInfo implements Serializable, Comparable<GbGradeInfo> {
 	@Setter
 	private boolean gradeable;
 
+	/**
+	* Whether this grade has been dropped from the category score calculation
+	*/
+	@Getter
+	@Setter
+	private boolean droppedFromCategoryScore = false;
+
 	/**
 	 * Constructor. Takes a GradeDefinition or null. If null, a stub is created.
 	 *

gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/util/FormatHelper.java

@@ -24,14 +24,18 @@
 import java.text.NumberFormat;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
+import java.util.List;
 import java.util.Locale;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.validator.routines.DoubleValidator;
 import org.sakaiproject.util.ResourceLoader;
 
 import lombok.extern.slf4j.Slf4j;
+import org.sakaiproject.service.gradebook.shared.CategoryDefinition;
 
 @Slf4j
 public class FormatHelper {
@@ -322,4 +326,37 @@ public static String decode(final String s) {
 			throw new AssertionError("UTF-8 not supported");
 		}
 	}
+
+	/**
+	 * Returns a list of drop highest/lowest labels based on the settings of the given category.
+	 * @param category the category
+	 * @return a list of 1 or 2 labels indicating that drop highest/lowest is in use, or an empty list if not in use.
+	 */
+	public static List<String> formatCategoryDropInfo(CategoryDefinition category) {
+
+		if (category == null) {
+			return Collections.emptyList();
+		}
+
+		int dropHighest = category.getDropHighest() == null ? 0 : category.getDropHighest();
+		int dropLowest = category.getDropLowest() == null ? 0 : category.getDropLowest();
+		int keepHighest = category.getKeepHighest() == null ? 0 : category.getKeepHighest();
+
+		if (dropHighest == 0 && dropLowest == 0 && keepHighest == 0) {
+			return Collections.emptyList();
+		}
+
+		List<String> info = new ArrayList<>(2);
+		if (dropHighest > 0) {
+			info.add(MessageHelper.getString("label.category.drophighest", dropHighest));
+		}
+		if (dropLowest > 0) {
+			info.add(MessageHelper.getString("label.category.droplowest", dropLowest));
+		}
+		if (keepHighest > 0) {
+			info.add(MessageHelper.getString("label.category.keephighest", keepHighest));
+		}
+
+		return info;
+	}
 }