Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-10 23:36:14 -05:00
Code Issues Activity

Added invalid import record errors and counts

Browse source 
no issue

- This new format allows to return additional metadata with failed import records. The data for invalid records is returned in following format:
```
{
    count: {count_of_invalid_records},
    errors: [{
      message:	"Members not imported. Members with duplicate Stripe customer ids are not allowed." // message field of the error
     context:	"Attempting to import members with duplicate Stripe customer ids." // context field of the error
     help:	"Remove duplicate Stripe customer ids from the import file, and re-run the import." // help field of the error
     count:	2 // count of this specific error
    }]
};
- Errors are grouped by their context fields because message fields sometimes can contain unique information like Stripe customer id, which would produce too many errors in case of bigger datasets.
This commit is contained in:
Nazar Gargol 2020-06-12 19:59:36 +12:00
parent 589d826afd
commit 7904c303a7
4 changed files with 65 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

71
core/server/api/canary/members.js
View file

@ -387,9 +387,14 @@ const members = {
},
async query(frame) {
let filePath = frame.file.path;
let fulfilled = 0;
let invalid = 0;
let duplicates = 0;
let imported = {
count: 0
};
let invalid = {
count: 0,
errors: []
};
let duplicateStripeCustomerIdCount = 0;
const columnsToExtract = [{
name: 'email',
@ -427,7 +432,16 @@ const members = {
columnsToExtract: columnsToExtract
}).then((result) => {
const sanitized = sanitizeInput(result);
invalid += result.length - sanitized.length;
duplicateStripeCustomerIdCount = result.length - sanitized.length;
invalid.count += duplicateStripeCustomerIdCount;
if (duplicateStripeCustomerIdCount) {
invalid.errors.push(new errors.ValidationError({
message: i18n.t('errors.api.members.duplicateStripeCustomerIds.message'),
context: i18n.t('errors.api.members.duplicateStripeCustomerIds.context'),
help: i18n.t('errors.api.members.duplicateStripeCustomerIds.help')
}));
}
return Promise.map(sanitized, ((entry) => {
const api = require('./index');
@ -465,30 +479,51 @@ const members = {
}), {concurrency: 10})
.each((inspection) => {
if (inspection.isFulfilled()) {
fulfilled = fulfilled + 1;
imported.count = imported.count + 1;
} else {
const error = inspection.reason();
if (error instanceof errors.ValidationError && !(error.message.match(/Stripe/g))) {
duplicates = duplicates + 1;
} else {
// NOTE: if the error happens as a result of pure API call it doesn't get logged anywhere
// for this reason we have to make sure any unexpected errors are logged here
if (Array.isArray(error)) {
logging.error(error[0]);
} else {
logging.error(error);
}
invalid = invalid + 1;
// NOTE: if the error happens as a result of pure API call it doesn't get logged anywhere
// for this reason we have to make sure any unexpected errors are logged here
if (Array.isArray(error)) {
logging.error(error[0]);
} else {
logging.error(error);
}
invalid.count = invalid.count + 1;
invalid.errors.push(error);
}
});
}).then(() => {
// NOTE: grouping by context because messages can contain unique data like "customer_id"
const groupedErrors = _.groupBy(invalid.errors, 'context');
const uniqueErrors = _.uniq(invalid.errors, 'context');
const outputErrors = uniqueErrors.map((error) => {
let errorGroup = groupedErrors[error.context];
let errorCount = errorGroup.length;
if (error.message === i18n.t('errors.api.members.duplicateStripeCustomerIds.message')) {
errorCount = duplicateStripeCustomerIdCount;
}
// NOTE: filtering only essential error information, so API doesn't leak more error details than it should
return {
message: error.message,
context: error.context,
help: error.help,
count: errorCount
};
});
invalid.errors = outputErrors;
return {
meta: {
stats: {
imported: fulfilled,
duplicates: duplicates,
imported: imported,
invalid: invalid
}
}

5
core/server/translations/en.json
View file

@ -377,6 +377,11 @@
"stripeCustomerNotFound": {
"context": "The customer does not exist in currently linked Stripe account.",
"help": "Connect the correct Stripe account, and re-run the import."
},
"duplicateStripeCustomerIds": {
"message": "Members not imported. Members with duplicate Stripe customer ids are not allowed.",
"context": "Attempting to import members with duplicate Stripe customer ids.",
"help": "Remove duplicate Stripe customer ids from the import file, and re-run the import."
}
},
"tags": {

5
test/api-acceptance/admin/members_spec.js
View file

@ -316,9 +316,8 @@ describe('Members API', function () {
should.exist(jsonResponse.meta);
should.exist(jsonResponse.meta.stats);
jsonResponse.meta.stats.imported.should.equal(2);
jsonResponse.meta.stats.duplicates.should.equal(0);
jsonResponse.meta.stats.invalid.should.equal(0);
jsonResponse.meta.stats.imported.count.should.equal(2);
jsonResponse.meta.stats.invalid.count.should.equal(0);
});
});

13
test/regression/api/canary/admin/members_spec.js
View file

@ -153,9 +153,8 @@ describe('Members API', function () {
should.exist(jsonResponse.meta);
should.exist(jsonResponse.meta.stats);
jsonResponse.meta.stats.imported.should.equal(2);
jsonResponse.meta.stats.duplicates.should.equal(0);
jsonResponse.meta.stats.invalid.should.equal(0);
jsonResponse.meta.stats.imported.count.should.equal(2);
jsonResponse.meta.stats.invalid.count.should.equal(0);
})
.then(() => {
return request
@ -202,9 +201,8 @@ describe('Members API', function () {
should.exist(jsonResponse.meta);
should.exist(jsonResponse.meta.stats);
jsonResponse.meta.stats.imported.should.equal(2);
jsonResponse.meta.stats.duplicates.should.equal(0);
jsonResponse.meta.stats.invalid.should.equal(0);
jsonResponse.meta.stats.imported.count.should.equal(2);
jsonResponse.meta.stats.invalid.count.should.equal(0);
})
.then(() => {
return request
@ -251,8 +249,7 @@ describe('Members API', function () {
should.exist(jsonResponse.meta);
should.exist(jsonResponse.meta.stats);
jsonResponse.meta.stats.imported.should.equal(0);
jsonResponse.meta.stats.duplicates.should.equal(0);
jsonResponse.meta.stats.imported.count.should.equal(0);
jsonResponse.meta.stats.invalid.should.equal(2);
});
});