Skip to content

Commit

Permalink
SAK-33466 fix the ordering and combine REST endpoints (sakaiproject#5260
Browse files Browse the repository at this point in the history
)

* Remove unused code and cleanup other compiler warnings

* Add rebel.xml to gitignore so that JRebel project config is not committed

* SAK-33466 order the CourseGradeSummary data according to the schema in use. Combine the REST endpoints

* SAK-22466 combine rest endpoint calls for rendering the graph

* SAK-33466 prevent submitting the form when enter is pressed
  • Loading branch information
steveswinsburg authored Jan 30, 2018
1 parent 88667b8 commit f31bc02
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public class GradebookNgBusinessService {

public static final String ASSIGNMENT_ORDER_PROP = "gbng_assignment_order";
public static final String ICON_SAKAI = "icon-sakai--";

public static final ResourceLoader externalAppLoader = new ResourceLoader("org.sakaiproject.localization.bundle.tool.tools");

/**
Expand All @@ -148,7 +148,7 @@ public class GradebookNgBusinessService {
public List<String> getGradeableUsers() {
return this.getGradeableUsers(getCurrentSiteId());
}

/**
* Get a list of all users in the given site that can have grades
*
Expand All @@ -162,7 +162,7 @@ public List<String> getGradeableUsers(final String siteId) {
* Get the list of gradeable users
* @param groupFilter
* @return
*
*
*/
public List<String> getGradeableUsers(final GbGroup groupFilter) {
return this.getGradeableUsers(null, groupFilter);
Expand Down Expand Up @@ -277,8 +277,8 @@ public List<User> getUsers(final List<String> userUuids) throws GbException {
public Map<String, GbUser> getUserEidMap() {
final List<GbUser> users = getGbUsers(getGradeableUsers());
final Map<String, GbUser> userEidMap = new HashMap<>();
for (GbUser user : users) {
String eid = user.getDisplayId();
for (final GbUser user : users) {
final String eid = user.getDisplayId();
if (StringUtils.isNotBlank(eid)) {
userEidMap.put(eid, user);
}
Expand All @@ -295,10 +295,10 @@ public Map<String, GbUser> getUserEidMap() {
*/
public List<GbUser> getGbUsers(final List<String> userUuids)
{
List<GbUser> gbUsers = new ArrayList<>(userUuids.size());
List<User> users = getUsers(userUuids);
final List<GbUser> gbUsers = new ArrayList<>(userUuids.size());
final List<User> users = getUsers(userUuids);

for (User u : users) {
for (final User u : users) {
gbUsers.add(new GbUser(u));
}

Expand Down Expand Up @@ -1295,7 +1295,7 @@ public User getCurrentUser() {
* @return true if the current user is admin, false otherwise.
*/
public boolean isSuperUser() {
return securityService.isSuperUser();
return this.securityService.isSuperUser();
}

/**
Expand Down Expand Up @@ -1854,12 +1854,20 @@ public Double getCategoryScoreForStudent(final Long categoryId, final String stu
}

/**
* Get the settings for this gradebook. Note that this CANNOT be called by a student.
* Get the settings for this gradebook. Note that this CANNOT be called by a student nor by an entityprovider
*
* @return
*/
public GradebookInformation getGradebookSettings() {
final String siteId = getCurrentSiteId();
return getGradebookSettings(getCurrentSiteId());
}

/**
* Get the settings for this gradebook. Note that this CANNOT be called by a student. Safe to use from an entityprovider.
*
* @return
*/
public GradebookInformation getGradebookSettings(final String siteId) {
final Gradebook gradebook = getGradebook(siteId);

final GradebookInformation settings = this.gradebookService.getGradebookInformation(gradebook.getUid());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.math.NumberUtils;
Expand All @@ -42,6 +43,7 @@
import org.sakaiproject.gradebookng.rest.model.CourseGradeSummary;
import org.sakaiproject.service.gradebook.shared.CourseGrade;
import org.sakaiproject.service.gradebook.shared.GradeMappingDefinition;
import org.sakaiproject.service.gradebook.shared.GradebookInformation;
import org.sakaiproject.site.api.SiteService;
import org.sakaiproject.tool.api.SessionManager;

Expand Down Expand Up @@ -199,21 +201,6 @@ public String getComments(final EntityView view, final Map<String, Object> param
@EntityCustomAction(action = "course-grades", viewKey = EntityView.VIEW_LIST)
public CourseGradeSummary getCourseGradeSummary(final EntityView view, final Map<String, Object> params) {

// get params
final String siteId = (String) params.get("siteId");

checkValidSite(siteId);
checkInstructor(siteId);

// get course grades and re-map
final Map<String, CourseGrade> courseGrades = this.businessService.getCourseGrades(siteId);
return reMap(courseGrades);
}

@SuppressWarnings("unused")
@EntityCustomAction(action = "rebuild-course-grades", viewKey = EntityView.VIEW_LIST)
public CourseGradeSummary rebuildCourseGradeSummary(final EntityView view, final Map<String, Object> params) {

// get params
final String siteId = (String) params.get("siteId");
final String schema = (String) params.get("schema");
Expand All @@ -223,23 +210,36 @@ public CourseGradeSummary rebuildCourseGradeSummary(final EntityView view, final
checkValidSite(siteId);
checkInstructor(siteId);

// get the passed in schema
final Gson gson = new Gson();
final Type mappingType = new TypeToken<LinkedHashMap<String, Double>>(){}.getType();
Map<String, Double> gradingSchema = gson.fromJson(schema, mappingType);
// if we have a schema provided, deserialise
Map<String, Double> gradingSchema = null;
if (StringUtils.isNotBlank(schema)) {
final Gson gson = new Gson();
final Type mappingType = new TypeToken<LinkedHashMap<String, Double>>() {
}.getType();
gradingSchema = gson.fromJson(schema, mappingType);

log.debug("gradeMap:" + gradingSchema);
log.debug("provided gradeMap:" + gradingSchema);

if (gradingSchema == null) {
throw new IllegalArgumentException("Grading schema data was missing / invalid");
}
}

// if still null, use the persistent one for this gradebook
if (gradingSchema == null) {
throw new IllegalArgumentException("Grading schema data was missing / invalid");
log.debug("gradeMap not provided, using persistent one");
final GradebookInformation info = this.businessService.getGradebookSettings(siteId);
gradingSchema = info.getSelectedGradingScaleBottomPercents();
log.debug("persistent gradeMap:" + gradingSchema);
}

// ensure it is sorted so the grade mapping works correctly
// ensure grading schema is sorted so the grade mapping works correctly
gradingSchema = GradeMappingDefinition.sortGradeMapping(gradingSchema);

// get the course grades using the passed in schema
// get the course grades and re-map to summary. Also sorts the data so it is ready for the consumer to use
final Map<String, CourseGrade> courseGrades = this.businessService.getCourseGrades(siteId, gradingSchema);
return reMap(courseGrades);

return reMap(courseGrades, gradingSchema.keySet());
}

/**
Expand Down Expand Up @@ -329,14 +329,25 @@ private GbRole getUserRole(final String siteId) {

/**
* Re-map the course grades returned from the business service into our CourseGradeSummary object for returning on the REST API.
*
* @param courseGrades map of student to course grade
* @param gradingSchema the grading schema that has the order
* @return
*/
private CourseGradeSummary reMap(final Map<String, CourseGrade> courseGrades) {
private CourseGradeSummary reMap(final Map<String, CourseGrade> courseGrades, final Set<String> order) {
final CourseGradeSummary summary = new CourseGradeSummary();
courseGrades.forEach((k,v) -> {
summary.add(v.getDisplayGrade());
});

//sort the map based on the ordered schema
Map<String, Integer> originalData = summary.getDataset();
Map<String, Integer> sortedData = new LinkedHashMap<>();
order.forEach(o -> {
sortedData.put(o, originalData.get(o));
});
summary.setDataset(sortedData);

return summary;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ <h2><wicket:message key="settingspage.main.heading">Gradebook Setup</wicket:mess
</div>

<div>
<input type="submit" class="active" wicket:message="value:button.savechanges" />
<input type="submit" wicket:id="cancel" wicket:message="value:button.cancel" />
<input type="button" wicket:id="submit" class="active" wicket:message="value:button.savechanges" />
<input type="button" wicket:id="cancel" wicket:message="value:button.cancel" />
</div>
</form>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@

import org.apache.wicket.Page;
import org.apache.wicket.ajax.AjaxRequestTarget;
import org.apache.wicket.ajax.markup.html.form.AjaxButton;
import org.apache.wicket.markup.head.CssHeaderItem;
import org.apache.wicket.markup.head.IHeaderResponse;
import org.apache.wicket.markup.head.JavaScriptHeaderItem;
import org.apache.wicket.markup.html.form.Button;
import org.apache.wicket.markup.html.form.Form;
import org.apache.wicket.model.CompoundPropertyModel;
import org.sakaiproject.component.cover.ServerConfigurationService;
Expand Down Expand Up @@ -54,7 +54,7 @@ public class SettingsPage extends BasePage {
private boolean categoryExpanded = false;
private boolean gradingSchemaExpanded = false;

private boolean hideGradeEntryFromNonAdmins;
private boolean showGradeEntryToNonAdmins;
private static final String SAK_PROP_SHOW_GRADE_ENTRY_TO_NON_ADMINS = "gradebook.settings.gradeEntry.showToNonAdmins";
private static final boolean SAK_PROP_SHOW_GRADE_ENTRY_TO_NON_ADMINS_DEFAULT = true;

Expand All @@ -65,7 +65,7 @@ public class SettingsPage extends BasePage {

public SettingsPage() {
disableLink(this.settingsPageLink);
setHideGradeEntryFromNonAdmins();
setShowGradeEntryToNonAdmins();
}

public SettingsPage(final boolean gradeEntryExpanded, final boolean gradeReleaseExpanded,
Expand All @@ -75,11 +75,11 @@ public SettingsPage(final boolean gradeEntryExpanded, final boolean gradeRelease
this.gradeReleaseExpanded = gradeReleaseExpanded;
this.categoryExpanded = categoryExpanded;
this.gradingSchemaExpanded = gradingSchemaExpanded;
setHideGradeEntryFromNonAdmins();
setShowGradeEntryToNonAdmins();
}

private void setHideGradeEntryFromNonAdmins() {
hideGradeEntryFromNonAdmins = ServerConfigurationService.getBoolean(SAK_PROP_SHOW_GRADE_ENTRY_TO_NON_ADMINS, SAK_PROP_SHOW_GRADE_ENTRY_TO_NON_ADMINS_DEFAULT);
private void setShowGradeEntryToNonAdmins() {
this.showGradeEntryToNonAdmins = ServerConfigurationService.getBoolean(SAK_PROP_SHOW_GRADE_ENTRY_TO_NON_ADMINS, SAK_PROP_SHOW_GRADE_ENTRY_TO_NON_ADMINS_DEFAULT);
}

@Override
Expand All @@ -98,9 +98,9 @@ public void onInitialize() {
this.categoryPanel = new SettingsCategoryPanel("categoryPanel", formModel, this.categoryExpanded);
this.gradingSchemaPanel = new SettingsGradingSchemaPanel("gradingSchemaPanel", formModel, this.gradingSchemaExpanded);

// Hide the panel if sakai.property is true and user is not admin
if (hideGradeEntryFromNonAdmins && !businessService.isSuperUser()) {
gradeEntryPanel.setVisible(false);
// Hide the panel if not showing to non admins and user is not admin
if (!this.showGradeEntryToNonAdmins && !this.businessService.isSuperUser()) {
this.gradeEntryPanel.setVisible(false);
}

// form
Expand Down Expand Up @@ -174,10 +174,18 @@ public void onValidate() {

}

@Override
public void onSubmit() {
};

final GbSettings model = getModelObject();
// submit button
// required so that we can process the form only when clicked, not when enter is pressed in text field
// must be accompanied by a plain html button, not a submit button.
final AjaxButton submit = new AjaxButton("submit") {
private static final long serialVersionUID = 1L;

@Override
public void onSubmit(final AjaxRequestTarget target, final Form f) {
super.onSubmit();
final GbSettings model = (GbSettings) f.getModelObject();

Page responsePage = new SettingsPage(SettingsPage.this.gradeEntryPanel.isExpanded(),
SettingsPage.this.gradeReleasePanel.isExpanded(), SettingsPage.this.categoryPanel.isExpanded(),
Expand All @@ -202,9 +210,10 @@ public void onSubmit() {
setResponsePage(responsePage);
}
};
form.add(submit);

// cancel button
final Button cancel = new Button("cancel") {
final AjaxButton cancel = new AjaxButton("cancel") {
private static final long serialVersionUID = 1L;

@Override
Expand All @@ -224,15 +233,15 @@ public void onSubmit() {
add(form);

// expand/collapse panel actions
add(new GbAjaxLink("expandAll") {
add(new GbAjaxLink<Void>("expandAll") {
private static final long serialVersionUID = 1L;

@Override
public void onClick(final AjaxRequestTarget target) {
target.appendJavaScript("$('#settingsAccordion .panel-collapse').collapse('show');");
}
});
add(new GbAjaxLink("collapseAll") {
add(new GbAjaxLink<Void>("collapseAll") {
private static final long serialVersionUID = 1L;

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ private void refreshCourseGradeChart() {
final String siteId = SettingsGradingSchemaPanel.this.businessService.getCurrentSiteId();

// TODO this could be a wicket component instead of Javascript
this.target.appendJavaScript("refreshChart('" + siteId + "', '" + FormatHelper.encode(schemaJson) + "')");
this.target.appendJavaScript("renderChart('" + siteId + "', '" + FormatHelper.encode(schemaJson) + "')");
}

/**
Expand Down
19 changes: 13 additions & 6 deletions gradebookng/tool/src/webapp/scripts/gradebook-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,22 @@ GradebookCategorySettings.prototype.updateCategoryOrders = function() {
};

/**
* Render the course grade summary chart for the site.
* This delegates to the REST service which calculates the course grades based on persistent data.
* Refresh the course grade summary chart for the site.
* If the schema is provided it is sent and the course grades will be recalculated based on transient data.
* Otherwise it will use the persistent data.
* @param siteId
* @param schemaJson the JSON of the grading schema
* @returns
*/
function renderChart(siteId) {

function renderChart(siteId, schemaJson) {

var url = "/direct/gbng/course-grades.json?siteId="+siteId;
if(schemaJson) {
url += "&schema="+schemaJson;
}

$.ajax({
url : "/direct/gbng/course-grades.json?siteId="+siteId,
url : url,
dataType : "json",
async : true,
cache: false,
Expand All @@ -106,7 +113,7 @@ function renderChart(siteId) {
function refreshChart(siteId, schemaJson) {

$.ajax({
url : "/direct/gbng/rebuild-course-grades.json?siteId="+siteId+"&schema="+schemaJson,
url : "/direct/gbng/course-grades.json?siteId="+siteId+"&schema="+schemaJson,
dataType : "json",
async : true,
cache: false,
Expand Down

0 comments on commit f31bc02

Please sign in to comment.