From d9d2d54e4bf4a829d3acca494d7c32ecf8144853 Mon Sep 17 00:00:00 2001 From: Ryan Kuang Date: Wed, 15 Aug 2018 16:57:42 -0500 Subject: [PATCH] Add custom columns to gradebook imports When importing a gradebook, the csv import can include custom columns closes GRADE-1315 Test Plan - Create a course with (not read_only) custom columns in the gradebook - Export the gradebook to csv - Make changes to the data in the writable custom columns in the csv - Import the csv into gradebook - Run the delayed jobs server - The gradebook will make changes in writable custom columns with or without additional changes in assignment columns - If no changes are made, or changes in non writable columns are made, the importer will skip these changes Change-Id: I0a529ea8727be864f6105b9e6b256f75e1c8c471 Reviewed-on: https://gerrit.instructure.com/156068 Tested-by: Jenkins Reviewed-by: Keith T. Garner Reviewed-by: Adrian Packel QA-Review: Adrian Packel Reviewed-by: Derek Bender Product-Review: Keith T. Garner --- .../gradebook_uploads_controller.rb | 3 + .../uploads/process_gradebook_upload.js | 51 ++++++- app/views/gradebook_uploads/show.html.erb | 4 + lib/gradebook_importer.rb | 124 ++++++++++++------ public/javascripts/gradebook_uploads.js | 77 +++++++++-- .../vendor/slickgrid/slick.editors.js | 14 +- .../gradebook/ProcessGradebookUploadSpec.js | 115 +++++++++++++++- spec/javascripts/jsx/gradebook_uploadsSpec.js | 4 +- spec/lib/gradebook_importer_spec.rb | 121 ++++++++++++----- 9 files changed, 421 insertions(+), 92 deletions(-) diff --git a/app/controllers/gradebook_uploads_controller.rb b/app/controllers/gradebook_uploads_controller.rb index 46baaa54376..a64a0b632c0 100644 --- a/app/controllers/gradebook_uploads_controller.rb +++ b/app/controllers/gradebook_uploads_controller.rb @@ -77,12 +77,15 @@ class GradebookUploadsController < ApplicationController end private + def gradebook_env(progress) { + course_id: @context.id, progress: progress_json(progress, @current_user, session), uploaded_gradebook_data_path: "/courses/#{@context.id}/gradebook_upload/data", gradebook_path: course_gradebook_path(@context), bulk_update_path: "/api/v1/courses/#{@context.id}/submissions/update_grades", + bulk_update_custom_columns_path: api_v1_course_custom_gradebook_column_bulk_data_path(@context), create_assignment_path: api_v1_course_assignments_path(@context), new_gradebook_upload_path: new_course_gradebook_upload_path(@context), } diff --git a/app/jsx/gradebook/uploads/process_gradebook_upload.js b/app/jsx/gradebook/uploads/process_gradebook_upload.js index bc1a5af7273..50db16821e9 100644 --- a/app/jsx/gradebook/uploads/process_gradebook_upload.js +++ b/app/jsx/gradebook/uploads/process_gradebook_upload.js @@ -30,9 +30,12 @@ import 'jquery.ajaxJSON' const ProcessGradebookUpload = { upload (gradebook) { - if (gradebook != null && _.isArray(gradebook.assignments) && _.isArray(gradebook.students)) { - const createAssignmentsResponses = this.createAssignments(gradebook); + if (gradebook != null && (_.isArray(gradebook.assignments) || _.isArray(gradebook.custom_columns)) && _.isArray(gradebook.students)) { + if (gradebook.custom_columns) { + this.uploadCustomColumnData(gradebook); + } + const createAssignmentsResponses = this.createAssignments(gradebook); return $.when(...createAssignmentsResponses).then((...responses) => { this.uploadGradeData(gradebook, responses); }); @@ -40,6 +43,50 @@ import 'jquery.ajaxJSON' return undefined; }, + uploadCustomColumnData (gradebook) { + const customColumnData = gradebook.students.reduce((accumulator, student) => { + const student_id = Number.parseInt(student.id, 10); + if (!(student_id in accumulator)) { + accumulator[student_id] = student.custom_column_data // eslint-disable-line no-param-reassign + } + return accumulator; + }, {}); + + if (!_.isEmpty(customColumnData)) { + this.parseCustomColumnData(customColumnData); + } + + if (!gradebook.assignments.length) { + alert(successMessage); // eslint-disable-line no-alert + this.goToGradebook(); + } + }, + + parseCustomColumnData (customColumnData) { + const data = []; + Object.keys(customColumnData).forEach(studentId => { + customColumnData[studentId].forEach((column) => { + data.push({ + column_id: Number.parseInt(column.column_id, 10), + user_id: studentId, + content: column.new_content + }) + }); + }) + + this.submitCustomColumnData(data); + return data; + }, + + submitCustomColumnData (data) { + return $.ajaxJSON(ENV.bulk_update_custom_columns_path, + 'PUT', + JSON.stringify({column_data: data}), + null, + null, + {contentType: 'application/json'}); + }, + createAssignments (gradebook) { const newAssignments = this.getNewAssignmentsFromGradebook(gradebook); return newAssignments.map(assignment => this.createIndividualAssignment(assignment)); diff --git a/app/views/gradebook_uploads/show.html.erb b/app/views/gradebook_uploads/show.html.erb index 626f6622830..48b7a1f43fb 100644 --- a/app/views/gradebook_uploads/show.html.erb +++ b/app/views/gradebook_uploads/show.html.erb @@ -115,6 +115,10 @@

