Added improved validations for 2nd step of members import

no issue

- Adds validations for imported CSV data
- These checks include obvious validation checks for data - like if email addresses are valid, checking if Stripe configured when entries with stripe_customer_id are present and additional server-side validation for entries with stripe_customer_id to check if they appear in connected Stripe account
- The validation set is calculated by naive choosing of first 5, middle 5 and 5 tail records from imported set. This logic comes from observations that errors usually apear withing "test" records in the beggining or the end of the file. These selection rules might change in the future if we find a need for it.
- Adds papaparse CSV parser, which was chosen for it's maturity and relatively small minified size. In the future this library should be lazy-loaded to make the first page load UX nicer
This commit is contained in:
Nazar Gargol 2020-06-12 17:22:36 +12:00
parent 7bf809449b
commit c3d141dd03
7 changed files with 249 additions and 10 deletions

View File

@ -33,6 +33,12 @@
{{else}}
<div class="modal-body">
{{#if (and this.filePresent (not this.failureMessage))}}
{{#each validationErrors as |error|}}
<div class="failed flex items-center gh-members-upload-failuremessage">
{{svg-jar "warning" class="w4 h4 fill-red mr1"}}
{{error.message}}
</div>
{{/each}}
<GhFormGroup>
<div class="bg-whitegrey-l2 ba b--whitegrey br3">
<div class="flex flex-column items-center justify-center gh-members-import-file">

View File

@ -1,5 +1,6 @@
import ModalComponent from 'ghost-admin/components/modal-base';
import ghostPaths from 'ghost-admin/utils/ghost-paths';
import papaparse from 'papaparse';
import {
UnsupportedMediaTypeError,
isRequestEntityTooLargeError,
@ -15,17 +16,19 @@ import {inject as service} from '@ember/service';
export default ModalComponent.extend({
ajax: service(),
notifications: service(),
memberImportValidator: service(),
labelText: 'Select or drag-and-drop a CSV File',
dragClass: null,
file: null,
fileData: null,
paramName: 'membersfile',
extensions: null,
uploading: false,
uploadPercentage: 0,
response: null,
failureMessage: null,
validationErrors: null,
labels: null,
// Allowed actions
@ -38,8 +41,9 @@ export default ModalComponent.extend({
return `${ghostPaths().apiRoot}/members/csv/`;
}),
importDisabled: computed('file', function () {
return !this.file || !(this._validateFileType(this.file));
importDisabled: computed('file', 'validationErrors', function () {
const hasEmptyDataFile = this.validationErrors && this.validationErrors.filter(error => error.message.includes('File is empty')).length;
return !this.file || !(this._validateFileType(this.file)) || hasEmptyDataFile;
}),
formData: computed('file', function () {
@ -73,7 +77,6 @@ export default ModalComponent.extend({
init() {
this._super(...arguments);
this.extensions = ['csv'];
// NOTE: nested label come from specific "gh-member-label-input" parameters, would be good to refactor
this.labels = {labels: []};
@ -85,10 +88,28 @@ export default ModalComponent.extend({
let validationResult = this._validateFileType(file);
if (validationResult !== true) {
this._uploadFailed(validationResult);
this._validationFailed(validationResult);
} else {
this.set('file', file);
this.set('failureMessage', null);
papaparse.parse(file, {
header: true,
skipEmptyLines: true,
worker: true, // NOTE: compare speed and file sizes with/without this flag
complete: async (results) => {
this.set('fileData', results.data);
let result = await this.memberImportValidator.check(results.data);
if (result !== true) {
this._importValidationFailed(result);
}
},
error: (error) => {
this._validationFailed(error);
}
});
}
},
@ -96,7 +117,8 @@ export default ModalComponent.extend({
this.set('failureMessage', null);
this.set('labels', {labels: []});
this.set('file', null);
this.set('failureMessage', null);
this.set('fileData', null);
this.set('validationErrors', null);
},
upload() {
@ -170,7 +192,7 @@ export default ModalComponent.extend({
}).then((response) => {
this._uploadSuccess(JSON.parse(response));
}).catch((error) => {
this._uploadFailed(error);
this._validationFailed(error);
}).finally(() => {
this._uploadFinished();
});
@ -199,7 +221,11 @@ export default ModalComponent.extend({
this.set('uploading', false);
},
_uploadFailed(error) {
_importValidationFailed(errors) {
this.set('validationErrors', errors);
},
_validationFailed(error) {
let message;
if (isVersionMismatchError(error)) {
@ -221,9 +247,8 @@ export default ModalComponent.extend({
_validateFileType(file) {
let [, extension] = (/(?:\.([^.]+))?$/).exec(file.name);
let extensions = this.extensions;
if (!extension || extensions.indexOf(extension.toLowerCase()) === -1) {
if (['csv'].indexOf(extension.toLowerCase()) === -1) {
return new UnsupportedMediaTypeError();
}

View File

@ -0,0 +1,6 @@
export default class EmailFailedError extends Error {
constructor(message) {
super(message);
this.name = 'MemberImportError';
}
}

View File

@ -0,0 +1,123 @@
import MemberImportError from 'ghost-admin/errors/member-import-error';
import Service, {inject as service} from '@ember/service';
import validator from 'validator';
export default Service.extend({
ajax: service(),
membersUtils: service(),
ghostPaths: service(),
async check(data) {
if (!data || !data.length) {
return [new MemberImportError('File is empty, nothing to import. Please select a different file.')];
}
let validatedSet = [];
let validationSampleSize = 15;
let validationResults = [];
if (data && data.length > validationSampleSize) {
// validated data size is larger than sample size take 3
// equal parts from head, tail and middle of the data set
const partitionSize = validationSampleSize / 3;
const head = data.slice(0, partitionSize);
const tail = data.slice((data.length - partitionSize), data.length);
const middleIndex = Math.floor(data.length / 2);
const middleStartIndex = middleIndex - 2;
const middleEndIndex = middleIndex + 3;
const middle = data.slice(middleStartIndex, middleEndIndex);
validatedSet.push(...head);
validatedSet.push(...middle);
validatedSet.push(...tail);
} else {
validatedSet = data;
}
let emailValidation = this._checkEmails(validatedSet);
if (emailValidation !== true) {
validationResults.push(new MemberImportError('Emails in provided data don\'t appear to be valid email addresses.'));
}
const hasStripeId = this._containsRecordsWithStripeId(validatedSet);
if (hasStripeId) {
let stripeLocalValidation = this._checkStripeLocal(validatedSet);
if (stripeLocalValidation !== true) {
validationResults.push(new MemberImportError('Stripe customer IDs exist in the data, but no stripe account is connected.'));
}
if (stripeLocalValidation === true) {
let stripeSeverValidation = await this._checkStripeServer(validatedSet);
if (stripeSeverValidation !== true) {
validationResults.push(new MemberImportError('Stripe customer IDs exist in the data, but we could not find such customer in connected Stripe account'));
}
}
}
if (validationResults.length) {
return validationResults;
} else {
return true;
}
},
_containsRecordsWithStripeId(validatedSet) {
let memberWithStripeId = validatedSet.find(m => !!(m.stripe_customer_id));
return !!memberWithStripeId;
},
_checkEmails(validatedSet) {
let result = true;
validatedSet.forEach((member) => {
if (!member.email) {
result = false;
}
if (member.email && !validator.isEmail(member.email)) {
result = false;
}
});
return result;
},
_checkStripeLocal(validatedSet) {
const isStripeConfigured = this.membersUtils.isStripeEnabled();
let result = true;
if (!isStripeConfigured) {
validatedSet.forEach((member) => {
if (member.stripe_customer_id) {
result = false;
}
});
}
return result;
},
async _checkStripeServer(validatedSet) {
const url = this.ghostPaths.get('url').api('members/validate');
let response;
try {
response = await this.ajax.post(url, {
data: {
members: validatedSet
}
});
} catch (e) {
return false;
}
if (response.errors) {
return false;
}
return true;
}
});

View File

@ -122,6 +122,7 @@
"markdown-it-mark": "3.0.0",
"matchdep": "2.0.0",
"normalize.css": "3.0.3",
"papaparse": "5.2.0",
"postcss-color-mod-function": "3.0.3",
"postcss-custom-media": "7.0.8",
"postcss-custom-properties": "9.1.1",

View File

@ -0,0 +1,73 @@
import Pretender from 'pretender';
import Service from '@ember/service';
import {describe, it} from 'mocha';
import {expect} from 'chai';
import {setupTest} from 'ember-mocha';
let MembersUtilsStub = Service.extend({
isStripeEnabled: () => (true)
});
describe('Integration: Service: member-import-validator', function () {
setupTest();
let server;
beforeEach(function () {
server = new Pretender();
this.owner.register('service:membersUtils', MembersUtilsStub);
});
afterEach(function () {
server.shutdown();
});
it('checks correct data without Stripe customer', async function () {
let service = this.owner.lookup('service:member-import-validator');
const result = await service.check([{
name: 'Rish',
email: 'validemail@example.com'
}]);
expect(result).to.equal(true);
});
it('returns validation error when no data is provided', async function () {
let service = this.owner.lookup('service:member-import-validator');
const result = await service.check([]);
expect(result.length).to.equal(1);
expect(result[0].message).to.equal('No data present in selected file.');
});
it('returns validation error for data with invalid email', async function () {
let service = this.owner.lookup('service:member-import-validator');
const result = await service.check([{
name: 'Egg',
email: 'notAnEmail'
}]);
expect(result.length).to.equal(1);
expect(result[0].message).to.equal('Emails in provided data don\'t appear to be valid email addresses.');
});
it('returns validation error for data with stripe_customer_id but no connected Stripe', async function () {
this.owner.register('service:membersUtils', Service.extend({
isStripeEnabled: () => (false)
}));
let service = this.owner.lookup('service:member-import-validator');
const result = await service.check([{
name: 'Kevin',
email: 'goodeamil@example.com',
stripe_customer_id: 'cus_XXXX'
}]);
expect(result.length).to.equal(1);
expect(result[0].message).to.equal('Stripe customer IDs exist in the data, but no stripe account is connected.');
});
});

View File

@ -10578,6 +10578,11 @@ pako@~1.0.5:
resolved "https://registry.yarnpkg.com/pako/-/pako-1.0.11.tgz#6c9599d340d54dfd3946380252a35705a6b992bf"
integrity sha512-4hLB8Py4zZce5s4yd9XzopqwVv/yGNhV1Bl8NTmCq1763HeK2+EwVTv+leGeL13Dnh2wfbqowVPXCIO0z4taYw==
papaparse@5.2.0:
version "5.2.0"
resolved "https://registry.yarnpkg.com/papaparse/-/papaparse-5.2.0.tgz#97976a1b135c46612773029153dc64995caa3b7b"
integrity sha512-ylq1wgUSnagU+MKQtNeVqrPhZuMYBvOSL00DHycFTCxownF95gpLAk1HiHdUW77N8yxRq1qHXLdlIPyBSG9NSA==
parallel-transform@^1.1.0:
version "1.2.0"
resolved "https://registry.yarnpkg.com/parallel-transform/-/parallel-transform-1.2.0.tgz#9049ca37d6cb2182c3b1d2c720be94d14a5814fc"