From c16abbf08599a61044a9060b9c14a89a0088c29f Mon Sep 17 00:00:00 2001
From: Rishabh Garg <zrishabhgarg@gmail.com>
Date: Wed, 24 Aug 2022 00:49:30 +0530
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20empty=20error=20csv=20fi?=
 =?UTF-8?q?le=20for=20member=20imports=20(#15274)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

closes https://github.com/TryGhost/Team/issues/1828

- members importer was sending empty error files in case of invalid rows in CSV, hiding both error and affected rows
- fixes typo in `content` option(was `contents` before) passed to attachment (ref - https://nodemailer.com/message/attachments )
---
 ghost/members-importer/lib/importer.js       | 45 ++++++++++---
 ghost/members-importer/test/importer.test.js | 71 +++++++++++++++++++-
 2 files changed, 104 insertions(+), 12 deletions(-)

diff --git a/ghost/members-importer/lib/importer.js b/ghost/members-importer/lib/importer.js
index c6558b6b7a..67cee8faac 100644
--- a/ghost/members-importer/lib/importer.js
+++ b/ghost/members-importer/lib/importer.js
@@ -258,6 +258,34 @@ module.exports = class MembersCSVImporter {
         return membersCSV.unparse(errorsWithFormattedMessages);
     }
 
+    /**
+     * Send email with attached CSV containing error rows info
+     *
+     * @param {Object} config
+     * @param {String} config.emailRecipient - email recipient for error file
+     * @param {String} config.emailSubject - email subject
+     * @param {String} config.emailContent - html content of email
+     * @param {String} config.errorCSV - error CSV content
+     * @param {Object} config.emailSubject - email subject
+     * @param {Object} config.importLabel -
+     * @param {String} config.importLabel.name - label name
+     */
+    async sendErrorEmail({emailRecipient, emailSubject, emailContent, errorCSV, importLabel}) {
+        await this._sendEmail({
+            to: emailRecipient,
+            subject: emailSubject,
+            html: emailContent,
+            forceTextContent: true,
+            attachments: [{
+                filename: `${importLabel.name} - Errors.csv`,
+                content: errorCSV,
+                contentType: 'text/csv',
+                contentDisposition: 'attachment'
+            }]
+        });
+        return;
+    }
+
     /**
      * Processes CSV file and imports member&label records depending on the size of the imported set
      *
@@ -303,17 +331,12 @@ module.exports = class MembersCSVImporter {
                     const errorCSV = this.generateErrorCSV(result);
                     const emailSubject = result.imported > 0 ? 'Your member import is complete' : 'Your member import was unsuccessful';
 
-                    await this._sendEmail({
-                        to: emailRecipient,
-                        subject: emailSubject,
-                        html: emailContent,
-                        forceTextContent: true,
-                        attachments: [{
-                            filename: `${importLabel.name} - Errors.csv`,
-                            contents: errorCSV,
-                            contentType: 'text/csv',
-                            contentDisposition: 'attachment'
-                        }]
+                    await this.sendErrorEmail({
+                        emailRecipient,
+                        emailSubject,
+                        emailContent,
+                        errorCSV,
+                        importLabel
                     });
                 },
                 offloaded: false
diff --git a/ghost/members-importer/test/importer.test.js b/ghost/members-importer/test/importer.test.js
index 661c5f91b6..ad515554c6 100644
--- a/ghost/members-importer/test/importer.test.js
+++ b/ghost/members-importer/test/importer.test.js
@@ -17,7 +17,7 @@ describe('Importer', function () {
     });
 
     afterEach(function () {
-        const writtenFile = fsWriteSpy.args[0][0];
+        const writtenFile = fsWriteSpy.args?.[0]?.[0];
 
         if (writtenFile) {
             fs.removeSync(writtenFile);
@@ -103,6 +103,75 @@ describe('Importer', function () {
         });
     });
 
+    describe('sendErrorEmail', function () {
+        it('should send email with errors for invalid CSV file', async function () {
+            const defaultProduct = {
+                id: 'default_product_id'
+            };
+
+            const memberCreateStub = sinon.stub().resolves(null);
+            const membersApi = {
+                productRepository: {
+                    list: async () => {
+                        return {
+                            data: [defaultProduct]
+                        };
+                    }
+                },
+                members: {
+                    get: async () => {
+                        return null;
+                    },
+                    create: memberCreateStub
+                }
+            };
+
+            const knexStub = {
+                transaction: sinon.stub().resolves({
+                    rollback: () => {},
+                    commit: () => {}
+                })
+            };
+
+            const sendEmailStub = sinon.stub();
+
+            const importer = new MembersCSVImporter({
+                storagePath: csvPath,
+                getTimezone: sinon.stub().returns('UTC'),
+                getMembersApi: () => membersApi,
+                sendEmail: sendEmailStub,
+                isSet: sinon.stub(),
+                addJob: sinon.stub(),
+                knex: knexStub,
+                urlFor: sinon.stub(),
+                context: {importer: true}
+            });
+
+            await importer.sendErrorEmail({
+                emailRecipient: 'test@example.com',
+                emailSubject: 'Your member import was unsuccessful',
+                emailContent: 'Import was unsuccessful',
+                errorCSV: 'id,email,invalid email',
+                importLabel: {name: 'Test import'}
+            });
+
+            sendEmailStub.calledWith({
+                to: 'test@example.com',
+                subject: 'Your member import was unsuccessful',
+                html: 'Import was unsuccessful',
+                forceTextContent: true,
+                attachments: [
+                    {
+                        filename: 'Test import - Errors.csv',
+                        content: 'id,email,invalid email',
+                        contentType: 'text/csv',
+                        contentDisposition: 'attachment'
+                    }
+                ]
+            }).should.be.true();
+        });
+    });
+
     describe('prepare', function () {
         it('processes a basic valid import file for members', async function () {
             const membersImporter = new MembersCSVImporter({