mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-01-20 22:42:53 -05:00
Simplified members CSV importer constructor
refs https://github.com/TryGhost/Team/issues/2077 - Passing in the whole "getMembersApi" is just too much state to know about for the importer - it only uses a concept of default tier and members repository, the rest is distracting fluff making it hard to reason about what the importer **has to** know to function - Passing in two functions breaking up the above state simplifies the constructor API. - This is also a groundwork before substituting productsRepository for tiersRepository (refed issue objective)
This commit is contained in:
Naz
parent
f5a55092e5
commit
a7f5ee0ad5
3 changed files with 70 additions and 65 deletions
|
|
@ -47,7 +47,14 @@ let verificationTrigger;
|
|||
const membersImporter = new MembersCSVImporter({
|
||||
storagePath: config.getContentPath('data'),
|
||||
getTimezone: () => settingsCache.get('timezone'),
|
||||
getMembersApi: () => module.exports.api,
|
||||
getMembersRepository: async () => {
|
||||
const api = await module.exports.api;
|
||||
return api.members;
|
||||
},
|
||||
getDefaultTier: async () => {
|
||||
const api = await module.exports.api;
|
||||
return api.productRepository.getDefaultProduct;
|
||||
},
|
||||
sendEmail: ghostMailer.send.bind(ghostMailer),
|
||||
isSet: labsService.isSet.bind(labsService),
|
||||
addJob: jobsService.addJob.bind(jobsService),
|
||||
|
|
|
|||
|
|
@ -29,7 +29,8 @@ module.exports = class MembersCSVImporter {
|
|||
* @param {Object} options
|
||||
* @param {string} options.storagePath - The path to store CSV's in before importing
|
||||
* @param {Function} options.getTimezone - function returning currently configured timezone
|
||||
* @param {() => Object} options.getMembersApi
|
||||
* @param {() => Object} options.getMembersRepository - member model access instance for data access and manipulation
|
||||
* @param {() => Object} options.getDefaultTier - async function returning default Member Tier
|
||||
* @param {Function} options.sendEmail - function sending an email
|
||||
* @param {(string) => boolean} options.isSet - Method checking if specific feature is enabled
|
||||
* @param {({job, offloaded}) => void} options.addJob - Method registering an async job
|
||||
|
|
@ -37,10 +38,11 @@ module.exports = class MembersCSVImporter {
|
|||
* @param {Function} options.urlFor - function generating urls
|
||||
* @param {Object} options.context
|
||||
*/
|
||||
constructor({storagePath, getTimezone, getMembersApi, sendEmail, isSet, addJob, knex, urlFor, context}) {
|
||||
constructor({storagePath, getTimezone, getMembersRepository, getDefaultTier, sendEmail, isSet, addJob, knex, urlFor, context}) {
|
||||
this._storagePath = storagePath;
|
||||
this._getTimezone = getTimezone;
|
||||
this._getMembersApi = getMembersApi;
|
||||
this._getMembersRepository = getMembersRepository;
|
||||
this._getDefaultTier = getDefaultTier;
|
||||
this._sendEmail = sendEmail;
|
||||
this._isSet = isSet;
|
||||
this._addJob = addJob;
|
||||
|
|
@ -107,9 +109,8 @@ module.exports = class MembersCSVImporter {
|
|||
async perform(filePath) {
|
||||
const rows = membersCSV.parse(filePath, DEFAULT_CSV_HEADER_MAPPING);
|
||||
|
||||
const membersApi = await this._getMembersApi();
|
||||
|
||||
const defaultTier = await membersApi.productRepository.getDefaultProduct();
|
||||
const defaultTier = await this._getDefaultTier();
|
||||
const membersRepository = await this._getMembersRepository();
|
||||
|
||||
const result = await rows.reduce(async (resultPromise, row) => {
|
||||
const resultAccumulator = await resultPromise;
|
||||
|
|
@ -131,14 +132,14 @@ module.exports = class MembersCSVImporter {
|
|||
created_at: row.created_at,
|
||||
labels: row.labels
|
||||
};
|
||||
const existingMember = await membersApi.members.get({email: memberValues.email}, {
|
||||
const existingMember = await membersRepository.get({email: memberValues.email}, {
|
||||
...options,
|
||||
withRelated: ['labels']
|
||||
});
|
||||
let member;
|
||||
if (existingMember) {
|
||||
const existingLabels = existingMember.related('labels') ? existingMember.related('labels').toJSON() : [];
|
||||
member = await membersApi.members.update({
|
||||
member = await membersRepository.update({
|
||||
...memberValues,
|
||||
labels: existingLabels.concat(memberValues.labels)
|
||||
}, {
|
||||
|
|
@ -146,7 +147,7 @@ module.exports = class MembersCSVImporter {
|
|||
id: existingMember.id
|
||||
});
|
||||
} else {
|
||||
member = await membersApi.members.create(memberValues, Object.assign({}, options, {
|
||||
member = await membersRepository.create(memberValues, Object.assign({}, options, {
|
||||
context: {
|
||||
import: true
|
||||
}
|
||||
|
|
@ -154,12 +155,12 @@ module.exports = class MembersCSVImporter {
|
|||
}
|
||||
|
||||
if (row.stripe_customer_id && typeof row.stripe_customer_id === 'string') {
|
||||
await membersApi.members.linkStripeCustomer({
|
||||
await membersRepository.linkStripeCustomer({
|
||||
customer_id: row.stripe_customer_id,
|
||||
member_id: member.id
|
||||
}, options);
|
||||
} else if (row.complimentary_plan) {
|
||||
await membersApi.members.update({
|
||||
await membersRepository.update({
|
||||
products: [{id: defaultTier.id}]
|
||||
}, {
|
||||
...options,
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ describe('Importer', function () {
|
|||
let memberCreateStub;
|
||||
let knexStub;
|
||||
let sendEmailStub;
|
||||
let membersApiStub;
|
||||
let membersRepositoryStub;
|
||||
|
||||
const defaultAllowedFields = {
|
||||
email: 'email',
|
||||
|
|
@ -42,29 +42,21 @@ describe('Importer', function () {
|
|||
sinon.restore();
|
||||
});
|
||||
|
||||
// @NOTE: this is way too much mocking! the MembersCSVImporter constructor API should be simplified
|
||||
const buildMockImporterInstance = () => {
|
||||
const defaultTier = {
|
||||
const defaultTierDummy = {
|
||||
id: 'default_tier_id'
|
||||
};
|
||||
|
||||
memberCreateStub = sinon.stub().resolves({
|
||||
id: `test_member_id`
|
||||
});
|
||||
membersApiStub = {
|
||||
productRepository: {
|
||||
getDefaultProduct: async () => {
|
||||
return defaultTier;
|
||||
}
|
||||
membersRepositoryStub = {
|
||||
get: async () => {
|
||||
return null;
|
||||
},
|
||||
members: {
|
||||
get: async () => {
|
||||
return null;
|
||||
},
|
||||
create: memberCreateStub,
|
||||
update: sinon.stub().resolves(null),
|
||||
linkStripeCustomer: sinon.stub().resolves(null)
|
||||
}
|
||||
create: memberCreateStub,
|
||||
update: sinon.stub().resolves(null),
|
||||
linkStripeCustomer: sinon.stub().resolves(null)
|
||||
};
|
||||
|
||||
knexStub = {
|
||||
|
|
@ -79,7 +71,12 @@ describe('Importer', function () {
|
|||
return new MembersCSVImporter({
|
||||
storagePath: csvPath,
|
||||
getTimezone: sinon.stub().returns('UTC'),
|
||||
getMembersApi: () => membersApiStub,
|
||||
getMembersRepository: () => {
|
||||
return membersRepositoryStub;
|
||||
},
|
||||
getDefaultTier: () => {
|
||||
return defaultTierDummy;
|
||||
},
|
||||
sendEmail: sendEmailStub,
|
||||
isSet: sinon.stub(),
|
||||
addJob: sinon.stub(),
|
||||
|
|
@ -169,43 +166,43 @@ describe('Importer', function () {
|
|||
fsWriteSpy.calledOnce.should.be.true();
|
||||
|
||||
// member records get inserted
|
||||
membersApiStub.members.create.calledTwice.should.be.true();
|
||||
membersRepositoryStub.create.calledTwice.should.be.true();
|
||||
|
||||
should.equal(membersApiStub.members.create.args[0][1].context.import, true, 'inserts are done in the "import" context');
|
||||
should.equal(membersRepositoryStub.create.args[0][1].context.import, true, 'inserts are done in the "import" context');
|
||||
|
||||
should.deepEqual(Object.keys(membersApiStub.members.create.args[0][0]), ['email', 'name', 'note', 'subscribed', 'created_at', 'labels']);
|
||||
should.equal(membersApiStub.members.create.args[0][0].id, undefined, 'id field should not be taken from the user input');
|
||||
should.equal(membersApiStub.members.create.args[0][0].email, 'member_complimentary_test@example.com');
|
||||
should.equal(membersApiStub.members.create.args[0][0].name, 'bobby tables');
|
||||
should.equal(membersApiStub.members.create.args[0][0].note, 'a note');
|
||||
should.equal(membersApiStub.members.create.args[0][0].subscribed, true);
|
||||
should.equal(membersApiStub.members.create.args[0][0].created_at, '2022-10-18T06:34:08.000Z');
|
||||
should.equal(membersApiStub.members.create.args[0][0].deleted_at, undefined, 'deleted_at field should not be taken from the user input');
|
||||
should.deepEqual(membersApiStub.members.create.args[0][0].labels, [{
|
||||
should.deepEqual(Object.keys(membersRepositoryStub.create.args[0][0]), ['email', 'name', 'note', 'subscribed', 'created_at', 'labels']);
|
||||
should.equal(membersRepositoryStub.create.args[0][0].id, undefined, 'id field should not be taken from the user input');
|
||||
should.equal(membersRepositoryStub.create.args[0][0].email, 'member_complimentary_test@example.com');
|
||||
should.equal(membersRepositoryStub.create.args[0][0].name, 'bobby tables');
|
||||
should.equal(membersRepositoryStub.create.args[0][0].note, 'a note');
|
||||
should.equal(membersRepositoryStub.create.args[0][0].subscribed, true);
|
||||
should.equal(membersRepositoryStub.create.args[0][0].created_at, '2022-10-18T06:34:08.000Z');
|
||||
should.equal(membersRepositoryStub.create.args[0][0].deleted_at, undefined, 'deleted_at field should not be taken from the user input');
|
||||
should.deepEqual(membersRepositoryStub.create.args[0][0].labels, [{
|
||||
name: 'user import label'
|
||||
}]);
|
||||
|
||||
should.deepEqual(Object.keys(membersApiStub.members.create.args[1][0]), ['email', 'name', 'note', 'subscribed', 'created_at', 'labels']);
|
||||
should.equal(membersApiStub.members.create.args[1][0].id, undefined, 'id field should not be taken from the user input');
|
||||
should.equal(membersApiStub.members.create.args[1][0].email, 'member_stripe_test@example.com');
|
||||
should.equal(membersApiStub.members.create.args[1][0].name, 'stirpey beaver');
|
||||
should.equal(membersApiStub.members.create.args[1][0].note, 'testing notes');
|
||||
should.equal(membersApiStub.members.create.args[1][0].subscribed, false);
|
||||
should.equal(membersApiStub.members.create.args[1][0].created_at, '2022-10-18T07:31:57.000Z');
|
||||
should.equal(membersApiStub.members.create.args[1][0].deleted_at, undefined, 'deleted_at field should not be taken from the user input');
|
||||
should.deepEqual(membersApiStub.members.create.args[1][0].labels, [], 'no labels should be assigned');
|
||||
should.deepEqual(Object.keys(membersRepositoryStub.create.args[1][0]), ['email', 'name', 'note', 'subscribed', 'created_at', 'labels']);
|
||||
should.equal(membersRepositoryStub.create.args[1][0].id, undefined, 'id field should not be taken from the user input');
|
||||
should.equal(membersRepositoryStub.create.args[1][0].email, 'member_stripe_test@example.com');
|
||||
should.equal(membersRepositoryStub.create.args[1][0].name, 'stirpey beaver');
|
||||
should.equal(membersRepositoryStub.create.args[1][0].note, 'testing notes');
|
||||
should.equal(membersRepositoryStub.create.args[1][0].subscribed, false);
|
||||
should.equal(membersRepositoryStub.create.args[1][0].created_at, '2022-10-18T07:31:57.000Z');
|
||||
should.equal(membersRepositoryStub.create.args[1][0].deleted_at, undefined, 'deleted_at field should not be taken from the user input');
|
||||
should.deepEqual(membersRepositoryStub.create.args[1][0].labels, [], 'no labels should be assigned');
|
||||
|
||||
// stripe customer import
|
||||
membersApiStub.members.linkStripeCustomer.calledOnce.should.be.true();
|
||||
should.equal(membersApiStub.members.linkStripeCustomer.args[0][0].customer_id, 'cus_MdR9tqW6bAreiq');
|
||||
should.equal(membersApiStub.members.linkStripeCustomer.args[0][0].member_id, 'test_member_id');
|
||||
membersRepositoryStub.linkStripeCustomer.calledOnce.should.be.true();
|
||||
should.equal(membersRepositoryStub.linkStripeCustomer.args[0][0].customer_id, 'cus_MdR9tqW6bAreiq');
|
||||
should.equal(membersRepositoryStub.linkStripeCustomer.args[0][0].member_id, 'test_member_id');
|
||||
|
||||
// complimentary_plan import
|
||||
membersApiStub.members.update.calledOnce.should.be.true();
|
||||
should.deepEqual(membersApiStub.members.update.args[0][0].products, [{
|
||||
membersRepositoryStub.update.calledOnce.should.be.true();
|
||||
should.deepEqual(membersRepositoryStub.update.args[0][0].products, [{
|
||||
id: 'default_tier_id'
|
||||
}]);
|
||||
should.deepEqual(membersApiStub.members.update.args[0][1].id, 'test_member_id');
|
||||
should.deepEqual(membersRepositoryStub.update.args[0][1].id, 'test_member_id');
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -280,11 +277,11 @@ describe('Importer', function () {
|
|||
|
||||
const result = await importer.perform(`${csvPath}/single-column-with-header.csv`, defaultAllowedFields);
|
||||
|
||||
assert.equal(membersApiStub.members.create.args[0][0].email, 'jbloggs@example.com');
|
||||
assert.deepEqual(membersApiStub.members.create.args[0][0].labels, []);
|
||||
assert.equal(membersRepositoryStub.create.args[0][0].email, 'jbloggs@example.com');
|
||||
assert.deepEqual(membersRepositoryStub.create.args[0][0].labels, []);
|
||||
|
||||
assert.equal(membersApiStub.members.create.args[1][0].email, 'test@example.com');
|
||||
assert.deepEqual(membersApiStub.members.create.args[1][0].labels, []);
|
||||
assert.equal(membersRepositoryStub.create.args[1][0].email, 'test@example.com');
|
||||
assert.deepEqual(membersRepositoryStub.create.args[1][0].labels, []);
|
||||
|
||||
assert.equal(result.total, 2);
|
||||
assert.equal(result.imported, 2);
|
||||
|
|
@ -296,13 +293,13 @@ describe('Importer', function () {
|
|||
|
||||
const result = await importer.perform(`${csvPath}/subscribed-to-emails-header.csv`);
|
||||
|
||||
assert.equal(membersApiStub.members.create.args[0][0].email, 'jbloggs@example.com');
|
||||
assert.equal(membersApiStub.members.create.args[0][0].subscribed, true);
|
||||
assert.deepEqual(membersApiStub.members.create.args[0][0].labels, []);
|
||||
assert.equal(membersRepositoryStub.create.args[0][0].email, 'jbloggs@example.com');
|
||||
assert.equal(membersRepositoryStub.create.args[0][0].subscribed, true);
|
||||
assert.deepEqual(membersRepositoryStub.create.args[0][0].labels, []);
|
||||
|
||||
assert.equal(membersApiStub.members.create.args[1][0].email, 'test@example.com');
|
||||
assert.equal(membersApiStub.members.create.args[1][0].subscribed, false);
|
||||
assert.deepEqual(membersApiStub.members.create.args[1][0].labels, []);
|
||||
assert.equal(membersRepositoryStub.create.args[1][0].email, 'test@example.com');
|
||||
assert.equal(membersRepositoryStub.create.args[1][0].subscribed, false);
|
||||
assert.deepEqual(membersRepositoryStub.create.args[1][0].labels, []);
|
||||
|
||||
assert.equal(result.total, 2);
|
||||
assert.equal(result.imported, 2);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue