Skip to content

Commit

Permalink
SAK-33827: make GBNG category colour selection deterministic (sakaipr…
Browse files Browse the repository at this point in the history
  • Loading branch information
bjones86 authored and ottenhoff committed Jan 18, 2018
1 parent 4c133fd commit 895415b
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@

public class GbGradebookData {

private int NULL_SENTINEL = 127;
private final int NULL_SENTINEL = 127;

@Data
private class StudentDefinition {
Expand Down Expand Up @@ -358,7 +358,7 @@ private Map<String, Object> serializeSettings() {
};

private List<String[]> courseGrades() {
final List<String[]> result = new ArrayList<String[]>();
final List<String[]> result = new ArrayList<>();

final Map<String, Double> gradeMap = settings.getSelectedGradingScaleBottomPercents();
final List<String> ascendingGrades = new ArrayList<>(gradeMap.keySet());
Expand Down Expand Up @@ -409,7 +409,7 @@ public int compare(final String a, final String b) {
}

private List<Score> gradeList() {
final List<Score> result = new ArrayList<Score>();
final List<Score> result = new ArrayList<>();

for (GbStudentGradeInfo studentGradeInfo : GbGradebookData.this.studentGradeInfoList) {
for (ColumnDefinition column : GbGradebookData.this.columns) {
Expand All @@ -427,7 +427,7 @@ private String getString(final String key) {
}

private List<StudentDefinition> loadStudents(final List<GbStudentGradeInfo> studentInfo) {
final List<StudentDefinition> result = new ArrayList<StudentDefinition>();
final List<StudentDefinition> result = new ArrayList<>();

for (GbStudentGradeInfo student : studentInfo) {
final StudentDefinition studentDefinition = new StudentDefinition();
Expand Down Expand Up @@ -458,7 +458,7 @@ private List<StudentDefinition> loadStudents(final List<GbStudentGradeInfo> stud
private List<ColumnDefinition> loadColumns(final List<Assignment> assignments) {
final GradebookUiSettings userSettings = ((GradebookPage) parent.getPage()).getUiSettings();

final List<ColumnDefinition> result = new ArrayList<ColumnDefinition>();
final List<ColumnDefinition> result = new ArrayList<>();

if (assignments.isEmpty()) {
return result;
Expand Down Expand Up @@ -495,7 +495,7 @@ private List<ColumnDefinition> loadColumns(final List<Assignment> assignments) {

nullable(a1.getCategoryId()),
a1.getCategoryName(),
userSettings.getCategoryColor(a1.getCategoryName()),
userSettings.getCategoryColor(a1.getCategoryName(), a1.getCategoryId()),
nullable(categoryWeight),
a1.isCategoryExtraCredit(),

Expand All @@ -512,7 +512,7 @@ private List<ColumnDefinition> loadColumns(final List<Assignment> assignments) {
.getString(),
nullable(categoryWeight),
a1.isCategoryExtraCredit(),
userSettings.getCategoryColor(a1.getCategoryName()),
userSettings.getCategoryColor(a1.getCategoryName(), a1.getCategoryId()),
!uiSettings.isCategoryScoreVisible(a1.getCategoryName())));
}
}
Expand All @@ -533,7 +533,7 @@ private List<ColumnDefinition> loadColumns(final List<Assignment> assignments) {
.getString(),
nullable(categoryWeight),
category.isExtraCredit(),
userSettings.getCategoryColor(category.getName()),
userSettings.getCategoryColor(category.getName(), category.getId()),
!uiSettings.isCategoryScoreVisible(category.getName())));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,14 @@ public GradebookUiSettings() {
// defaults. Note there is no default for assignmentSortOrder as that
// requires an assignmentId which will differ between gradebooks
this.categoriesEnabled = false;
this.assignmentVisibility = new HashMap<Long, Boolean>();
this.categoryScoreVisibility = new HashMap<String, Boolean>();
this.assignmentVisibility = new HashMap<>();
this.categoryScoreVisibility = new HashMap<>();

// default sort order to student
this.nameSortOrder = GbStudentNameSortOrder.LAST_NAME;
this.studentSortOrder = SortDirection.ASCENDING;

this.categoryColors = new HashMap<String, String>();
this.categoryColors = new HashMap<>();
this.showPoints = false;
this.gradeSummaryGroupedByCategory = false;
}
Expand All @@ -151,16 +151,16 @@ public void setCategoryColor(final String categoryName, final String rgbColorStr
this.categoryColors.put(categoryName, rgbColorString);
}

public String getCategoryColor(final String categoryName) {
public String getCategoryColor(final String categoryName, final Long categoryID) {
if (!this.categoryColors.containsKey(categoryName)) {
setCategoryColor(categoryName, generateRandomRGBColorString());
setCategoryColor(categoryName, generateRandomRGBColorString(categoryID));
}
return this.categoryColors.get(categoryName);
}

public void initializeCategoryColors(final List<CategoryDefinition> categories) {
for (CategoryDefinition category : categories) {
setCategoryColor(category.getName(), generateRandomRGBColorString());
setCategoryColor(category.getName(), generateRandomRGBColorString(category.getId()));
}
}

Expand All @@ -178,8 +178,11 @@ public void setGroupedByCategory(final boolean groupedByCategory) {
/**
* Helper to generate a RGB CSS color string with values between 180-250 to ensure a lighter color e.g. rgb(181,222,199)
*/
public static String generateRandomRGBColorString() {
final Random rand = new Random();
public static String generateRandomRGBColorString(Long categoryID) {
if (categoryID == null) {
categoryID = -1L;
}
final Random rand = new Random(categoryID);
final int min = 180;
final int max = 250;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import org.sakaiproject.gradebookng.business.GbRole;
import org.sakaiproject.gradebookng.business.model.GbGroup;
import org.sakaiproject.gradebookng.business.util.GbStopWatch;
import org.sakaiproject.gradebookng.business.util.MessageHelper;
import org.sakaiproject.gradebookng.tool.actions.DeleteAssignmentAction;
import org.sakaiproject.gradebookng.tool.actions.EditAssignmentAction;
import org.sakaiproject.gradebookng.tool.actions.EditCommentAction;
Expand Down Expand Up @@ -141,7 +140,7 @@ public GradebookPage() {
stopwatch.start();
stopwatch.time("GradebookPage init", stopwatch.getTime());

this.form = new Form<Void>("form");
this.form = new Form<>("form");
add(this.form);

form.add(new AttributeModifier("data-siteid", businessService.getCurrentSiteId()));
Expand Down Expand Up @@ -194,10 +193,7 @@ public void onSubmit(final AjaxRequestTarget target, final Form form) {

@Override
public boolean isVisible() {
if (GradebookPage.this.role != GbRole.INSTRUCTOR) {
return false;
}
return true;
return GradebookPage.this.role == GbRole.INSTRUCTOR;
}
};
addGradeItem.setDefaultFormProcessing(false);
Expand Down Expand Up @@ -483,7 +479,7 @@ public GradebookUiSettings getUiSettings() {
settings = new GradebookUiSettings();
settings.setCategoriesEnabled(this.businessService.categoriesAreEnabled());
settings.initializeCategoryColors(this.businessService.getGradebookCategories());
settings.setCategoryColor(getString(GradebookPage.UNCATEGORISED), settings.generateRandomRGBColorString());
settings.setCategoryColor(getString(GradebookPage.UNCATEGORISED), GradebookUiSettings.generateRandomRGBColorString(null));
setUiSettings(settings);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

public class SortGradeItemsByCategoryPanel extends Panel {
Expand Down Expand Up @@ -66,22 +67,22 @@ protected void populateItem(final ListItem<CategoryDefinition> categoryItem) {
Collections.sort(assignments, new CategorizedAssignmentComparator());

categoryItem.add(new AttributeModifier("style",
String.format("border-left-color: %s", settings.getCategoryColor(category.getName()))));
String.format("border-left-color: %s", settings.getCategoryColor(category.getName(), category.getId()))));
categoryItem.add(new Label("name", category.getName()));
categoryItem.add(new ListView<Assignment>("gradeItemList", assignments) {
@Override
protected void populateItem(final ListItem<Assignment> assignmentItem) {
final Assignment assignment = assignmentItem.getModelObject();
assignmentItem.add(new Label("name", assignment.getName()));
assignmentItem.add(new HiddenField<Long>("id",
assignmentItem.add(new HiddenField<>("id",
Model.of(assignment.getId())).add(
new AttributeModifier("name",
String.format("id", assignment.getId()))));
assignmentItem.add(new HiddenField<Integer>("order",
assignmentItem.add(new HiddenField<>("order",
Model.of(assignment.getCategorizedSortOrder())).add(
new AttributeModifier("name",
String.format("item_%s[order]", assignment.getId()))));
assignmentItem.add(new HiddenField<Integer>("current_order",
assignmentItem.add(new HiddenField<>("current_order",
Model.of(assignment.getCategorizedSortOrder())).add(
new AttributeModifier("name",
String.format("item_%s[current_order]", assignment.getId()))));
Expand All @@ -96,7 +97,7 @@ class CategorizedAssignmentComparator implements Comparator<Assignment> {
@Override
public int compare(final Assignment a1, final Assignment a2) {
// if in the same category, sort by their categorized sort order
if (a1.getCategoryId() == a2.getCategoryId()) {
if (Objects.equals(a1.getCategoryId(), a2.getCategoryId())) {
// handles null orders by putting them at the end of the list
if (a1.getCategorizedSortOrder() == null) {
return 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ public void onInitialize() {
super.onInitialize();

// setup
final List<String> categoryNames = new ArrayList<String>();
final Map<String, List<Assignment>> categoryNamesToAssignments = new HashMap<String, List<Assignment>>();
final Map<String, Long> categoryNameToIdMap = new HashMap<>();
final Map<String, List<Assignment>> categoryNamesToAssignments = new HashMap<>();

final Map<String, Object> model = (Map<String, Object>) getDefaultModelObject();
final List<Assignment> assignments = (List<Assignment>) model.get("assignments");
Expand All @@ -66,21 +66,25 @@ public void onInitialize() {
for (final Assignment assignment : assignments) {

final String categoryName = getCategoryName(assignment, categoriesEnabled);
final Long categoryID = assignment.getCategoryId();

if (!categoryNamesToAssignments.containsKey(categoryName)) {
categoryNames.add(categoryName);
categoryNamesToAssignments.put(categoryName, new ArrayList<Assignment>());
categoryNameToIdMap.put(categoryName, categoryID);
categoryNamesToAssignments.put(categoryName, new ArrayList<>());
}

categoryNamesToAssignments.get(categoryName).add(assignment);
}

List<String> categoryNames = new ArrayList<>(categoryNameToIdMap.keySet());
add(new ListView<String>("categoriesList", categoryNames) {
private static final long serialVersionUID = 1L;

@Override
protected void populateItem(final ListItem<String> categoryItem) {
final String categoryName = categoryItem.getModelObject();
final Long categoryID = categoryNameToIdMap.get(categoryName);
final String categoryColor = settings.getCategoryColor(categoryName, categoryID);

WebMarkupContainer categoryFilter = new WebMarkupContainer("categoryFilter");
if (!categoriesEnabled) {
Expand All @@ -92,13 +96,11 @@ protected void populateItem(final ListItem<String> categoryItem) {
final GradebookPage gradebookPage = (GradebookPage) getPage();

final Label categoryLabel = new Label("category", categoryName);
categoryLabel.add(new AttributeModifier("data-category-color", settings.getCategoryColor(categoryName)));
categoryLabel.add(new AttributeModifier("data-category-color", categoryColor));
categoryFilter.add(categoryLabel);

categoryFilter.add(new WebMarkupContainer("categorySignal").add(new AttributeModifier("style",
String.format("background-color: %s; border-color: %s",
settings.getCategoryColor(categoryName),
settings.getCategoryColor(categoryName)))));
String.format("background-color: %s; border-color: %s", categoryColor, categoryColor))));

final CheckBox categoryCheckbox = new CheckBox("categoryCheckbox");
categoryCheckbox.add(new AttributeModifier("value", categoryName));
Expand All @@ -119,14 +121,12 @@ protected void populateItem(final ListItem<Assignment> assignmentItem) {
final WebMarkupContainer assignmentSignal = new WebMarkupContainer("assignmentSignal");
if (settings.isCategoriesEnabled()) {
assignmentSignal.add(new AttributeModifier("style",
String.format("background-color: %s; border-color: %s",
settings.getCategoryColor(getCategoryName(assignment, categoriesEnabled)),
settings.getCategoryColor(getCategoryName(assignment, categoriesEnabled)))));
String.format("background-color: %s; border-color: %s", categoryColor, categoryColor)));
}
assignmentItem.add(assignmentSignal);

final CheckBox assignmentCheckbox = new AjaxCheckBox("assignmentCheckbox",
Model.of(Boolean.valueOf(settings.isAssignmentVisible(assignment.getId())))) {
Model.of(settings.isAssignmentVisible(assignment.getId()))) {
@Override
protected void onUpdate(final AjaxRequestTarget target) {
GradebookUiSettings settings = gradebookPage.getUiSettings();
Expand All @@ -144,14 +144,12 @@ protected void onUpdate(final AjaxRequestTarget target) {
});

final WebMarkupContainer categoryScoreFilter = new WebMarkupContainer("categoryScore");
categoryScoreFilter.setVisible(categoryName != getString(GradebookPage.UNCATEGORISED));
categoryScoreFilter.setVisible(!StringUtils.equals(categoryName, getString(GradebookPage.UNCATEGORISED)));
categoryScoreFilter.add(new Label("categoryScoreLabel",
new StringResourceModel("label.toolbar.categoryscorelabel", null, new Object[] { categoryName })));

categoryScoreFilter.add(new WebMarkupContainer("categoryScoreSignal").add(new AttributeModifier("style",
String.format("background-color: %s; border-color: %s",
settings.getCategoryColor(categoryName),
settings.getCategoryColor(categoryName)))));
String.format("background-color: %s; border-color: %s", categoryColor, categoryColor))));

final CheckBox categoryScoreCheckbox = new AjaxCheckBox("categoryScoreCheckbox",
new Model<Boolean>(settings.isCategoryScoreVisible(categoryName))) {// Model.of(Boolean.valueOf(settings.isCategoryScoreVisible(category))))
Expand Down

0 comments on commit 895415b

Please sign in to comment.