From 8392c5af66e82ea4122a7b6964069027c68a60ce Mon Sep 17 00:00:00 2001 From: Ha Nguyen Date: Wed, 20 Mar 2019 13:24:46 -0700 Subject: [PATCH 1/3] Add more fields to Fit cohort CSV file. Sanitize content so it can be opened in spreadsheet tool. --- .../admin_cohort_view.jsx | 28 +++++++++++++++++-- .../api/v1/pd/fit_cohort_view_serializer.rb | 9 ++++-- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/apps/src/code-studio/pd/application_dashboard/admin_cohort_view.jsx b/apps/src/code-studio/pd/application_dashboard/admin_cohort_view.jsx index ad40448f1a8c7..d02d282ea1ec7 100644 --- a/apps/src/code-studio/pd/application_dashboard/admin_cohort_view.jsx +++ b/apps/src/code-studio/pd/application_dashboard/admin_cohort_view.jsx @@ -9,6 +9,7 @@ import Select from 'react-select'; import $ from 'jquery'; import downloadCsv from '../downloadCsv'; import AdminCohortViewTable from './admin_cohort_view_table'; +import _ from 'lodash'; const styles = { downloadCsvButton: { @@ -63,6 +64,18 @@ export default class AdminCohortView extends React.Component { } } + sanitizeStringForCsv = str => { + let res = str; + if (str) { + // Convert line breaker to dot, multiple spaces to single space + res = res.replace(/\n|\r\n|\r+/gm, '.'); + res = res.replace(/\s+/gm, ' '); + res = res.trim(); + } + + return res; + }; + handleDownloadCsv = () => { const headers = { date_accepted: 'Date Accepted', @@ -72,6 +85,8 @@ export default class AdminCohortView extends React.Component { email: 'Email', assigned_workshop: 'Assigned Workshop', registered_workshop: 'Registered Workshop', + assigned_fit: 'Assigned FiT', + registered_fit_submission_time: 'Registered FiT Submission Time', accepted_seat: 'Accepted Seat?', course_name: 'Course', regional_partner_name: 'Regional Partner', @@ -91,14 +106,23 @@ export default class AdminCohortView extends React.Component { // Make sure we include all form_data keys that appear on any row: Object.keys(row.form_data).forEach(formDataHeader => { if (!headers[formDataHeader]) { - // Use the raw formData key as the column header - headers[formDataHeader] = formDataHeader; + // Convert formData key to more readable format, use it as the column header + headers[formDataHeader] = _.startCase(formDataHeader); } }); return {...row, ...row.form_data}; }); + // Sanitize string content before exporting it to CSV + filteredCohortWithFormData.forEach(row => { + Object.keys(row).forEach(key => { + if (_.isString(row[key])) { + row[key] = this.sanitizeStringForCsv(row[key]); + } + }); + }); + downloadCsv({ data: filteredCohortWithFormData, filename: `${this.props.route.cohortType.toLowerCase()}_cohort.csv`, diff --git a/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb b/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb index d8b5bc904d4a9..658be7a4cebd8 100644 --- a/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb +++ b/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb @@ -14,6 +14,7 @@ class Api::V1::Pd::FitCohortViewSerializer < ActiveModel::Serializer :assigned_fit, :registered_fit, :accepted_fit, + :registered_fit_submission_time, :role, :status, :locked, @@ -47,12 +48,16 @@ def registered_fit object.try(:registered_fit_workshop?) end + def registered_fit_submission_time + object.try(FIT_WEEKEND_REGISTRATION_SYMBOL).try(:created_at) + end + def fit_assigned_at_registration - object.try(FIT_WEEKEND_REGISTRATION_FACTORY).try(:fit_city) + object.try(FIT_WEEKEND_REGISTRATION_SYMBOL).try(:fit_city) end def accepted_fit - object.try(FIT_WEEKEND_REGISTRATION_FACTORY).try(:accepted_seat_simplified) + object.try(FIT_WEEKEND_REGISTRATION_SYMBOL).try(:accepted_seat_simplified) end def role From 732846fc4c51f12ff182fe527d51545f508f6bb0 Mon Sep 17 00:00:00 2001 From: Ha Nguyen Date: Fri, 22 Mar 2019 14:54:14 -0700 Subject: [PATCH 2/3] Fix PR comments, add unit tests --- .../admin_cohort_view.jsx | 32 +++-- .../admin_cohort_viewTest.js | 134 ++++++++++++++++++ .../api/v1/pd/fit_cohort_view_serializer.rb | 3 +- 3 files changed, 153 insertions(+), 16 deletions(-) create mode 100644 apps/test/unit/code-studio/pd/application_dashboard/admin_cohort_viewTest.js diff --git a/apps/src/code-studio/pd/application_dashboard/admin_cohort_view.jsx b/apps/src/code-studio/pd/application_dashboard/admin_cohort_view.jsx index d02d282ea1ec7..b5d0218c8d201 100644 --- a/apps/src/code-studio/pd/application_dashboard/admin_cohort_view.jsx +++ b/apps/src/code-studio/pd/application_dashboard/admin_cohort_view.jsx @@ -39,9 +39,12 @@ export default class AdminCohortView extends React.Component { static propTypes = { route: PropTypes.shape({ cohortType: PropTypes.oneOf(['TeacherCon', 'FiT']) - }) + }), + downloadCsv: PropTypes.func }; + static defaultProps = {downloadCsv: downloadCsv}; + constructor(props) { super(props); @@ -64,17 +67,15 @@ export default class AdminCohortView extends React.Component { } } - sanitizeStringForCsv = str => { - let res = str; - if (str) { - // Convert line breaker to dot, multiple spaces to single space - res = res.replace(/\n|\r\n|\r+/gm, '.'); - res = res.replace(/\s+/gm, ' '); - res = res.trim(); - } - - return res; - }; + /** + * Clean a string, convert line breaker to dot and multiple spaces to single space. + */ + static sanitizeString(str) { + return (str || '') + .replace(/(\n|\r)+/gm, '. ') + .replace(/\s+/gm, ' ') + .trim(); + } handleDownloadCsv = () => { const headers = { @@ -114,16 +115,17 @@ export default class AdminCohortView extends React.Component { return {...row, ...row.form_data}; }); - // Sanitize string content before exporting it to CSV + // Clean string content of line breakers and whitespaces before exporting it to CSV. + // Separator (comma) will be escaped later in downloadCsv function. filteredCohortWithFormData.forEach(row => { Object.keys(row).forEach(key => { if (_.isString(row[key])) { - row[key] = this.sanitizeStringForCsv(row[key]); + row[key] = AdminCohortView.sanitizeString(row[key]); } }); }); - downloadCsv({ + this.props.downloadCsv({ data: filteredCohortWithFormData, filename: `${this.props.route.cohortType.toLowerCase()}_cohort.csv`, headers diff --git a/apps/test/unit/code-studio/pd/application_dashboard/admin_cohort_viewTest.js b/apps/test/unit/code-studio/pd/application_dashboard/admin_cohort_viewTest.js new file mode 100644 index 0000000000000..b1e94e98a7e15 --- /dev/null +++ b/apps/test/unit/code-studio/pd/application_dashboard/admin_cohort_viewTest.js @@ -0,0 +1,134 @@ +import AdminCohortView from '@cdo/apps/code-studio/pd/application_dashboard/admin_cohort_view'; +import {assert, expect} from '../../../../util/configuredChai'; +import React from 'react'; +import {shallow} from 'enzyme'; +import sinon from 'sinon'; + +describe('AdminCohortView component', () => { + // FiT cohort data is combination of application and registration data + const DEFAULT_FIT_COHORT_DATA = [ + { + accepted_fit: 'Yes', + applicant_name: 'applicant', + assigned_fit: 'fit', + assigned_workshop: 'workshop', + course_name: 'CS Discoveries', + date_accepted: '2019-03-22', + district_name: 'district', + email: 'teacher@school.edu', + form_data: { + ableToAttend: 'Yes', + addressCity: 'city', + addressState: 'state', + addressStreet: 'street', + addressZip: '12345', + agreeShareContact: true, + city: null, + contactFirstName: 'first', + contactLastName: 'last', + contactPhone: '1234567890', + contactRelationship: 'me', + date: 'June 1-1, 2019', + dietaryNeeds: ['None'], + email: 'teacher@school.edu', + howTraveling: 'I will drive by myself', + howTraveling_carpooling_with_attendee: 'the passenger', + lastName: 'last', + liabilityWaiver: true, + liveFarAway: 'Yes', + needAda: 'Yes', + needDisabilitySupport: 'Yes', + needHotel: 'Yes', + phone: '1234567890', + photoRelease: true, + preferredFirstName: 'first' + }, + id: 1, + locked: true, + notes: '3/21: Approved by Code.org.', + notes_2: 'Questions has for us:\n', + notes_3: + 'Strengths: \nWeaknesses: \nPotential red flags to follow- up on: \nOther notes:', + notes_4: null, + notes_5: null, + regional_partner_name: 'partner', + registered_fit: true, + registered_fit_submission_time: 'Mar 22 2019 12:02am UTC', + registered_workshop: false, + role: 'Lead Facilitator', + school_name: 'school', + status: 'accepted', + type: 'Pd::Application::Facilitator1920Application' + } + ]; + + const minProps = {route: {cohortType: 'FiT'}}; + + before(() => { + // Prevent AdminCohortView load() function to send ajax request to server + // by stubbing it and faking the server-returned data. + sinon.stub(AdminCohortView.prototype, 'load'); + }); + + after(() => { + AdminCohortView.prototype.load.restore(); + }); + + it('can be rendered', () => { + const wrapper = shallow(); + expect(wrapper).not.to.be.null; + }); + + it('can sanitize strings', () => { + const testCases = [ + {input: undefined, expected: ''}, + {input: '', expected: ''}, + {input: '\ta ', expected: 'a'}, + {input: 'a b\t\tc \t d', expected: 'a b c d'}, + {input: '\na\nb\rc\n\rd\r\ne\n\nf\r\r', expected: '. a. b. c. d. e. f.'} + ]; + + testCases.forEach(testCase => { + expect(AdminCohortView.sanitizeString(testCase.input)).to.equal( + testCase.expected + ); + }); + }); + + it('can compile data to export to CSV', () => { + let downloadCsvSpy = sinon.spy(); + const wrapper = shallow( + + ); + + // Fake component state instead of loading real data from server + wrapper.setState({ + cohort: DEFAULT_FIT_COHORT_DATA, + filteredCohort: DEFAULT_FIT_COHORT_DATA, + loading: false + }); + + // Test part of handleDownloadCsv function to the point where it calls downloadCsv function. + wrapper.instance().handleDownloadCsv(); + + expect(downloadCsvSpy).to.have.been.calledOnce; + + let spyCallArgs = downloadCsvSpy.lastCall.args[0]; + assert.strictEqual( + spyCallArgs.filename, + `${minProps.route.cohortType.toLowerCase()}_cohort.csv`, + 'CSV file name is not as expected' + ); + + let missingKeyCnt = 0; + Object.keys(DEFAULT_FIT_COHORT_DATA[0].form_data).forEach(key => { + if ( + !spyCallArgs.headers.hasOwnProperty(key) || + !spyCallArgs.data[0].hasOwnProperty(key) + ) { + missingKeyCnt += 1; + } + }); + assert.equal(missingKeyCnt, 0, 'Form data keys are missing'); + }); +}); diff --git a/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb b/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb index 658be7a4cebd8..ce8c88a8e6c32 100644 --- a/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb +++ b/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb @@ -49,7 +49,8 @@ def registered_fit end def registered_fit_submission_time - object.try(FIT_WEEKEND_REGISTRATION_SYMBOL).try(:created_at) + # Return friendly time format: "Mar 21 2019 10:33am UTC" + object.try(FIT_WEEKEND_REGISTRATION_SYMBOL).try(:created_at).strftime('%b %d %Y %l:%M%P %Z') end def fit_assigned_at_registration From 955ad0dcb80abad22974a1fbe34bd00e99b24ed5 Mon Sep 17 00:00:00 2001 From: Ha Nguyen Date: Fri, 22 Mar 2019 16:29:19 -0700 Subject: [PATCH 3/3] fix applications_controller unit test --- .../app/serializers/api/v1/pd/fit_cohort_view_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb b/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb index ce8c88a8e6c32..30ec49c04eabd 100644 --- a/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb +++ b/dashboard/app/serializers/api/v1/pd/fit_cohort_view_serializer.rb @@ -50,7 +50,7 @@ def registered_fit def registered_fit_submission_time # Return friendly time format: "Mar 21 2019 10:33am UTC" - object.try(FIT_WEEKEND_REGISTRATION_SYMBOL).try(:created_at).strftime('%b %d %Y %l:%M%P %Z') + object.try(FIT_WEEKEND_REGISTRATION_SYMBOL).try(:created_at).try(:strftime, '%b %d %Y %l:%M%P %Z') end def fit_assigned_at_registration