0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00

Removed cleaned up use of "Job" object

refs https://github.com/TryGhost/Toolbox/issues/430

- Importer code was filled with an unnecessarily complex "job" object that was passed around. It had an "id" property, which confusingly was a path to a file at all times.
- Simplified the logic significantly by keeping and passing around the path to a "prepared" members CSV.
This commit is contained in:
Naz 2022-10-13 15:04:22 +08:00
parent 812973d962
commit 8dc630b3a0
No known key found for this signature in database
2 changed files with 9 additions and 33 deletions

View file

@ -37,28 +37,6 @@ module.exports = class MembersCSVImporter {
this._context = context; this._context = context;
} }
/**
* @typedef {string} JobID
*/
/**
* @typedef {Object} Job
* @prop {string} filename
* @prop {JobID} id
*/
/**
* Get the Job for a jobCode
* @param {JobID} jobId
* @returns {Promise<Job>}
*/
async getJob(jobId) {
return {
id: jobId,
filename: jobId
};
}
/** /**
* Prepares a CSV file for import * Prepares a CSV file for import
* - Maps headers based on headerMapping, this allows for a non standard CSV * - Maps headers based on headerMapping, this allows for a non standard CSV
@ -70,7 +48,7 @@ module.exports = class MembersCSVImporter {
* @param {Object.<string, string>} headerMapping - An object whose keys are headers in the input CSV and values are the header to replace it with * @param {Object.<string, string>} headerMapping - An object whose keys are headers in the input CSV and values are the header to replace it with
* @param {Array<string>} defaultLabels - A list of labels to apply to every member * @param {Array<string>} defaultLabels - A list of labels to apply to every member
* *
* @returns {Promise<{id: JobID, batches: number, metadata: Object.<string, any>}>} - A promise resolving to the id of the MemberImport Job * @returns {Promise<{filePath: string, batches: number, metadata: Object.<string, any>}>} - A promise resolving to the data including filePath of "prepared" CSV
*/ */
async prepare(inputFilePath, headerMapping, defaultLabels) { async prepare(inputFilePath, headerMapping, defaultLabels) {
// @NOTE: investigate why is it "1" and do we even need this concept anymore? // @NOTE: investigate why is it "1" and do we even need this concept anymore?
@ -102,7 +80,7 @@ module.exports = class MembersCSVImporter {
await fs.writeFile(outputFilePath, mappedCSV); await fs.writeFile(outputFilePath, mappedCSV);
return { return {
id: outputFilePath, filePath: outputFilePath,
batches: numberOfBatches, batches: numberOfBatches,
metadata: { metadata: {
hasStripeData hasStripeData
@ -113,12 +91,10 @@ module.exports = class MembersCSVImporter {
/** /**
* Performs an import of a CSV file * Performs an import of a CSV file
* *
* @param {JobID} id - The id of the job to perform * @param {string} filePath - the path to a "prepared" CSV file
*/ */
async perform(id) { async perform(filePath) {
const job = await this.getJob(id); const rows = membersCSV.parse(filePath);
const rows = membersCSV.parse(job.filename);
const membersApi = await this._getMembersApi(); const membersApi = await this._getMembersApi();
@ -306,7 +282,7 @@ module.exports = class MembersCSVImporter {
meta.originalImportSize = job.batches; meta.originalImportSize = job.batches;
if (job.batches <= 500 && !job.metadata.hasStripeData) { if (job.batches <= 500 && !job.metadata.hasStripeData) {
const result = await this.perform(job.id); const result = await this.perform(job.filePath);
const importLabelModel = result.imported ? await LabelModel.findOne(importLabel) : null; const importLabelModel = result.imported ? await LabelModel.findOne(importLabel) : null;
return { return {
@ -322,7 +298,7 @@ module.exports = class MembersCSVImporter {
const emailRecipient = user.email; const emailRecipient = user.email;
this._addJob({ this._addJob({
job: async () => { job: async () => {
const result = await this.perform(job.id); const result = await this.perform(job.filePath);
const importLabelModel = result.imported ? await LabelModel.findOne(importLabel) : null; const importLabelModel = result.imported ? await LabelModel.findOne(importLabel) : null;
const emailContent = this.generateCompletionEmail(result, { const emailContent = this.generateCompletionEmail(result, {
emailRecipient, emailRecipient,

View file

@ -148,8 +148,8 @@ describe('Importer', function () {
const result = await membersImporter.prepare(`${csvPath}/single-column-with-header.csv`); const result = await membersImporter.prepare(`${csvPath}/single-column-with-header.csv`);
should.exist(result.id); should.exist(result.filePath);
result.id.should.match(/\/members-importer\/test\/fixtures\/Members Import/); result.filePath.should.match(/\/members-importer\/test\/fixtures\/Members Import/);
result.batches.should.equal(2); result.batches.should.equal(2);
should.exist(result.metadata); should.exist(result.metadata);