Skip to content

Commit

Permalink
SAK-33317 Assignments grade point display improvements (sakaiproject#…
Browse files Browse the repository at this point in the history 
…4802)

- removed FormattedText Cover
- getMaxGradePointDisplay now uses getGradeDisplay
- Added unit tests for getGradeDisplay
  • Loading branch information
@ern
ern authored Sep 15, 2017
1 parent bcd2141 commit 54fa1cc
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 94 deletions.
2 changes: 2 additions & 0 deletions assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,8 @@ public String getDeepLinkWithPermissions(String context, String assignmentId, bo

String getGradeDisplay(String grade, Assignment.GradeType typeOfGrade, Integer scaleFactor);

String getMaxPointGradeDisplay(int factor, int maxGradePoint);

Optional<AssignmentSubmissionSubmitter> getSubmissionSubmittee(AssignmentSubmission submission);

Collection<User> getSubmissionSubmittersAsUsers(AssignmentSubmission submission);
Expand Down
164 changes: 95 additions & 69 deletions assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@
import org.sakaiproject.user.api.User;
import org.sakaiproject.user.api.UserDirectoryService;
import org.sakaiproject.user.api.UserNotDefinedException;
import org.sakaiproject.util.*;
import org.sakaiproject.util.BaseResourceProperties;
import org.sakaiproject.util.ResourceLoader;
import org.sakaiproject.util.SortedIterator;
import org.sakaiproject.util.Validator;
import org.sakaiproject.util.api.FormattedText;
import org.springframework.beans.factory.ObjectFactory;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.Assert;
Expand Down Expand Up @@ -115,6 +119,7 @@ public class AssignmentServiceImpl implements AssignmentService {
@Setter private EmailService emailService;
@Setter private EntityManager entityManager;
@Setter private EventTrackingService eventTrackingService;
@Setter private FormattedText formattedText;
@Setter private FunctionManager functionManager;
@Setter private GradebookExternalAssessmentService gradebookExternalAssessmentService;
@Setter private GradebookService gradebookService;
Expand Down Expand Up @@ -1981,7 +1986,7 @@ public String getDeepLink(String context, String assignmentId) throws Exception
public String getCsvSeparator() {
String defaultSeparator = ",";
//If the decimal separator is a comma
if (",".equals(FormattedText.getDecimalSeparator())) {
if (",".equals(formattedText.getDecimalSeparator())) {
defaultSeparator = ";";
}

Expand Down Expand Up @@ -2085,9 +2090,9 @@ public String getGradeForUserInGradeBook(String assignmentId, String userId) {
String gString = def.getGrade();
if (StringUtils.isNotBlank(gString)) {
try {
String decSeparator = FormattedText.getDecimalSeparator();
String decSeparator = formattedText.getDecimalSeparator();
rv = StringUtils.replace(gString, (",".equals(decSeparator) ? "." : ","), decSeparator);
NumberFormat nbFormat = FormattedText.getNumberFormat(new Double(Math.log10(m.getScaleFactor())).intValue(), new Double(Math.log10(m.getScaleFactor())).intValue(), false);
NumberFormat nbFormat = formattedText.getNumberFormat(new Double(Math.log10(m.getScaleFactor())).intValue(), new Double(Math.log10(m.getScaleFactor())).intValue(), false);
DecimalFormat dcformat = (DecimalFormat) nbFormat;
Double dblGrade = dcformat.parse(rv).doubleValue();
rv = nbFormat.format(dblGrade);
Expand All @@ -2100,77 +2105,98 @@ public String getGradeForUserInGradeBook(String assignmentId, String userId) {
return rv;
}

/**
* Contains logic to consistently output a String based version of a grade
* Interprets the grade using the scale for display
*
* This should probably be moved to a static utility class - ern
*
* @param grade
* @param typeOfGrade
* @param scaleFactor
* @return
*/
@Override
public String getGradeDisplay(String grade, Assignment.GradeType typeOfGrade, Integer scaleFactor) {
if (StringUtils.isBlank(grade)) return null;

if (typeOfGrade == Assignment.GradeType.SCORE_GRADE_TYPE) {
if (grade != null && grade.length() > 0 && !"0".equals(grade)) {
int dec = new Double(Math.log10(scaleFactor)).intValue();
String decSeparator = FormattedText.getDecimalSeparator();
String decimalGradePoint = "";
try {
Integer.parseInt(grade);
// if point grade, display the grade with factor decimal place
int length = grade.length();
if (length > dec) {
decimalGradePoint = grade.substring(0, grade.length() - dec) + decSeparator + grade.substring(grade.length() - dec);
} else {
String newGrade = "0".concat(decSeparator);
for (int i = length; i < dec; i++) {
newGrade = newGrade.concat("0");
String returnGrade = StringUtils.trimToEmpty(grade);

switch (typeOfGrade) {
case SCORE_GRADE_TYPE:
if (!grade.isEmpty() && !"0".equals(grade)) {
int dec = new Double(Math.log10(scaleFactor)).intValue();
String decSeparator = formattedText.getDecimalSeparator();
String decimalGradePoint = null;
try {
Integer.parseInt(grade);
// if point grade, display the grade with factor decimal place
if (grade.length() > dec) {
decimalGradePoint = grade.substring(0, grade.length() - dec) + decSeparator + grade.substring(grade.length() - dec);
} else {
String newGrade = "0".concat(decSeparator);
for (int i = grade.length(); i < dec; i++) {
newGrade = newGrade.concat("0");
}
decimalGradePoint = newGrade.concat(grade);
}
} catch (NumberFormatException nfe1) {
log.debug("Could not parse grade [{}] as an Integer trying as a Float, {}", grade, nfe1.getMessage());
try {
Float.parseFloat(grade);
decimalGradePoint = grade;
} catch (NumberFormatException nfe2) {
log.debug("Could not parse grade [{}] as a Float, {}", grade, nfe2.getMessage());
}
decimalGradePoint = newGrade.concat(grade);
}
} catch (NumberFormatException e) {
// get localized number format
NumberFormat nbFormat = formattedText.getNumberFormat(dec, dec, false);
DecimalFormat dcformat = (DecimalFormat) nbFormat;
// show grade in localized number format
try {
Float.parseFloat(grade);
decimalGradePoint = grade;
} catch (Exception e1) {
return grade;
Double dblGrade = dcformat.parse(decimalGradePoint).doubleValue();
decimalGradePoint = nbFormat.format(dblGrade);
returnGrade = decimalGradePoint;
} catch (Exception e) {
log.warn("Could not parse grade [{}], {}", grade, e.getMessage());
}
}
// get localized number format
NumberFormat nbFormat = FormattedText.getNumberFormat(dec, dec, false);
DecimalFormat dcformat = (DecimalFormat) nbFormat;
// show grade in localized number format
try {
Double dblGrade = dcformat.parse(decimalGradePoint).doubleValue();
decimalGradePoint = nbFormat.format(dblGrade);
} catch (Exception e) {
return grade;
break;
case UNGRADED_GRADE_TYPE:
returnGrade = "";
if (grade.equalsIgnoreCase("gen.nograd")) {
returnGrade = resourceLoader.getString("gen.nograd");
}
break;
case PASS_FAIL_GRADE_TYPE:
returnGrade = resourceLoader.getString("ungra");
if (grade.equalsIgnoreCase("Pass")) {
returnGrade = resourceLoader.getString("pass");
} else if (grade.equalsIgnoreCase("Fail")) {
returnGrade = resourceLoader.getString("fail");
}
break;
case CHECK_GRADE_TYPE:
returnGrade = resourceLoader.getString("ungra");
if (grade.equalsIgnoreCase("Checked")) {
returnGrade = resourceLoader.getString("gen.checked");
}
break;
default:
if (grade.isEmpty()) {
returnGrade = resourceLoader.getString("ungra");
}
return decimalGradePoint;
} else {
return StringUtils.trimToEmpty(grade);
}
} else if (typeOfGrade == Assignment.GradeType.UNGRADED_GRADE_TYPE) {
String ret = "";
if (grade != null) {
if (grade.equalsIgnoreCase("gen.nograd")) ret = resourceLoader.getString("gen.nograd");
}
return ret;
} else if (typeOfGrade == Assignment.GradeType.PASS_FAIL_GRADE_TYPE) {
String ret = resourceLoader.getString("ungra");
if (grade != null) {
if (grade.equalsIgnoreCase("Pass")) ret = resourceLoader.getString("pass");
else if (grade.equalsIgnoreCase("Fail")) ret = resourceLoader.getString("fail");
}
return ret;
} else if (typeOfGrade == Assignment.GradeType.CHECK_GRADE_TYPE) {
String ret = resourceLoader.getString("ungra");
if (grade != null) {
if (grade.equalsIgnoreCase("Checked")) ret = resourceLoader.getString("gen.checked");
}
return ret;
} else {
if (grade != null && grade.length() > 0) {
return StringUtils.trimToEmpty(grade);
} else {
// return "ungraded" in stead
return resourceLoader.getString("ungra");
}
}
return returnGrade;
}

@Override
public String getMaxPointGradeDisplay(int factor, int maxGradePoint) {
// formated to show factor decimal places, for example, 1000 to 100.0
// get localized number format
// NumberFormat nbFormat = FormattedText.getNumberFormat((int)Math.log10(factor),(int)Math.log10(factor),false);
// show grade in localized number format
// Double dblGrade = new Double(maxGradePoint/(double)factor);
// return nbFormat.format(dblGrade);
return getGradeDisplay(Integer.toString(maxGradePoint), Assignment.GradeType.SCORE_GRADE_TYPE, factor);
}

@Override
Expand Down Expand Up @@ -2680,7 +2706,7 @@ private void zipSubmissions(String assignmentReference, String assignmentTitle,
// the comments.txt file to show instructor's comments
ZipEntry textEntry = new ZipEntry(submittersName + "comments" + AssignmentConstants.ZIP_COMMENT_FILE_TYPE);
out.putNextEntry(textEntry);
byte[] b = FormattedText.encodeUnicode(s.getFeedbackComment()).getBytes();
byte[] b = formattedText.encodeUnicode(s.getFeedbackComment()).getBytes();
out.write(b);
textEntry.setSize(b.length);
out.closeEntry();
Expand Down Expand Up @@ -2910,7 +2936,7 @@ protected void zipGroupSubmissions(String assignmentReference, String assignment
// the comments.txt file to show instructor's comments
ZipEntry textEntry = new ZipEntry(submittersName + "comments" + AssignmentConstants.ZIP_COMMENT_FILE_TYPE);
out.putNextEntry(textEntry);
byte[] b = FormattedText.encodeUnicode(s.getFeedbackComment()).getBytes();
byte[] b = formattedText.encodeUnicode(s.getFeedbackComment()).getBytes();
out.write(b);
textEntry.setSize(b.length);
out.closeEntry();
Expand All @@ -2930,7 +2956,7 @@ protected void zipGroupSubmissions(String assignmentReference, String assignment
// the comments.txt file to show instructor's comments
ZipEntry textEntry = new ZipEntry(submittersName + "members" + AssignmentConstants.ZIP_COMMENT_FILE_TYPE);
out.putNextEntry(textEntry);
byte[] b = FormattedText.encodeUnicode(submittersString).getBytes();
byte[] b = formattedText.encodeUnicode(submittersString).getBytes();
out.write(b);
textEntry.setSize(b.length);
out.closeEntry();
Expand Down
58 changes: 54 additions & 4 deletions assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.text.DecimalFormat;
import java.text.DecimalFormatSymbols;
import java.text.NumberFormat;
import java.time.Instant;
import java.util.Collection;
import java.util.Date;
import java.util.Set;
import java.util.UUID;
import java.util.*;

import com.github.javafaker.Faker;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -56,6 +56,7 @@
import org.sakaiproject.user.api.UserDirectoryService;
import org.sakaiproject.user.api.UserNotDefinedException;
import org.sakaiproject.util.ResourceLoader;
import org.sakaiproject.util.api.FormattedText;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests;
Expand All @@ -75,6 +76,7 @@ public class AssignmentServiceTest extends AbstractTransactionalJUnit4SpringCont
@Autowired private ServerConfigurationService serverConfigurationService;
@Autowired private UserDirectoryService userDirectoryService;
@Autowired private SiteService siteService;
@Autowired private FormattedText formattedText;

private ResourceLoader resourceLoader;

Expand All @@ -85,6 +87,11 @@ public void setUp() {
when(resourceLoader.getString("gen.inpro")).thenReturn("In progress");
when(resourceLoader.getString("gen.dra2")).thenReturn("Draft -");
when(resourceLoader.getString("gen.subm4")).thenReturn("Submitted");
when(resourceLoader.getString("gen.nograd")).thenReturn("No Grade");
when(resourceLoader.getString("ungra")).thenReturn("Ungraded");
when(resourceLoader.getString("pass")).thenReturn("Pass");
when(resourceLoader.getString("fail")).thenReturn("Fail");
when(resourceLoader.getString("gen.checked")).thenReturn("Checked");
((AssignmentServiceImpl) assignmentService).setResourceLoader(resourceLoader);
}

Expand Down Expand Up @@ -328,6 +335,40 @@ public void submissionStatus() {
}
}

@Test
public void gradeDisplay() {
Character ds = DecimalFormatSymbols.getInstance().getDecimalSeparator();
when(formattedText.getDecimalSeparator()).thenReturn(ds.toString());

Assert.assertEquals("0", assignmentService.getGradeDisplay("0", Assignment.GradeType.SCORE_GRADE_TYPE, null));

configureScale(10);
Assert.assertEquals("0.5", assignmentService.getGradeDisplay("5", Assignment.GradeType.SCORE_GRADE_TYPE, 10));
Assert.assertEquals("10.0", assignmentService.getGradeDisplay("100", Assignment.GradeType.SCORE_GRADE_TYPE, 10));

configureScale(100);
Assert.assertEquals("0.05", assignmentService.getGradeDisplay("5", Assignment.GradeType.SCORE_GRADE_TYPE, 100));
Assert.assertEquals("5.00", assignmentService.getGradeDisplay("500", Assignment.GradeType.SCORE_GRADE_TYPE, 100));
Assert.assertEquals("100.00", assignmentService.getGradeDisplay("10000", Assignment.GradeType.SCORE_GRADE_TYPE, 100));

configureScale(1000);
Assert.assertEquals("0.005", assignmentService.getGradeDisplay("5", Assignment.GradeType.SCORE_GRADE_TYPE, 1000));
Assert.assertEquals("50.000", assignmentService.getGradeDisplay("50000", Assignment.GradeType.SCORE_GRADE_TYPE, 1000));

Assert.assertEquals("", assignmentService.getGradeDisplay("", Assignment.GradeType.UNGRADED_GRADE_TYPE, null));
Assert.assertEquals("No Grade", assignmentService.getGradeDisplay("gen.nograd", Assignment.GradeType.UNGRADED_GRADE_TYPE, null));

Assert.assertEquals("Pass", assignmentService.getGradeDisplay("pass", Assignment.GradeType.PASS_FAIL_GRADE_TYPE, null));
Assert.assertEquals("Fail", assignmentService.getGradeDisplay("fail", Assignment.GradeType.PASS_FAIL_GRADE_TYPE, null));
Assert.assertEquals("Ungraded", assignmentService.getGradeDisplay("any", Assignment.GradeType.PASS_FAIL_GRADE_TYPE, null));

Assert.assertEquals("Ungraded", assignmentService.getGradeDisplay("any", Assignment.GradeType.CHECK_GRADE_TYPE, null));
Assert.assertEquals("Checked", assignmentService.getGradeDisplay("checked", Assignment.GradeType.CHECK_GRADE_TYPE, null));

Assert.assertEquals("Ungraded", assignmentService.getGradeDisplay("", Assignment.GradeType.GRADE_TYPE_NONE, null));
Assert.assertEquals("self", assignmentService.getGradeDisplay("self", Assignment.GradeType.GRADE_TYPE_NONE, null));
}

private AssignmentSubmission createNewSubmission(String context, String submitterId) throws UserNotDefinedException {
Assignment assignment = createNewAssignment(context);
User userMock = Mockito.mock(User.class);
Expand All @@ -354,4 +395,13 @@ private Assignment createNewAssignment(String context) {
}
return assignment;
}

private void configureScale(int scale) {
int dec = new Double(Math.log10(scale)).intValue();
NumberFormat nf = NumberFormat.getInstance();
nf.setMaximumFractionDigits(dec);
nf.setMinimumFractionDigits(dec);
nf.setGroupingUsed(false);
when(formattedText.getNumberFormat(dec, dec, false)).thenReturn(nf);
}
}
8 changes: 7 additions & 1 deletion assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTestConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.sakaiproject.tool.api.ToolManager;
import org.sakaiproject.user.api.UserDirectoryService;
import org.sakaiproject.util.ResourceLoader;
import org.sakaiproject.util.api.FormattedText;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand Down Expand Up @@ -179,6 +180,11 @@ public EmailService emailService() {
return mock(EmailService.class);
}

@Bean(name = "org.sakaiproject.util.api.FormattedText")
public FormattedText formattedText() {
return mock(FormattedText.class);
}

@Bean(name = "org.sakaiproject.authz.api.FunctionManager")
public FunctionManager functionManager() {
return mock(FunctionManager.class);
Expand Down Expand Up @@ -243,4 +249,4 @@ public ScheduledInvocationManager scheduledInvocationManager() {
public ContentReviewService contentReviewService() {
return mock(ContentReviewService.class);
}
}
}
Loading

0 comments on commit 54fa1cc

Please sign in to comment.