From 854a41e556e1262368e9485262f554ba3a5604d7 Mon Sep 17 00:00:00 2001
From: Fabien 'egg' O'Carroll <fabien@allou.is>
Date: Tue, 29 Sep 2020 12:51:35 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20CSV=20import=20json-sche?=
 =?UTF-8?q?ma=20email=20validation=20(#12239)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

no-issue

By using the "email" validation, we were validating emails in CSV
imports using a different validator to the rest of the API. AJV's built
in email validation was failing on emails with "special" characters,
such as letters with an umlaut above them.

This commit brings the validation for CSV imports in line with the rest
of the API.
---
 .../input/schemas/members-upload.json         |  2 +-
 .../api/canary/admin/members_spec.js          | 35 +++++++++++++------
 .../fixtures/csv/members-invalid-values.csv   |  5 +--
 3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/core/server/api/canary/utils/validators/input/schemas/members-upload.json b/core/server/api/canary/utils/validators/input/schemas/members-upload.json
index 8c1712f73a..53a9af6b7f 100644
--- a/core/server/api/canary/utils/validators/input/schemas/members-upload.json
+++ b/core/server/api/canary/utils/validators/input/schemas/members-upload.json
@@ -19,7 +19,7 @@
                 "type": "string",
                 "minLength": 1,
                 "maxLength": 191,
-                "format": "email"
+                "pattern": "^([^,]|$)"
             },
             "note": {
                 "type": ["string", "null"],
diff --git a/test/regression/api/canary/admin/members_spec.js b/test/regression/api/canary/admin/members_spec.js
index 76c2460224..e7a1d593d7 100644
--- a/test/regression/api/canary/admin/members_spec.js
+++ b/test/regression/api/canary/admin/members_spec.js
@@ -474,20 +474,35 @@ describe('Members API', function () {
                 should.exist(jsonResponse.meta.stats);
 
                 jsonResponse.meta.stats.imported.count.should.equal(0);
-                jsonResponse.meta.stats.invalid.count.should.equal(2);
+                jsonResponse.meta.stats.invalid.count.should.equal(3);
 
-                should.equal(jsonResponse.meta.stats.invalid.errors.length, 4);
-                jsonResponse.meta.stats.invalid.errors[0].message.should.equal('Validation failed for \'name\'.');
-                jsonResponse.meta.stats.invalid.errors[0].count.should.equal(1);
+                const validationErrors = jsonResponse.meta.stats.invalid.errors;
 
-                jsonResponse.meta.stats.invalid.errors[1].message.should.equal('Validation failed for \'email\'.');
-                jsonResponse.meta.stats.invalid.errors[1].count.should.equal(2);
+                should.equal(validationErrors.length, 4);
 
-                jsonResponse.meta.stats.invalid.errors[2].message.should.equal('Validation failed for \'created_at\'.');
-                jsonResponse.meta.stats.invalid.errors[2].count.should.equal(1);
+                const nameValidationErrors = validationErrors.find(
+                    obj => obj.message === 'Validation failed for \'name\'.'
+                );
+                should.exist(nameValidationErrors);
+                nameValidationErrors.count.should.equal(1);
 
-                jsonResponse.meta.stats.invalid.errors[3].message.should.equal('Validation failed for \'complimentary_plan\'.');
-                jsonResponse.meta.stats.invalid.errors[3].count.should.equal(1);
+                const emailValidationErrors = validationErrors.find(
+                    obj => obj.message === 'Validation (isEmail) failed for email'
+                );
+                should.exist(emailValidationErrors);
+                emailValidationErrors.count.should.equal(1);
+
+                const createdAtValidationErrors = validationErrors.find(
+                    obj => obj.message === 'Validation failed for \'created_at\'.'
+                );
+                should.exist(createdAtValidationErrors);
+                createdAtValidationErrors.count.should.equal(1);
+
+                const compedPlanValidationErrors = validationErrors.find(
+                    obj => obj.message === 'Validation failed for \'complimentary_plan\'.'
+                );
+                should.exist(compedPlanValidationErrors);
+                compedPlanValidationErrors.count.should.equal(1);
 
                 should.exist(jsonResponse.meta.import_label);
                 jsonResponse.meta.import_label.slug.should.equal('new-global-label');
diff --git a/test/utils/fixtures/csv/members-invalid-values.csv b/test/utils/fixtures/csv/members-invalid-values.csv
index 49511a8900..059ec54e18 100644
--- a/test/utils/fixtures/csv/members-invalid-values.csv
+++ b/test/utils/fixtures/csv/members-invalid-values.csv
@@ -1,3 +1,4 @@
 email,name,note,subscribed_to_emails,complimentary_plan,stripe_customer_id,created_at,labels
-invalid_email_value1,",name starting with coma",,not_boolean,false,,not_a_date,labels
-invalid_email_value2,"good name",,true,not_boolean,,2019-10-30T14:52:08.000Z,more-labels
+valid@email.com,",name starting with coma",,not_boolean,false,,not_a_date,labels
+valid2@email.com,"good name",,true,not_boolean,,2019-10-30T14:52:08.000Z,more-labels
+invalid_email_value,"good name",,true,false,,2019-10-30T14:52:08.000Z,more-labels