1389: From review, move the category comparator from the tool to the …

…service and create a new SortType so the sorting is only performed once by the service (and doesn't need to be resort by the tool). Also change the Categories button to refresh the page to ensure the sort order is updated from the service.

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

@@ -412,6 +412,21 @@ public boolean isAssignmentDefined(String gradebookUid, String assignmentTitle)
 	 */
 	public List<Assignment> getViewableAssignmentsForCurrentUser(String gradebookUid);
 
+	/**
+	 *
+	 * @param gradebookUid
+	 * @return list of gb items that the current user is authorized to view
+	 * sorted by the provided SortType.
+	 * If user has gradeAll permission, returns all gb items.
+	 * If user has gradeSection perm with no grader permissions,
+	 * returns all gb items.
+	 * If user has gradeSection with grader perms, returns only the items that
+	 * the current user is authorized to view or grade.
+	 * If user does not have grading privileges but does have viewOwnGrades perm,
+	 * will return all released gb items.
+	 */
+	public List<Assignment> getViewableAssignmentsForCurrentUser(String gradebookUid, SortType sortBy);
+
 	/**
 	 * 
 	 * @param gradebookUid

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

@@ -16,5 +16,6 @@ public enum SortType {
 	SORT_BY_RELEASED,
 	SORT_BY_COUNTED,
 	SORT_BY_EDITOR,
+	SORT_BY_CATEGORY,
 	SORT_BY_SORTING; //default
 }

edu-services/gradebook-service/hibernate/src/java/org/sakaiproject/tool/gradebook/Assignment.java

@@ -64,6 +64,7 @@ public class Assignment extends GradableObject {
     public static Comparator releasedComparator;
     public static Comparator countedComparator;
     public static Comparator gradeEditorComparator;
+    public static Comparator categoryComparator;
 
     private Double pointsPossible;
     private Date dueDate;
@@ -230,6 +231,63 @@ public String toString() {
                 return "Assignment.gradeEditorComparator";
             }
         };
+
+        categoryComparator = new Comparator() {
+            public int compare(Object o1, Object o2) {
+                if(log.isDebugEnabled()) log.debug("Comparing assignment + " + o1 + " to " + o2 + " by category ordering");
+                Assignment one = (Assignment)o1;
+                Assignment two = (Assignment)o2;
+
+                // if categories are null
+                if (one.getCategory() == null && two.getCategory() == null) {
+                    // sort by assignment sort order
+                    return one.getSortOrder().compareTo(two.getSortOrder());
+                } else if (one.getCategory() == null) {
+                    return 1;
+                } else if (two.getCategory() == null) {
+                    return -1;
+                }
+
+                // if in the same category, sort by their categorized sort order
+                if (one.getCategory().equals(two.getCategory())) {
+                    // handles null orders by putting them at the end of the list
+                    if (one.getCategorizedSortOrder() == null) {
+                        return 1;
+                    } else if (two.getCategorizedSortOrder() == null) {
+                        return -1;
+                    }
+                    return Integer.compare(one.getCategorizedSortOrder(), two.getCategorizedSortOrder());
+
+                // otherwise, sort by their category order
+                } else {
+                    // check if category has a order (not required)
+                    if (one.getCategory().getCategoryOrder() == null && two.getCategory().getCategoryOrder() == null) {
+                        // both orders are null.. so order by A-Z
+                        if (one.getCategory().getName() == null && two.getCategory().getName() == null) {
+                            // both names are null so order by id
+                            return one.getCategory().getId().compareTo(two.getCategory().getId());
+                        } else if (one.getCategory().getName() == null) {
+                            return 1;
+                        } else if (two.getCategory().getName() == null) {
+                            return -1;
+                        } else {
+                            return one.getCategory().getName().compareTo(two.getCategory().getName());
+                        }
+                    } else if (one.getCategory().getCategoryOrder() == null) {
+                        return 1;
+                    } else if (two.getCategory().getCategoryOrder() == null) {
+                        return -1;
+                    } else {
+                        return one.getCategory().getCategoryOrder().compareTo(two.getCategory().getCategoryOrder());
+                    }
+                }
+            }
+
+            @Override
+            public String toString() {
+                return "Assignment.categoryComparator";
+            }
+        };
     }
 
     public Assignment(Gradebook gradebook, String name, Double pointsPossible, Date dueDate) {

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

@@ -38,8 +38,7 @@
 import java.util.Map.Entry;
 import java.util.Set;
 import java.util.HashSet;
-import java.util.SortedSet;
-import java.util.TreeSet;
+import java.util.LinkedHashSet;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.builder.ToStringBuilder;
@@ -1487,6 +1486,9 @@ private void sortAssignments(List assignments, SortType sortBy, boolean ascendin
 	  	case SORT_BY_SORTING:
 			 comp = Assignment.sortingComparator;
 			 break;
+			case SORT_BY_CATEGORY:
+				comp = Assignment.categoryComparator;
+				break;
 		default:
 			comp = GradableObject.defaultComparator;	
 	}
@@ -1500,28 +1502,39 @@ private void sortAssignments(List assignments, SortType sortBy, boolean ascendin
   	}
   }
 
+/*
+ * (non-Javadoc)
+ * @see org.sakaiproject.service.gradebook.shared.GradebookService#getViewableAssignmentsForCurrentUser(java.lang.String)
+ */
+	@Override
+	@SuppressWarnings({ "rawtypes", "unchecked" })
+	public List<org.sakaiproject.service.gradebook.shared.Assignment> getViewableAssignmentsForCurrentUser(String gradebookUid)
+			throws GradebookNotFoundException {
+		return getViewableAssignmentsForCurrentUser(gradebookUid, SortType.SORT_BY_SORTING);
+	}
+
   /*
    * (non-Javadoc)
-   * @see org.sakaiproject.service.gradebook.shared.GradebookService#getViewableAssignmentsForCurrentUser(java.lang.String)
+   * @see org.sakaiproject.service.gradebook.shared.GradebookService#getViewableAssignmentsForCurrentUser(java.lang.String, java.)
    */
   @Override
   @SuppressWarnings({ "rawtypes", "unchecked" })
-  public List<org.sakaiproject.service.gradebook.shared.Assignment> getViewableAssignmentsForCurrentUser(String gradebookUid)
+  public List<org.sakaiproject.service.gradebook.shared.Assignment> getViewableAssignmentsForCurrentUser(String gradebookUid, SortType sortBy)
   throws GradebookNotFoundException {
 
 	  List<Assignment> viewableAssignments = new ArrayList<>();
-	  SortedSet<org.sakaiproject.service.gradebook.shared.Assignment> assignmentsToReturn = new TreeSet<>();
+		LinkedHashSet<org.sakaiproject.service.gradebook.shared.Assignment> assignmentsToReturn = new LinkedHashSet<>();
 
 	  Gradebook gradebook = getGradebook(gradebookUid);
 
 	  // will send back all assignments if user can grade all
 	  if (getAuthz().isUserAbleToGradeAll(gradebookUid)) {
-		  viewableAssignments = getAssignments(gradebook.getId(), SortType.SORT_BY_SORTING, true);
+		  viewableAssignments = getAssignments(gradebook.getId(), sortBy, true);
 	  } else if (getAuthz().isUserAbleToGrade(gradebookUid)) {
 		  // if user can grade and doesn't have grader perm restrictions, they
 		  // may view all assigns
 		  if (!getAuthz().isUserHasGraderPermissions(gradebookUid)) {
-			  viewableAssignments = getAssignments(gradebook.getId(), SortType.SORT_BY_SORTING, true);
+			  viewableAssignments = getAssignments(gradebook.getId(), sortBy, true);
 		  } else {
 			  // this user has grader perms, so we need to filter the items returned
 			  // if this gradebook has categories enabled, we need to check for category-specific restrictions

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

@@ -55,6 +55,7 @@
 import org.sakaiproject.service.gradebook.shared.GraderPermission;
 import org.sakaiproject.service.gradebook.shared.InvalidGradeException;
 import org.sakaiproject.service.gradebook.shared.PermissionDefinition;
+import org.sakaiproject.service.gradebook.shared.SortType;
 import org.sakaiproject.site.api.Group;
 import org.sakaiproject.site.api.Site;
 import org.sakaiproject.site.api.SiteService;
@@ -251,26 +252,49 @@ private Gradebook getGradebook(final String siteId) {
 		}
 	}
 
+
 	/**
 	 * Get a list of assignments in the gradebook in the current site that the current user is allowed to access
 	 *
 	 * @return a list of assignments or null if no gradebook
 	 */
 	public List<Assignment> getGradebookAssignments() {
-		return getGradebookAssignments(getCurrentSiteId());
+		return getGradebookAssignments(getCurrentSiteId(), SortType.SORT_BY_SORTING);
+	}
+
+
+	/**
+	 * Get a list of assignments in the gradebook in the current site that the current user is allowed to access
+	 *
+	 * @return a list of assignments or null if no gradebook
+	 */
+	public List<Assignment> getGradebookAssignments(final String siteId) {
+		return getGradebookAssignments(siteId, SortType.SORT_BY_SORTING);
 	}
 
+
+	/**
+	 * Get a list of assignments in the gradebook in the current site that the current user is allowed to access
+	 * sorted by the provided SortType
+	 *
+	 * @return a list of assignments or null if no gradebook
+	 */
+	public List<Assignment> getGradebookAssignments(final SortType sortBy) {
+		return getGradebookAssignments(getCurrentSiteId(), sortBy);
+	}
+
+
 	/**
 	 * Get a list of assignments in the gradebook in the specified site that the current user is allowed to access, sorted by sort order
 	 *
 	 * @param siteId the siteId
 	 * @return a list of assignments or null if no gradebook
 	 */
-	public List<Assignment> getGradebookAssignments(final String siteId) {
+	public List<Assignment> getGradebookAssignments(final String siteId, final SortType sortBy) {
 		final Gradebook gradebook = getGradebook(siteId);
 		if (gradebook != null) {
 			// applies permissions (both student and TA) and default sort is SORT_BY_SORTING
-			return this.gradebookService.getViewableAssignmentsForCurrentUser(gradebook.getUid());
+			return this.gradebookService.getViewableAssignmentsForCurrentUser(gradebook.getUid(), sortBy);
 		}
 		return null;
 	}

gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/pages/GradebookPage.java

@@ -27,6 +27,7 @@
 import org.apache.wicket.markup.html.WebComponent;
 import org.apache.wicket.markup.html.WebMarkupContainer;
 import org.apache.wicket.markup.html.basic.Label;
+import org.apache.wicket.markup.html.form.Button;
 import org.apache.wicket.markup.html.form.ChoiceRenderer;
 import org.apache.wicket.markup.html.form.DropDownChoice;
 import org.apache.wicket.markup.html.form.Form;
@@ -57,6 +58,7 @@
 import org.sakaiproject.gradebookng.tool.panels.ToggleGradeItemsToolbarPanel;
 import org.sakaiproject.service.gradebook.shared.Assignment;
 import org.sakaiproject.service.gradebook.shared.CategoryDefinition;
+import org.sakaiproject.service.gradebook.shared.SortType;
 
 /**
  * Grades page. Instructors and TAs see this one. Students see the {@link StudentPage}.
@@ -156,16 +158,17 @@ public boolean isVisible() {
 		// first get any settings data from the session
 		final GradebookUiSettings settings = getUiSettings();
 
-		// get list of assignments. this allows us to build the columns and then fetch the grades for each student for each assignment from
-		// the map
-		final List<Assignment> assignments = this.businessService.getGradebookAssignments();
-		Temp.time("getGradebookAssignments", stopwatch.getTime());
-
+		SortType sortBy = SortType.SORT_BY_SORTING;
 		if (settings.isCategoriesEnabled()) {
 			// Pre-sort assignments by the categorized sort order
-			Collections.sort(assignments, new CategorizedAssignmentComparator());
+			sortBy = SortType.SORT_BY_CATEGORY;
 		}
 
+		// get list of assignments. this allows us to build the columns and then fetch the grades for each student for each assignment from
+		// the map
+		final List<Assignment> assignments = this.businessService.getGradebookAssignments(sortBy);
+		Temp.time("getGradebookAssignments", stopwatch.getTime());
+
 		// get the grade matrix. It should be sorted if we have that info
 		final List<GbStudentGradeInfo> grades = this.businessService.buildGradeMatrix(assignments,
 				settings.getAssignmentSortOrder(), settings.getNameSortOrder(), settings.getCategorySortOrder(),
@@ -417,7 +420,7 @@ protected WebComponent newNavigatorLabel(final String navigatorId, final DataTab
 		final WebMarkupContainer toggleGradeItemsToolbarItem = new WebMarkupContainer("toggleGradeItemsToolbarItem");
 		this.form.add(toggleGradeItemsToolbarItem);
 
-		final AjaxButton toggleCategoriesToolbarItem = new AjaxButton("toggleCategoriesToolbarItem") {
+		final Button toggleCategoriesToolbarItem = new Button("toggleCategoriesToolbarItem") {
 			@Override
 			protected void onInitialize() {
 				super.onInitialize();
@@ -427,17 +430,12 @@ protected void onInitialize() {
 			}
 
 			@Override
-			protected void onSubmit(final AjaxRequestTarget target, final Form<?> form) {
+			public void onSubmit() {
 				settings.setCategoriesEnabled(!settings.isCategoriesEnabled());
 				setUiSettings(settings);
 
-				if (settings.isCategoriesEnabled()) {
-					add(new AttributeModifier("class", "on"));
-				} else {
-					add(new AttributeModifier("class", ""));
-				}
-				target.add(this);
-				target.appendJavaScript("sakai.gradebookng.spreadsheet.toggleCategories();");
+				// refresh
+				setResponsePage(new GradebookPage());
 			}
 
 			@Override