From 9dcc17a01728046089d3f1ede9203aebf8df6603 Mon Sep 17 00:00:00 2001 From: Naz Gargol Date: Tue, 16 Jul 2019 12:01:44 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20import=20for=20tag=20wit?= =?UTF-8?q?hout=20slugs=20that=20belongs=20to=20a=20post=20(#10917)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #10785 - The behavior for tags will now be similar to posts' one described in the docs - "The only strictly required field when importing posts is the title. Ghost will automatically generate slugs and set every other field to the default or empty." - The breaking change was introduced with: https://github.com/TryGhost/Ghost/commit/68d8154d4fb90f6534a652a5e56f92ee3882c782#diff-e712df50c0dc7cf33746eeff0564003cR97 (assumed there's always slug in the imported object which is not true) - Added originalIdMap to the importer base class to track id substitution so it can be used when dealing with relational resource updates - Removed explicit use of 'this.stripProperties(['id']);' in beforeImport of base class because we need to assign and remove the id property in the same place to track this change - Only calling 'this.stripProperties(['id']);' in settings/trusted_domain imports as the method won't be called otherwise - Expanded regression tests with new supported import case --- .../data/importer/importers/data/base.js | 13 +++-- .../data/importer/importers/data/posts.js | 6 ++- .../data/importer/importers/data/settings.js | 1 + .../data/importer/importers/data/tags.js | 1 + .../importers/data/trusted-domains.js | 1 + .../test/regression/importer/importer_spec.js | 49 +++++++++++++++++++ 6 files changed, 67 insertions(+), 4 deletions(-) diff --git a/core/server/data/importer/importers/data/base.js b/core/server/data/importer/importers/data/base.js index bdea18af6e..8057829dd7 100644 --- a/core/server/data/importer/importers/data/base.js +++ b/core/server/data/importer/importers/data/base.js @@ -21,6 +21,7 @@ class Base { this.dataKeyToImport = options.dataKeyToImport; this.dataToImport = _.cloneDeep(allDataFromFile[this.dataKeyToImport] || []); + this.originalIdMap = {}; this.importedDataToReturn = []; this.importedData = []; @@ -53,7 +54,7 @@ class Base { } /** - * Never ever import these attributes! + * Strips attributes of the object */ stripProperties(properties) { _.each(this.dataToImport, (obj) => { @@ -86,7 +87,13 @@ class Base { generateIdentifier() { _.each(this.dataToImport, (obj) => { - obj.id = ObjectId.generate(); + const newId = ObjectId.generate(); + + if (obj.id) { + this.originalIdMap[newId] = obj.id; + } + + obj.id = newId; }); } @@ -95,7 +102,6 @@ class Base { } beforeImport() { - this.stripProperties(['id']); this.sanitizeValues(); this.generateIdentifier(); return Promise.resolve(); @@ -317,6 +323,7 @@ class Base { // for identifier lookup this.importedData.push({ id: importedModel.id, + originalId: this.originalIdMap[importedModel.id], slug: importedModel.get('slug'), originalSlug: obj.slug, email: importedModel.get('email') diff --git a/core/server/data/importer/importers/data/posts.js b/core/server/data/importer/importers/data/posts.js index 2ea0caeb00..e7008ed429 100644 --- a/core/server/data/importer/importers/data/posts.js +++ b/core/server/data/importer/importers/data/posts.js @@ -100,7 +100,11 @@ class PostsImporter extends BaseImporter { // CASE: search through imported data. // EDGE CASE: uppercase tag slug was imported and auto modified - let importedObject = _.find(this.requiredImportedData[tableName], {originalSlug: objectInFile.slug}); + let importedObject = null; + + if (objectInFile.id) { + importedObject = _.find(this.requiredImportedData[tableName], {originalId: objectInFile.id}); + } if (importedObject) { this.dataToImport[postIndex][targetProperty][index].id = importedObject.id; diff --git a/core/server/data/importer/importers/data/settings.js b/core/server/data/importer/importers/data/settings.js index 187de54ca9..b0d410c3c7 100644 --- a/core/server/data/importer/importers/data/settings.js +++ b/core/server/data/importer/importers/data/settings.js @@ -109,6 +109,7 @@ class SettingsImporter extends BaseImporter { } generateIdentifier() { + this.stripProperties(['id']); return Promise.resolve(); } diff --git a/core/server/data/importer/importers/data/tags.js b/core/server/data/importer/importers/data/tags.js index ffd8ce6556..98314beba3 100644 --- a/core/server/data/importer/importers/data/tags.js +++ b/core/server/data/importer/importers/data/tags.js @@ -58,6 +58,7 @@ class TagsImporter extends BaseImporter { // for identifier lookup this.importedData.push({ id: importedModel.id, + originalId: this.originalIdMap[importedModel.id], slug: importedModel.get('slug'), originalSlug: obj.slug }); diff --git a/core/server/data/importer/importers/data/trusted-domains.js b/core/server/data/importer/importers/data/trusted-domains.js index 5cec76992c..d52d64a6d1 100644 --- a/core/server/data/importer/importers/data/trusted-domains.js +++ b/core/server/data/importer/importers/data/trusted-domains.js @@ -75,6 +75,7 @@ class TrustedDomainsImporter extends BaseImporter { } generateIdentifier() { + this.stripProperties(['id']); return Promise.resolve(); } diff --git a/core/test/regression/importer/importer_spec.js b/core/test/regression/importer/importer_spec.js index 6f0393133a..9d69806a01 100644 --- a/core/test/regression/importer/importer_spec.js +++ b/core/test/regression/importer/importer_spec.js @@ -809,6 +809,55 @@ describe('Integration: Importer', function () { }); }); + it('can handle related tags with missing optional fields', function () { + const exportData = exportedLatestBody().db[0]; + + exportData.data.posts[0] = testUtils.DataGenerator.forKnex.createPost(); + + // NOTE: not including slug, description etc. fields as the only required field + // to handle the import of tags is 'name' + exportData.data.tags[0] = { + id: ObjectId.generate(), + name: 'first tag' + }; + exportData.data.tags[1] = { + id: ObjectId.generate(), + name: 'second tag' + }; + exportData.data.tags[2] = { + id: ObjectId.generate(), + name: 'third tag' + }; + + exportData.data.posts_tags = [ + testUtils.DataGenerator.forKnex.createPostsTags(exportData.data.posts[0].id, exportData.data.tags[0].id), + testUtils.DataGenerator.forKnex.createPostsTags(exportData.data.posts[0].id, exportData.data.tags[1].id), + testUtils.DataGenerator.forKnex.createPostsTags(exportData.data.posts[0].id, exportData.data.tags[2].id) + ]; + + return dataImporter.doImport(exportData, importOptions) + .then(function (imported) { + imported.problems.length.should.eql(0); + + return Promise.all([ + models.Tag.findPage(Object.assign({order: 'slug ASC'}, testUtils.context.internal)), + models.Post.findPage(Object.assign({withRelated: ['tags']}, testUtils.context.internal)) + ]); + }).then(function (result) { + const tags = result[0].data.map(model => model.toJSON()); + const posts = result[1].data.map(model => model.toJSON()); + + posts.length.should.eql(1); + tags.length.should.eql(3); + + posts[0].tags.length.should.eql(3); + tags[0].name.should.eql('first tag'); + tags[0].slug.should.eql('first-tag'); + tags[1].name.should.eql('second tag'); + tags[2].name.should.eql('third tag'); + }); + }); + it('can handle uppercase tags', function () { const exportData = exportedLatestBody().db[0];