Skip to content

Commit

Permalink
Fix bug where import was failing when more than one item was being im…
Browse files Browse the repository at this point in the history 
…ported (sakaiproject#3178)

* #3153 move parser to helper and clean up the maps and lists in the processor

* #3153 fix bug where two new assignments were overwriting each other in the model. This was completely replacing the data rather than adding to it, which then causes failures downstream. Added some more doco.

* #3153 Update tests
  • Loading branch information
@steveswinsburg @payten
steveswinsburg authored and payten committed Aug 15, 2016
1 parent b6ffa1e commit 25de044
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 79 deletions.
60 changes: 46 additions & 14 deletions gradebookng/tool/src/java/org/sakaiproject/gradebookng/business/util/ImportGradesHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import org.apache.commons.lang.ArrayUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.math.NumberUtils;
import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
Expand All @@ -22,6 +23,7 @@
import org.apache.poi.ss.usermodel.WorkbookFactory;
import org.sakaiproject.gradebookng.business.exception.GbImportCommentMissingItemException;
import org.sakaiproject.gradebookng.business.exception.GbImportExportInvalidColumnException;
import org.sakaiproject.gradebookng.business.exception.GbImportExportInvalidFileTypeException;
import org.sakaiproject.gradebookng.business.model.GbGradeInfo;
import org.sakaiproject.gradebookng.business.model.GbStudentGradeInfo;
import org.sakaiproject.gradebookng.business.model.ImportColumn;
Expand Down Expand Up @@ -54,6 +56,37 @@ public class ImportGradesHelper {
final static Pattern POINTS_PATTERN = Pattern.compile("(\\d+)(?=]$)");
final static Pattern IGNORE_PATTERN = Pattern.compile("(\\#.+)");

// list of mimetypes for each category. Must be compatible with the parser
private static final String[] XLS_MIME_TYPES = { "application/vnd.ms-excel", "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" };
private static final String[] CSV_MIME_TYPES = { "text/csv", "text/plain", "text/comma-separated-values", "application/csv" };


/**
* Helper to parse the imported file into an {@link ImportedGradeWrapper} depending on its type
* @param is
* @param mimetype
* @param userMap
* @return
* @throws GbImportExportInvalidColumnException
* @throws GbImportExportInvalidFileTypeException
* @throws IOException
* @throws InvalidFormatException
*/
public static ImportedGradeWrapper parseImportedGradeFile(final InputStream is, final String mimetype, final Map<String, String> userMap) throws GbImportExportInvalidColumnException, GbImportExportInvalidFileTypeException, IOException, InvalidFormatException {

ImportedGradeWrapper rval = null;

// determine file type and delegate
if (ArrayUtils.contains(CSV_MIME_TYPES, mimetype)) {
rval = ImportGradesHelper.parseCsv(is, userMap);
} else if (ArrayUtils.contains(XLS_MIME_TYPES, mimetype)) {
rval = ImportGradesHelper.parseXls(is, userMap);
} else {
throw new GbImportExportInvalidFileTypeException("Invalid file type for grade import: " + mimetype);
}
return rval;
}

/**
* Parse a CSV into a list of ImportedGrade objects. Returns list if ok, or null if error
*
Expand All @@ -62,7 +95,7 @@ public class ImportGradesHelper {
* @throws IOException
* @throws GbImportExportInvalidColumnException
*/
public static ImportedGradeWrapper parseCsv(final InputStream is, final Map<String, String> userMap) throws GbImportExportInvalidColumnException, IOException {
private static ImportedGradeWrapper parseCsv(final InputStream is, final Map<String, String> userMap) throws GbImportExportInvalidColumnException, IOException {

// manually parse method so we can support arbitrary columns
final CSVReader reader = new CSVReader(new InputStreamReader(is));
Expand Down Expand Up @@ -107,7 +140,7 @@ public static ImportedGradeWrapper parseCsv(final InputStream is, final Map<Stri
* @throws InvalidFormatException
* @throws GbImportExportInvalidColumnException
*/
public static ImportedGradeWrapper parseXls(final InputStream is, final Map<String, String> userMap) throws GbImportExportInvalidColumnException, InvalidFormatException, IOException {
private static ImportedGradeWrapper parseXls(final InputStream is, final Map<String, String> userMap) throws GbImportExportInvalidColumnException, InvalidFormatException, IOException {

int lineCount = 0;
final List<ImportedGrade> list = new ArrayList<ImportedGrade>();
Expand Down Expand Up @@ -214,31 +247,27 @@ private static ImportedGrade mapLine(final String[] line, final Map<Integer, Imp
public static List<ProcessedGradeItem> processImportedGrades(final ImportedGradeWrapper importedGradeWrapper,
final List<Assignment> assignments, final List<GbStudentGradeInfo> currentGrades) {

//setup
final List<ProcessedGradeItem> processedGradeItems = new ArrayList<>();
final Map<String, Assignment> assignmentNameMap = new HashMap<>();
// setup
// TODO this will ensure dupes can't be added. Provide a report to the user that dupes were added. There would need to be a step before this though
final Map<String, ProcessedGradeItem> assignmentProcessedGradeItemMap = new HashMap<>();

// process grades
final Map<Long, AssignmentStudentGradeInfo> transformedGradeMap = transformCurrentGrades(currentGrades);


// Map the assignment name back to the Id
for (final Assignment assignment : assignments) {
assignmentNameMap.put(assignment.getName(), assignment);
}
// Map assignment name to assignment
final Map<String, Assignment> assignmentNameMap = assignments.stream().collect(Collectors.toMap(Assignment::getName, a -> a));

//for every column, setup the data
for (final ImportColumn column : importedGradeWrapper.getColumns()) {
boolean needsAdded = false;
boolean needsToBeAdded = false;

final String columnTitle = StringUtils.trim(column.getColumnTitle()); // trim whitespace so we can match properly

//setup a new one unless it already exists (ie there were duplicate columns)
ProcessedGradeItem processedGradeItem = assignmentProcessedGradeItemMap.get(columnTitle);
if (processedGradeItem == null) {
processedGradeItem = new ProcessedGradeItem();
needsAdded = true;
needsToBeAdded = true;

//default to gb_item
//overridden if a comment type
Expand Down Expand Up @@ -293,12 +322,15 @@ public static List<ProcessedGradeItem> processImportedGrades(final ImportedGrade
}
processedGradeItem.setProcessedGradeItemDetails(processedGradeItemDetails);

if (needsAdded) {
processedGradeItems.add(processedGradeItem);
// add to list
if (needsToBeAdded) {
assignmentProcessedGradeItemMap.put(columnTitle, processedGradeItem);
}
}

// get just a list
final List<ProcessedGradeItem> processedGradeItems = new ArrayList<>(assignmentProcessedGradeItemMap.values());

// comment columns must have an associated gb item column
// this ensures we have a processed grade item for each one
final List<ProcessedGradeItem> commentColumns = processedGradeItems.stream().filter(p -> p.getType() == ProcessedGradeItem.Type.COMMENT).collect(Collectors.toList());
Expand Down
11 changes: 2 additions & 9 deletions ...l/src/java/org/sakaiproject/gradebookng/tool/panels/importExport/CreateGradeItemStep.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
package org.sakaiproject.gradebookng.tool.panels.importExport;

import java.util.ArrayList;
import java.util.List;

import org.apache.commons.lang.StringUtils;
import org.apache.wicket.Component;
import org.apache.wicket.markup.html.basic.Label;
Expand Down Expand Up @@ -65,13 +62,11 @@ public void onInitialize() {

@Override
protected void onSubmit() {
final List<Assignment> assignmentsToCreate = new ArrayList<Assignment>();

final Assignment a = (Assignment)getDefaultModel().getObject();

if (a != null) {
assignmentsToCreate.add(assignment);
}
//add to model
importWizardModel.getAssignmentsToCreate().add(a);

log.debug("Assignment: " + assignment);

Expand All @@ -82,8 +77,6 @@ protected void onSubmit() {
//Figure out if there are more steps
//If so, go to the next step (ie do it all over again)
Component newPanel = null;
importWizardModel.setAssignmentsToCreate(assignmentsToCreate);

if (step < importWizardModel.getTotalSteps()) {
importWizardModel.setStep(step+1);
newPanel = new CreateGradeItemStep(CreateGradeItemStep.this.panelId, Model.of(importWizardModel));
Expand Down
4 changes: 3 additions & 1 deletion ...va/org/sakaiproject/gradebookng/tool/panels/importExport/GradeImportConfirmationStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,17 @@ protected void onSubmit() {
log.debug("Looping through detail items to save");

//get data
// if its an update/modify, this will get the id
Long assignmentId = processedGradeItem.getItemId();

//if assignment title was modified, we need to use that instead
final String assignmentTitle = StringUtils.trim((processedGradeItem.getAssignmentTitle() != null) ? processedGradeItem.getAssignmentTitle() : processedGradeItem.getItemTitle());

// a newly created assignment will have a null ID here and need a lookup from the map to get the ID
if (assignmentId == null) {
// Should be a newly created GB item
assignmentId = assignmentMap.get(assignmentTitle);
}
//TODO if assignmentId is still null, there will be a problem

final GradeSaveResponse saveResponse = GradeImportConfirmationStep.this.businessService.saveGrade(assignmentId,
processedGradeItemDetail.getStudentUuid(),
Expand Down
33 changes: 1 addition & 32 deletions ...src/java/org/sakaiproject/gradebookng/tool/panels/importExport/GradeImportUploadStep.java
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package org.sakaiproject.gradebookng.tool.panels.importExport;

import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import org.apache.commons.lang.ArrayUtils;
import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
import org.apache.wicket.Component;
import org.apache.wicket.markup.html.form.Button;
Expand Down Expand Up @@ -41,10 +39,6 @@ public class GradeImportUploadStep extends Panel {

private static final long serialVersionUID = 1L;

// list of mimetypes for each category. Must be compatible with the parser
private static final String[] XLS_MIME_TYPES = { "application/vnd.ms-excel", "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" };
private static final String[] CSV_MIME_TYPES = { "text/csv", "text/plain", "text/comma-separated-values", "application/csv" };

private final String panelId;

@SpringBean(name = "org.sakaiproject.gradebookng.business.GradebookNgBusinessService")
Expand Down Expand Up @@ -105,7 +99,7 @@ public void onSubmit() {
// turn file into list
ImportedGradeWrapper importedGradeWrapper = null;
try {
importedGradeWrapper = parseImportedGradeFile(upload.getInputStream(), upload.getContentType(), userMap);
importedGradeWrapper = ImportGradesHelper.parseImportedGradeFile(upload.getInputStream(), upload.getContentType(), userMap);
} catch (final GbImportExportInvalidColumnException e) {
error(getString("importExport.error.incorrectformat"));
return;
Expand Down Expand Up @@ -176,29 +170,4 @@ private Map<String, String> getUserMap() {
return rval;
}

/**
* Helper to parse the imported file into an {@link ImportedGradeWrapper} depending on its type
* @param is
* @param mimetype
* @param userMap
* @return
* @throws GbImportExportInvalidColumnException
* @throws GbImportExportInvalidFileTypeException
* @throws IOException
* @throws InvalidFormatException
*/
private ImportedGradeWrapper parseImportedGradeFile(final InputStream is, final String mimetype, final Map<String, String> userMap) throws GbImportExportInvalidColumnException, GbImportExportInvalidFileTypeException, IOException, InvalidFormatException {

ImportedGradeWrapper rval = null;

// determine file type and delegate
if (ArrayUtils.contains(CSV_MIME_TYPES, mimetype)) {
rval = ImportGradesHelper.parseCsv(is, userMap);
} else if (ArrayUtils.contains(XLS_MIME_TYPES, mimetype)) {
rval = ImportGradesHelper.parseXls(is, userMap);
} else {
throw new GbImportExportInvalidFileTypeException("Invalid file type for grade import: " + mimetype);
}
return rval;
}
}
Loading

0 comments on commit 25de044

Please sign in to comment.