<%= t("Some submissions are not gradeable; grade changes for those submissions have been ignored.") %>

+ + diff --git a/lib/gradebook_importer.rb b/lib/gradebook_importer.rb index 07dd2fdac3a..4d0db783947 100644 --- a/lib/gradebook_importer.rb +++ b/lib/gradebook_importer.rb @@ -110,15 +110,17 @@ class GradebookImporter @pp_row = [] @warning_messages = { prevented_new_assignment_creation_in_closed_period: false, - prevented_grading_ungradeable_submission: false + prevented_grading_ungradeable_submission: false, + prevented_changing_read_only_column: false } @custom_column_titles_and_indexes = [] + @gradebook_importer_assignments = {} + @gradebook_importer_custom_columns = {} begin csv_stream do |row| already_processed = check_for_non_student_row(row) unless already_processed - row = process_custom_column(row) @students << process_student(row) process_submissions(row, @students.last) end @@ -170,7 +172,7 @@ class GradebookImporter end @students.each do |student| - student.gradebook_importer_submissions.each do |submission| + @gradebook_importer_assignments[student.id].each do |submission| submission_assignment_id = submission.fetch('assignment_id').to_i assignment = original_submissions_by_student. fetch(student.id, {}). @@ -192,9 +194,14 @@ class GradebookImporter ) end end + @gradebook_importer_custom_columns[student.id].each do |student_custom_column_cell| + custom_column = custom_gradebook_columns.detect {|custom_col| custom_col.id == student_custom_column_cell['column_id']} + datum = custom_column.custom_gradebook_column_data.detect {|custom_column_datum| custom_column_datum.user_id == student.id} + student_custom_column_cell['current_content'] = datum&.content + end end - translate_pass_fail(@assignments, @students) + translate_pass_fail(@assignments, @students, @gradebook_importer_assignments) unless @missing_student # weed out assignments with no changes @@ -202,36 +209,56 @@ class GradebookImporter @assignments.each_with_index do |assignment, idx| next if assignment.changed? && !readonly_assignment?(idx) indexes_to_delete << idx if readonly_assignment?(idx) || @students.all? do |student| - submission = student.gradebook_importer_submissions[idx] + submission = @gradebook_importer_assignments[student.id][idx] # Have potentially mixed case excused in grade match case # expectations for the compare so it doesn't look changed - submission['grade'] = 'EX' if submission['grade'].to_s.upcase == 'EX' + submission['grade'] = 'EX' if submission['grade'].to_s.casecmp('EX') == 0 no_change = submission['grade'] == submission['original_grade'] || (submission['original_grade'].present? && submission['grade'].present? && submission['original_grade'].to_f == submission['grade'].to_f) || (submission['original_grade'].blank? && submission['grade'].blank?) - if !submission['gradeable'] && !no_change - @warning_messages[:prevented_grading_ungradeable_submission] = true - end + @warning_messages[:prevented_grading_ungradeable_submission] = true if !submission['gradeable'] && !no_change no_change || !submission['gradeable'] end end - indexes_to_delete.reverse_each do |idx| - @assignments.delete_at(idx) + custom_column_ids_to_skip_on_import = [] + custom_gradebook_columns.each_with_index do |custom_column, index| + exclude_custom_column_on_import = @students.all? do |student| + custom_column_datum = @gradebook_importer_custom_columns[student.id][index] + no_change = (custom_column_datum['current_content'] == custom_column_datum['new_content']) || + (custom_column_datum['current_content'].blank? && custom_column_datum['new_content'].blank?) + + @warning_messages[:prevented_changing_read_only_column] = true if !no_change && custom_column.read_only + + no_change + end + + custom_column_ids_to_skip_on_import << custom_column.id if exclude_custom_column_on_import || custom_column.read_only + end + + indexes_to_delete.reverse_each do |index| + @assignments.delete_at(index) @students.each do |student| - student.gradebook_importer_submissions.delete_at(idx) + @gradebook_importer_assignments[student.id].delete_at(index) + end + end + + @custom_gradebook_columns = @custom_gradebook_columns.reject { |custom_col| custom_column_ids_to_skip_on_import.include?(custom_col.id) } + @students.each do |student| + @gradebook_importer_custom_columns[student.id] = @gradebook_importer_custom_columns[student.id].reject do |custom_col| + custom_column_ids_to_skip_on_import.include?(custom_col.fetch('column_id')) end end @students.each do |student| - student.gradebook_importer_submissions.select! { |sub| sub['gradeable'] } + @gradebook_importer_assignments[student.id].select! { |sub| sub['gradeable'] } end @unchanged_assignments = !indexes_to_delete.empty? - @students = [] if @assignments.empty? + @students = [] if @assignments.empty? && @custom_gradebook_columns.empty? end # remove concluded enrollments @@ -256,33 +283,25 @@ class GradebookImporter @upload.save! end - def process_custom_column(row) - @custom_column_titles_and_indexes.each do |cc| - row.delete(cc.fetch(:index)) - end - row - end - - def translate_pass_fail(assignments, students) + def translate_pass_fail(assignments, students, gradebook_importer_assignments) assignments.each_with_index do |assignment, idx| next unless assignment.grading_type == "pass_fail" students.each do |student| - submission = student.gradebook_importer_submissions[idx] + submission = gradebook_importer_assignments.fetch(student.id)[idx] if submission['grade'].present? - submission['grade'] = assignment.score_to_grade(submission['grade'], - submission['grade']) + gradebook_importer_assignments.fetch(student.id)[idx]['grade'] = assignment.score_to_grade(submission['grade'], \ + submission['grade']) end if submission['original_grade'].present? - submission['original_grade'] = - assignment.score_to_grade(submission['original_grade'], - submission['original_grade']) + gradebook_importer_assignments.fetch(student.id)[idx]['original_grade'] = assignment.score_to_grade(submission['original_grade'], + submission['original_grade']) end end end end def process_custom_columns_headers(row) - custom_columns = @context.custom_gradebook_columns.to_a + custom_columns = custom_gradebook_columns row.each_with_index do |header_column, index| cc = custom_columns.detect { |column| column.title == header_column } if cc.present? @@ -441,11 +460,26 @@ class GradebookImporter end new_submission = { 'grade' => grade, - 'assignment_id' => assignment_id + 'assignment_id' => assignment_id, } importer_submissions << new_submission end - student.gradebook_importer_submissions = importer_submissions + + # custom columns + first_custom_column_index = @student_columns - custom_gradebook_columns.length + importer_custom_columns = [] + + custom_gradebook_columns.each_with_index do |cc, index| + col_index = index + first_custom_column_index + new_custom_column_data = { + 'new_content' => row[col_index], + 'column_id' => cc.id, + } + importer_custom_columns << new_custom_column_data + end + + @gradebook_importer_custom_columns[student.id] = importer_custom_columns + @gradebook_importer_assignments[student.id] = importer_submissions end def assignment_visible_to_student(student, assignment, assignment_id, visible_assignments) @@ -466,12 +500,17 @@ class GradebookImporter }, :original_submissions => @original_submissions, :unchanged_assignments => @unchanged_assignments, - :warning_messages => @warning_messages + :warning_messages => @warning_messages, + :custom_columns => custom_gradebook_columns.map { |cc| custom_columns_to_hash(cc) }, } end protected + def custom_gradebook_columns + @custom_gradebook_columns ||= @context.custom_gradebook_columns.to_a + end + def identify_delimiter(rows) field_counts = {} %w[; ,].each do |separator| @@ -559,13 +598,22 @@ class GradebookImporter end end - def student_to_hash(user) + def custom_columns_to_hash(cc) { - :last_name_first => user.last_name_first, - :name => user.name, - :previous_id => user.previous_id, - :id => user.id, - :submissions => user.gradebook_importer_submissions + id: cc.id, + title: cc.title, + read_only: cc.read_only, + } + end + + def student_to_hash(student) + { + :last_name_first => student.last_name_first, + :name => student.name, + :previous_id => student.previous_id, + :id => student.id, + :submissions => @gradebook_importer_assignments[student.id], + :custom_column_data => @gradebook_importer_custom_columns[student.id] } end diff --git a/public/javascripts/gradebook_uploads.js b/public/javascripts/gradebook_uploads.js index e5115f4d59a..33f350db263 100644 --- a/public/javascripts/gradebook_uploads.js +++ b/public/javascripts/gradebook_uploads.js @@ -91,6 +91,7 @@ import './jquery.templateData' /* fillTemplateData */ $.each(uploadedGradebook.assignments, function(){ var newGrade = { id: this.id, + type: 'assignments', name: htmlEscape(I18n.t('To')), field: this.id, width: 125, @@ -98,7 +99,7 @@ import './jquery.templateData' /* fillTemplateData */ formatter: self.createNumberFormatter('grade'), active: true, previous_id: this.previous_id, - cssClass: "new-grade" + cssClass: 'new-grade' }; if (this.grading_type !== 'letter_grade') { @@ -109,10 +110,10 @@ import './jquery.templateData' /* fillTemplateData */ } var conflictingGrade = { - id: this.id + "_conflicting", + id: `${this.id}_conflicting`, width: 125, formatter: self.createNumberFormatter('original_grade'), - field: this.id + "_conflicting", + field: `${this.id}_conflicting`, name: htmlEscape(I18n.t('From')), cssClass: 'conflicting-grade' }; @@ -121,7 +122,7 @@ import './jquery.templateData' /* fillTemplateData */ id: this.id, width: 250, name: htmlEscape(this.title), - headerCssClass: "assignment" + headerCssClass: 'assignment' }; labelData.columns.push(assignmentHeaderColumn); @@ -129,24 +130,68 @@ import './jquery.templateData' /* fillTemplateData */ gridData.columns.push(newGrade); }); + uploadedGradebook.custom_columns.forEach((column) => { + const newCustomColumn = { + id: `custom_col_${column.id}`, + customColumnId: column.id, + type: 'custom_column', + name: htmlEscape(I18n.t('To')), + field: `custom_col_${column.id}`, + width: 125, + editor: Slick.Editors.UploadGradeCellEditor, + formatter: self.createGeneralFormatter('new_content'), + editorFormatter: 'custom_column', + editorParser: 'custom_column', + active: true, + cssClass: 'new-grade' + }; + + const conflictingCustomColumn = { + id: `custom_col_${column.id}_conflicting`, + width: 125, + formatter: self.createGeneralFormatter('current_content'), + field: `custom_col_${column.id}_conflicting`, + name: htmlEscape(I18n.t('From')), + cssClass: 'conflicting-grade' + }; + + const customColumnHeaderColumn = { + id: `custom_col_${column.id}`, + width: 250, + name: htmlEscape(column.title), + headerCssClass: "assignment" + }; + + labelData.columns.push(customColumnHeaderColumn); + gridData.columns.push(conflictingCustomColumn); + gridData.columns.push(newCustomColumn); + }); + $.each(uploadedGradebook.students, function(index){ var row = { student : this, id : this.id }; $.each(this.submissions, function(){ - var originalGrade = parseInt(this.original_grade), - updatedGrade = parseInt(this.grade), - updateWillRemoveGrade = !isNaN(originalGrade) && isNaN(updatedGrade); + const originalGrade = Number.parseInt(this.original_grade, 10); + const updatedGrade = Number.parseInt(this.grade, 10); + const updateWillRemoveGrade = !Number.isNaN(originalGrade) && Number.isNaN(updatedGrade); - if ( (originalGrade > updatedGrade || updateWillRemoveGrade) && - ((this.grade || "").toUpperCase() !== "EX") ) { - rowsToHighlight.push({rowIndex: index, id: this.assignment_id}); - } + if ( (originalGrade > updatedGrade || updateWillRemoveGrade) && + ((this.grade || "").toUpperCase() !== "EX") ) { + rowsToHighlight.push({rowIndex: index, id: this.assignment_id}); + } - row['assignmentId'] = this.assignment_id; - row[this.assignment_id] = this; - row[this.assignment_id + "_conflicting"] = this; + row.assignmentId = this.assignment_id; + row[this.assignment_id] = this; + row[`${this.assignment_id}_conflicting`] = this; + }); + $.each(this.custom_column_data, function(){ + if (this.current_content !== this.new_content) { + rowsToHighlight.push({rowIndex: index, id: `custom_col_${this.column_id}`}); + } + row[`custom_col_${this.column_id}`] = this; + row[`custom_col_${this.column_id}_conflicting`] = this; }); gridData.data.push(row); row.active = true; @@ -204,6 +249,10 @@ import './jquery.templateData' /* fillTemplateData */ if (uploadedGradebook.warning_messages.prevented_grading_ungradeable_submission) { $("#prevented-grading-ungradeable-submission").show(); } + + if (uploadedGradebook.warning_messages.prevented_changing_read_only_column) { + $("#prevented_changing_read_only_column").show(); + } }, handleThingsNeedingToBeResolved: function() { diff --git a/public/javascripts/vendor/slickgrid/slick.editors.js b/public/javascripts/vendor/slickgrid/slick.editors.js index cc2f32d863b..89a767a0f76 100644 --- a/public/javascripts/vendor/slickgrid/slick.editors.js +++ b/public/javascripts/vendor/slickgrid/slick.editors.js @@ -522,7 +522,14 @@ import 'vendor/slickgrid/slick.core' if (columnDef.active) { value = value || {}; var $input; - var defaultValue = value.grade; + let defaultValue; + + if (columnDef.editorFormatter === 'custom_column') { + defaultValue = value.new_content; + } else { + defaultValue = value.grade; + } + var scope = this; this.init = function() { @@ -564,6 +571,9 @@ import 'vendor/slickgrid/slick.core' $input[0].defaultValue = value.grade; $input.val(defaultValue); } + } else if(columnDef.editorFormatter === 'custom_column') { + $input[0].defaultValue = value.new_content; + $input.val(defaultValue); } $input.appendTo($container); @@ -590,6 +600,8 @@ import 'vendor/slickgrid/slick.core' this.applyValue = function(item, state) { if (typeof(columnDef.editorParser) === 'function') { item[columnDef.id].grade = columnDef.editorParser(state); + } else if(columnDef.editorParser === 'custom_column') { + item[columnDef.id].new_content = state; } else { item[columnDef.id].grade = state; } diff --git a/spec/javascripts/jsx/gradebook/ProcessGradebookUploadSpec.js b/spec/javascripts/jsx/gradebook/ProcessGradebookUploadSpec.js index a0c3a68a539..122d0f9c2f6 100644 --- a/spec/javascripts/jsx/gradebook/ProcessGradebookUploadSpec.js +++ b/spec/javascripts/jsx/gradebook/ProcessGradebookUploadSpec.js @@ -43,10 +43,10 @@ define([ const newAssignment2 = {id: -1, title: 'New Assignment 2', points_possible: 25, published: true}; - const submissionNew2Change = {assignment_id: -1, grade: '20', original_grade: '25'}; - const submissionNew2Excused = {assignment_id: -1, grade: 'EX', original_grade: '20'}; + const submissionNew2Change = {assignment_id: -1, grade: '20', original_grade: '25', type: 'assignment'}; + const submissionNew2Excused = {assignment_id: -1, grade: 'EX', original_grade: '20', type: 'assignment'}; - const submissionIgnored = {assignment_id: -2, grade: '25', original_grade: '25'}; + const submissionIgnored = {assignment_id: -2, grade: '25', original_grade: '25', type: 'assignment'}; const createAssignmentResponse1 = {id: 3}; const createAssignmentResponse2 = {id: 4}; @@ -770,4 +770,113 @@ define([ ok(goToGradebookStub.called); }); + + QUnit.module('ProcessGradebookUpload.parseCustomColumnData'); + + test('correctly parses data for one student', () => { + const customColumnData = { + 10: [ + { + new_content: "first content", + column_id: 1 + }, + { + new_content: "second content", + column_id: 3 + }, + ] + } + + const data = ProcessGradebookUpload.parseCustomColumnData(customColumnData) + equal(data.length, 2); + equal(data[0].user_id, 10); + equal(data[0].column_id, 1); + equal(data[0].content, "first content"); + equal(data[1].user_id, 10); + equal(data[1].column_id, 3); + equal(data[1].content, "second content"); + }); + + test('correctly parses data for multiple students', () => { + const customColumnData = { + 10: [ + { + new_content: "first content", + column_id: 1 + }, + ], + 1: [ + { + new_content: "second content", + column_id: 2 + }, + ] + } + + const data = ProcessGradebookUpload.parseCustomColumnData(customColumnData) + equal(data.length, 2); + equal(data[0].user_id, 1); + equal(data[0].column_id, 2); + equal(data[0].content, "second content"); + equal(data[1].user_id, 10); + equal(data[1].column_id, 1); + equal(data[1].content, "first content"); + }); + + QUnit.module('ProcessGradebookUpload.submitCustomColumnData', { + setup () { + sandbox.stub(window, 'alert'); + xhr = sinon.useFakeXMLHttpRequest(); + requests = []; + + xhr.onCreate = function (request) { + requests.push(request); + }; + + goToGradebookStub = sinon.stub(ProcessGradebookUpload, 'goToGradebook'); + + clock = sinon.useFakeTimers(); + + fakeENV.setup(); + ENV.bulk_update_custom_columns_path = '/bulk_update_custom_columns_path/url'; + }, + teardown () { + xhr.restore(); + + ProcessGradebookUpload.goToGradebook.restore(); + + clock.restore(); + + fakeENV.teardown(); + } + }); + + test('correctly submits custom column data', () => { + const gradeData = [ + { + column_id: 1, + user_id: 2, + content: "test content" + }, + { + column_id: 3, + user_id: 4, + content: "test content 2" + } + ]; + + ProcessGradebookUpload.submitCustomColumnData(gradeData); + + equal(requests.length, 1); + equal(requests[0].url, '/bulk_update_custom_columns_path/url'); + equal(requests[0].method, 'PUT'); + + const bulkUpdateRequest = JSON.parse(requests[0].requestBody); + equal(bulkUpdateRequest.column_data[0].column_id, 1); + equal(bulkUpdateRequest.column_data[0].user_id, 2); + equal(bulkUpdateRequest.column_data[0].content, "test content"); + equal(bulkUpdateRequest.column_data[1].column_id, 3); + equal(bulkUpdateRequest.column_data[1].user_id, 4); + equal(bulkUpdateRequest.column_data[1].content, "test content 2"); + }); }); diff --git a/spec/javascripts/jsx/gradebook_uploadsSpec.js b/spec/javascripts/jsx/gradebook_uploadsSpec.js index f691601ac90..69edcf98bd5 100644 --- a/spec/javascripts/jsx/gradebook_uploadsSpec.js +++ b/spec/javascripts/jsx/gradebook_uploadsSpec.js @@ -55,6 +55,7 @@ QUnit.module('gradebook_uploads#handleThingsNeedingToBeResolved', (hooks) => { defaultUploadedGradebook = { assignments: [{grading_type: null, id: '-1', points_possible: 10, previous_id: null, title: 'imported'}], + custom_columns: [], missing_objects: { assignments: [{grading_type: 'points', id: '73', points_possible: 10, previous_id: null, title: 'existing'}], students: [] @@ -65,7 +66,8 @@ QUnit.module('gradebook_uploads#handleThingsNeedingToBeResolved', (hooks) => { last_name_first: 'Efron, Zac', name: 'Zac Efron', previous_id: '1', - submissions: [{assignment_id: '-1', grade: '0.0', gradeable: true, original_grade: null}] + submissions: [{assignment_id: '-1', grade: '0.0', gradeable: true, original_grade: null}], + custom_column_data: [], }], warning_messages: { prevented_grading_ungradeable_submission: false, diff --git a/spec/lib/gradebook_importer_spec.rb b/spec/lib/gradebook_importer_spec.rb index 50831aef68c..02d5a134720 100644 --- a/spec/lib/gradebook_importer_spec.rb +++ b/spec/lib/gradebook_importer_spec.rb @@ -111,14 +111,14 @@ describe GradebookImporter do it 'normalizes pure numbers' do expected_grades = %w[123.4 1234.5 1234.5 -1234.50 1234.5 1234.5] - actual_grades = @gi.students.map { |student_row| student_row.gradebook_importer_submissions[0]['grade'] } + actual_grades = @gi.upload.gradebook.fetch('students').map { |student| student.fetch('submissions').first.fetch('grade') } expect(actual_grades).to match_array(expected_grades) end it 'normalizes percentages' do expected_grades = %w[57.4% 4200.3% 4200.3% -4200.30% 4200.3% 4200.3%] - actual_grades = @gi.students.map { |student_row| student_row.gradebook_importer_submissions[1]['grade'] } + actual_grades = @gi.upload.gradebook.fetch('students').map { |student| student.fetch('submissions').second.fetch('grade') } expect(actual_grades).to match_array(expected_grades) end @@ -158,14 +158,14 @@ describe GradebookImporter do it 'normalizes pure numbers' do expected_grades = %w[123.4 1234.5 1234.5 -1234.50 1234.5 1234.5] - actual_grades = @gi.students.map { |student_row| student_row.gradebook_importer_submissions[0]['grade'] } + actual_grades = @gi.upload.gradebook.fetch('students').map { |student| student.fetch('submissions').first.fetch('grade') } expect(actual_grades).to match_array(expected_grades) end it 'normalizes percentages' do expected_grades = %w[57.4% 4200.3% 4200.3% -4200.30% 4200.3% 4200.3%] - actual_grades = @gi.students.map { |student_row| student_row.gradebook_importer_submissions[1]['grade'] } + actual_grades = @gi.upload.gradebook.fetch('students').map { |student| student.fetch('submissions').second.fetch('grade') } expect(actual_grades).to match_array(expected_grades) end @@ -546,12 +546,51 @@ describe GradebookImporter do ",#{@student.id},,10" ) expect(@gi.assignments).to eq [@assignment1] - submission = @gi.students.first.gradebook_importer_submissions.first + submission = @gi.upload.gradebook.fetch('students').first.fetch('submissions').first expect(submission['original_grade']).to eq '8.0' expect(submission['grade']).to eq '10' expect(submission['assignment_id']).to eq @assignment1.id end + context "custom gradebook columns" do + before do + @student = User.create! + course_with_student(course: @course, user: @student, active_enrollment: true) + @course.custom_gradebook_columns.create!({title: "CustomColumn1", read_only: false}) + @course.custom_gradebook_columns.create!({title: "CustomColumn2", read_only: false}) + end + + it "includes non read only custom columns" do + importer_with_rows( + "Student,ID,Section,CustomColumn1,CustomColumn2,Assignment 1", + ",#{@student.id},,test 1,test 2,10" + ) + col = @gi.upload.gradebook.fetch('custom_columns').map do |custom_column| + custom_column.fetch('title') + end + expect(col).to eq ['CustomColumn1', 'CustomColumn2'] + end + + it "excludes read only custom columns" do + @course.custom_gradebook_columns.create!({title: "CustomColumn3", read_only: true}) + importer_with_rows( + "Student,ID,Section,CustomColumn1,CustomColumn2,CustomColumn3,Assignment 1", + ",#{@student.id},,test 1,test 2,test 3,10" + ) + col = @gi.upload.gradebook.fetch('custom_columns').find { |custom_column| custom_column.fetch('title') == 'CustomColumn3' } + expect(col).to eq nil + end + + it "expects custom column datum from non read only columns" do + importer_with_rows( + "Student,ID,Section,CustomColumn1,CustomColumn2,Assignment 1", + ",#{@student.id},,test 1,test 2,10" + ) + col = @gi.upload.gradebook.fetch('students').first.fetch('custom_column_data').map { |custom_column| custom_column.fetch('new_content') } + expect(col).to eq ['test 1', 'test 2'] + end + end + context "to_json" do before do course_model @@ -565,15 +604,18 @@ describe GradebookImporter do describe "simplified json output" do it "has only the specified keys" do - keys = [:assignments, :missing_objects, - :original_submissions, :students, + keys = [:assignments, + :custom_columns, + :missing_objects, + :original_submissions, + :students, :unchanged_assignments, :warning_messages] expect(hash.keys.sort).to eql(keys) end it "a student only has specified keys" do - keys = [:id, :last_name_first, :name, :previous_id, :submissions] + keys = [:custom_column_data, :id, :last_name_first, :name, :previous_id, :submissions] expect(student.keys.sort).to eql(keys) end @@ -802,6 +844,14 @@ describe GradebookImporter do expect(student_submissions.map {|s| s['assignment_id']}).to include @open_assignment.id end end + + it "marks excused submission as 'EX' even if 'ex' is not capitalized" do + importer_with_rows( + "Student,ID,Section,Assignment in closed period,Assignment in open period", + ",#{@student.id},,,eX", + ) + expect(student_submissions.first.fetch('grade')).to eq 'EX' + end end context "assignments with overrides" do @@ -924,8 +974,6 @@ describe GradebookImporter do let(:course) { Course.create account: account } let(:student) do student = User.create - student.gradebook_importer_submissions = [{ "grade" => "", - "original_grade" => ""}] student end let(:assignment) do @@ -938,46 +986,53 @@ describe GradebookImporter do let(:progress) { Progress.create tag: "test", context: student } let(:gradebook_upload){ GradebookUpload.create!(course: course, user: student, progress: progress) } let(:importer) { GradebookImporter.new(gradebook_upload, "", student, progress) } - let(:submission) { student.gradebook_importer_submissions.first } - it "translates positive score in submission['grade'] to complete" do - submission['grade'] = "3" - importer.translate_pass_fail(assignments, students) + it "translates positive score in gradebook_importer_assignments grade to complete" do + gradebook_importer_assignments = { student.id => [{ "grade" => "3", "original_grade" => ""}] } + importer.translate_pass_fail(assignments, students, gradebook_importer_assignments) + grade = gradebook_importer_assignments.fetch(student.id).first['grade'] - expect(submission['grade']).to eq "complete" + expect(grade).to eq "complete" end - it "translates positive grade in submission['original_grade'] to complete" do - submission['original_grade'] = "3" - importer.translate_pass_fail(assignments, students) + it "translates positive grade in gradebook_importer_assignments original_grade to complete" do + gradebook_importer_assignments = { student.id => [{ "grade" => "", "original_grade" => "5"}] } + importer.translate_pass_fail(assignments, students, gradebook_importer_assignments) + original_grade = gradebook_importer_assignments.fetch(student.id).first['original_grade'] - expect(submission['original_grade']).to eq "complete" + expect(original_grade).to eq "complete" end - it "translates 0 grade in submission['grade'] to incomplete" do - submission['grade'] = "0" - importer.translate_pass_fail(assignments, students) + it "translates 0 grade in gradebook_importer_assignments grade to incomplete" do + gradebook_importer_assignments = { student.id => [{ "grade" => "0", "original_grade" => ""}] } + importer.translate_pass_fail(assignments, students, gradebook_importer_assignments) + grade = gradebook_importer_assignments.fetch(student.id).first['grade'] - expect(submission['grade']).to eq "incomplete" + expect(grade).to eq "incomplete" end - it "translates 0 grade in submission['original_grade'] to incomplete" do - submission['original_grade'] = "0" - importer.translate_pass_fail(assignments, students) + it "translates 0 grade in gradebook_importer_assignments original_grade to incomplete" do + gradebook_importer_assignments = { student.id => [{ "grade" => "", "original_grade" => "0"}] } + importer.translate_pass_fail(assignments, students, gradebook_importer_assignments) + original_grade = gradebook_importer_assignments.fetch(student.id).first['original_grade'] - expect(submission['original_grade']).to eq "incomplete" + expect(original_grade).to eq "incomplete" end - it "doesn't change empty string grade in submission['grade']" do - importer.translate_pass_fail(assignments, students) + it "doesn't change empty string grade in gradebook_importer_assignments grade" do + gradebook_importer_assignments = { student.id => [{ "grade" => "", "original_grade" => ""}] } + importer.translate_pass_fail(assignments, students, gradebook_importer_assignments) + grade = gradebook_importer_assignments.fetch(student.id).first['grade'] - expect(submission['grade']).to eq "" + expect(grade).to eq "" end - it "doesn't change empty string grade in submission['original_grade']" do - importer.translate_pass_fail(assignments, students) + it "doesn't change empty string grade in gradebook_importer_assignments original_grade" do + gradebook_importer_assignments = { student.id => [{ "grade" => "", "original_grade" => ""}] } + importer.translate_pass_fail(assignments, students, gradebook_importer_assignments) + original_grade = gradebook_importer_assignments.fetch(student.id).first['original_grade'] - expect(submission['grade']).to eq "" + expect(original_grade).to eq "" end end