Skip to content

Commit

Permalink
SAK-43936 - Gradebook: no points validation when editing a Gradebook … (
Browse files Browse the repository at this point in the history 
sakaiproject#8393)

Co-authored-by: Sam Ottenhoff <[email protected]>
  • Loading branch information
@jonespm @ottenhoff
jonespm and ottenhoff authored Aug 17, 2020
1 parent 79402bf commit 926ae50
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 77 deletions.
32 changes: 29 additions & 3 deletions ...ebook-service/api/src/java/org/sakaiproject/service/gradebook/shared/GradebookHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,37 @@ public class GradebookHelper {
* Validate a grade item title by checking against the reserved characters
* @param title
* @throws InvalidGradeItemNameException
* @throws ConflictingAssignmentNameException
* returns validatedName
*/
public static void validateGradeItemName(final String title) throws InvalidGradeItemNameException {
if (StringUtils.isBlank(title)
|| StringUtils.startsWithAny(title, GradebookService.INVALID_CHARS_AT_START_OF_GB_ITEM_NAME)) {
public static String validateGradeItemName(String title) throws InvalidGradeItemNameException, ConflictingAssignmentNameException {
// validate the name
title = StringUtils.trimToNull(title);
if (StringUtils.isBlank(title)) {
throw new ConflictingAssignmentNameException("You cannot save an assignment without a name");
}
else if (StringUtils.startsWithAny(title, GradebookService.INVALID_CHARS_AT_START_OF_GB_ITEM_NAME)) {
throw new InvalidGradeItemNameException("Grade Item name is invalid: " + title);
}
return title;
}

/**
* Validate assignment points and name is valid
* @param assignmentDefition
* @throws InvalidGradeItemNameException
* @throws AssignmentHasIllegalPointsException
* @throws ConflictingAssignmentNameException
* @return validated name
*/

public static String validateAssignmentNameAndPoints(final org.sakaiproject.service.gradebook.shared.Assignment assignmentDefinition)
throws InvalidGradeItemNameException, AssignmentHasIllegalPointsException, ConflictingAssignmentNameException {
// Ensure that points is > zero.
final Double points = assignmentDefinition.getPoints();
if ((points == null) || (points <= 0)) {
throw new AssignmentHasIllegalPointsException("Points must be > 0");
}
return validateGradeItemName(assignmentDefinition.getName());
}
}
7 changes: 1 addition & 6 deletions ...book-service/impl/src/java/org/sakaiproject/component/gradebook/BaseHibernateManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,13 +298,8 @@ private Long createNewAssignment(final Long gradebookId, final Long categoryId,
private GradebookAssignment prepareNewAssignment(final String name, final Double points, final Date dueDate, final Boolean isNotCounted, final Boolean isReleased,
final Boolean isExtraCredit, final Integer sortOrder, final Integer categorizedSortOrder)
{
final String validatedName = StringUtils.trimToNull(name);
if (validatedName == null){
throw new ConflictingAssignmentNameException("You cannot save an assignment without a name");
}

// name cannot contain these special chars as they are reserved for special columns in import/export
GradebookHelper.validateGradeItemName(validatedName);
final String validatedName = GradebookHelper.validateGradeItemName(name);

final GradebookAssignment asn = new GradebookAssignment();
asn.setName(validatedName);
Expand Down
24 changes: 7 additions & 17 deletions ...ice/impl/src/java/org/sakaiproject/component/gradebook/GradebookServiceHibernateImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,29 +614,26 @@ public Object doInHibernate(final Session session) throws HibernateException {

}


@Override
public Long addAssignment(final String gradebookUid, final org.sakaiproject.service.gradebook.shared.Assignment assignmentDefinition) {
if (!getAuthz().isUserAbleToEditAssessments(gradebookUid)) {
log.error("AUTHORIZATION FAILURE: User {} in gradebook {} attempted to add an assignment", getUserUid(), gradebookUid);
throw new GradebookSecurityException();
}

// Ensure that points is > zero.
final Double points = assignmentDefinition.getPoints();
if ((points == null) || (points <= 0)) {
throw new AssignmentHasIllegalPointsException("Points must be > 0");
}
final String validatedName = GradebookHelper.validateAssignmentNameAndPoints(assignmentDefinition);

final Gradebook gradebook = getGradebook(gradebookUid);

// if attaching to category
if (assignmentDefinition.getCategoryId() != null) {
return createAssignmentForCategory(gradebook.getId(), assignmentDefinition.getCategoryId(), assignmentDefinition.getName(),
points, assignmentDefinition.getDueDate(), !assignmentDefinition.isCounted(), assignmentDefinition.isReleased(),
return createAssignmentForCategory(gradebook.getId(), assignmentDefinition.getCategoryId(), validatedName,
assignmentDefinition.getPoints(), assignmentDefinition.getDueDate(), !assignmentDefinition.isCounted(), assignmentDefinition.isReleased(),
assignmentDefinition.isExtraCredit(), assignmentDefinition.getCategorizedSortOrder());
}

return createAssignment(gradebook.getId(), assignmentDefinition.getName(), points, assignmentDefinition.getDueDate(),
return createAssignment(gradebook.getId(), validatedName, assignmentDefinition.getPoints(), assignmentDefinition.getDueDate(),
!assignmentDefinition.isCounted(), assignmentDefinition.isReleased(), assignmentDefinition.isExtraCredit(), assignmentDefinition.getSortOrder());
}

Expand All @@ -649,15 +646,8 @@ public void updateAssignment(final String gradebookUid, final Long assignmentId,
gradebookUid, assignmentId);
throw new GradebookSecurityException();
}

// validate the name
final String validatedName = StringUtils.trimToNull(assignmentDefinition.getName());
if (validatedName == null) {
throw new ConflictingAssignmentNameException("You cannot save an assignment without a name");
}

// name cannot contain these chars as they are reserved for special columns in import/export
GradebookHelper.validateGradeItemName(validatedName);

final String validatedName = GradebookHelper.validateAssignmentNameAndPoints(assignmentDefinition);

final Gradebook gradebook = this.getGradebook(gradebookUid);

Expand Down
22 changes: 7 additions & 15 deletions ...ookng/tool/src/java/org/sakaiproject/gradebookng/business/GradebookNgBusinessService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2147,30 +2147,22 @@ public int getAssignmentSortOrder(final long assignmentId) {
* @param assignment
* @return
*/
public boolean updateAssignment(final Assignment assignment) {
public void updateAssignment(final Assignment assignment) {
final String siteId = getCurrentSiteId();
final Gradebook gradebook = getGradebook(siteId);

// need the original name as the service needs that as the key...
final Assignment original = this.getAssignment(assignment.getId());

try {
this.gradebookService.updateAssignment(gradebook.getUid(), original.getId(), assignment);

EventHelper.postUpdateAssignmentEvent(gradebook, assignment, getUserRoleOrNone());
this.gradebookService.updateAssignment(gradebook.getUid(), original.getId(), assignment);

if (original.getCategoryId() != null && assignment.getCategoryId() != null
&& original.getCategoryId().longValue() != assignment.getCategoryId().longValue()) {
updateAssignmentCategorizedOrder(gradebook.getUid(), assignment.getCategoryId(), assignment.getId(),
Integer.MAX_VALUE);
}
EventHelper.postUpdateAssignmentEvent(gradebook, assignment, getUserRoleOrNone());

return true;
} catch (final Exception e) {
log.error("An error occurred updating the assignment", e);
if (original.getCategoryId() != null && assignment.getCategoryId() != null
&& original.getCategoryId().longValue() != assignment.getCategoryId().longValue()) {
updateAssignmentCategorizedOrder(gradebook.getUid(), assignment.getCategoryId(), assignment.getId(),
Integer.MAX_VALUE);
}

return false;
}

/**
Expand Down
50 changes: 27 additions & 23 deletions ...ookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/AddOrEditGradeItemPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,36 @@ private void createGradeItem(final AjaxRequestTarget target, final Form<?> form,

// OK
if (validated) {
if (AddOrEditGradeItemPanel.this.mode == UiMode.EDIT) {

final boolean success = AddOrEditGradeItemPanel.this.businessService.updateAssignment(assignment);
Long assignmentId = null;
boolean success = true;

try {
if (AddOrEditGradeItemPanel.this.mode == UiMode.EDIT) {
assignmentId = assignment.getId();
AddOrEditGradeItemPanel.this.businessService.updateAssignment(assignment);
}
else {
assignmentId = AddOrEditGradeItemPanel.this.businessService.addAssignment(assignment);
}
rubricsService.saveRubricAssociation(RubricsConstants.RBCS_TOOL_GRADEBOOKNG, assignmentId.toString(), getRubricParameters(""));
}
catch (final AssignmentHasIllegalPointsException e) {
error(new ResourceModel("error.addgradeitem.points").getObject());
success = false;
} catch (final ConflictingAssignmentNameException e) {
error(new ResourceModel("error.addgradeitem.title").getObject());
success = false;
} catch (final ConflictingExternalIdException e) {
error(new ResourceModel("error.addgradeitem.exception").getObject());
success = false;
} catch (final Exception e) {
error(new ResourceModel("error.addgradeitem.exception").getObject());
success = false;
}

if (AddOrEditGradeItemPanel.this.mode == UiMode.EDIT) {
if (success) {
rubricsService.saveRubricAssociation(RubricsConstants.RBCS_TOOL_GRADEBOOKNG, assignment.getId().toString(), getRubricParameters(""));
getSession().success(MessageFormat.format(getString("message.edititem.success"), assignment.getName()));
setResponsePage(getPage().getPageClass(),
new PageParameters().add(GradebookPage.FOCUS_ASSIGNMENT_ID_PARAM, assignment.getId()));
Expand All @@ -231,27 +255,7 @@ private void createGradeItem(final AjaxRequestTarget target, final Form<?> form,
}

} else {

Long assignmentId = null;

boolean success = true;
try {
assignmentId = AddOrEditGradeItemPanel.this.businessService.addAssignment(assignment);
} catch (final AssignmentHasIllegalPointsException e) {
error(new ResourceModel("error.addgradeitem.points").getObject());
success = false;
} catch (final ConflictingAssignmentNameException e) {
error(new ResourceModel("error.addgradeitem.title").getObject());
success = false;
} catch (final ConflictingExternalIdException e) {
error(new ResourceModel("error.addgradeitem.exception").getObject());
success = false;
} catch (final Exception e) {
error(new ResourceModel("error.addgradeitem.exception").getObject());
success = false;
}
if (success) {
rubricsService.saveRubricAssociation(RubricsConstants.RBCS_TOOL_GRADEBOOKNG, assignmentId.toString(), getRubricParameters(""));
final String successMessage = MessageFormat.format(getString("notification.addgradeitem.success"), assignment.getName());
getSession()
.success(successMessage);
Expand Down
23 changes: 12 additions & 11 deletions gradebookng/tool/src/java/org/sakaiproject/gradebookng/tool/panels/BulkEditItemsPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,20 +162,21 @@ public void onSubmit(final AjaxRequestTarget target, final Form<?> form) {

final List<Assignment> assignments = (List<Assignment>) form.getModelObject();

boolean result = false;
for (final Assignment a : assignments) {
try {
for (final Assignment a : assignments) {

log.debug("Bulk edit assignment: {}", a);
result = BulkEditItemsPanel.this.businessService.updateAssignment(a);
}
for (int count=0; count < BulkEditItemsPanel.this.getDeletableItemsList().size(); count++){
BulkEditItemsPanel.this.businessService.removeAssignment(BulkEditItemsPanel.this.getDeletableItemsList().get(count));
}
BulkEditItemsPanel.this.clearDeletableItemsList();
if (result) {
log.debug("Bulk edit assignment: {}", a);
BulkEditItemsPanel.this.businessService.updateAssignment(a);
}
for (int count=0; count < BulkEditItemsPanel.this.getDeletableItemsList().size(); count++){
BulkEditItemsPanel.this.businessService.removeAssignment(BulkEditItemsPanel.this.getDeletableItemsList().get(count));
}
BulkEditItemsPanel.this.clearDeletableItemsList();
getSession().success(getString("bulkedit.update.success"));
} else {
}
catch (final Exception e) {
getSession().error(getString("bulkedit.update.error"));
log.warn("An error occurred updating the assignment", e);
}
setResponsePage(GradebookPage.class);
}
Expand Down
7 changes: 5 additions & 2 deletions ...va/org/sakaiproject/gradebookng/tool/panels/importExport/GradeImportConfirmationStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,14 @@ public void onSubmit(AjaxRequestTarget target, Form<?> form) {
final Assignment assignment = GradeImportConfirmationStep.this.businessService.getAssignment(item.getItemTitle());
assignment.setPoints(points);

final boolean updated = GradeImportConfirmationStep.this.businessService.updateAssignment(assignment);
if (!updated) {
try {
GradeImportConfirmationStep.this.businessService.updateAssignment(assignment);
}
catch (final Exception e) {
getSession().error(MessageHelper.getString("importExport.error.pointsmodification", assignment.getName()));
GradeImportConfirmationStep.this.errors = true;
errorColumns.add(item);
log.warn("An error occurred updating the assignment", e);
}

assignmentMap.put(StringUtils.trim(assignment.getName()), assignment.getId());
Expand Down

0 comments on commit 926ae50

Please sign in to comment.