From d59eb6561c4f76e8d52dd21f6ae6b6f7823a0080 Mon Sep 17 00:00:00 2001 From: Ben Surgison Date: Wed, 12 Apr 2023 11:32:09 +0100 Subject: [PATCH] Fix migration script for layouts --- migrator/index.js | 20 +++--------- migrator/migration-steps.js | 29 +++++++++++++++-- migrator/migration-steps.spec.js | 56 +++++++++++++++++++++++--------- migrator/migrator.spec.js | 1 + 4 files changed, 73 insertions(+), 33 deletions(-) diff --git a/migrator/index.js b/migrator/index.js index f7fd895a6d..1063972e65 100644 --- a/migrator/index.js +++ b/migrator/index.js @@ -19,6 +19,7 @@ const { deleteIfUnchanged, removeOldPatternIncludesFromSassFile, updateUnbrandedLayouts, + upgradeLayoutIfUnchanged, upgradeIfPossible } = require('./migration-steps') @@ -81,6 +82,8 @@ const filesToDeleteIfUnchanged = [ 'app/assets/sass/unbranded.scss', 'app/views/includes/breadcrumb_examples.html', 'app/views/includes/cookie-banner.html', + 'app/views/includes/head.html', + 'app/views/includes/scripts.html', 'app/views/layout_unbranded.html' ] @@ -118,19 +121,6 @@ async function prepareMigration (kitDependency, projectDirectory) { ]) } -// Special case app/views/layout.html, as it has moved in prototype -// starter files, but we don't want to move for existing users -async function upgradeLayoutIfUnchanged () { - const results = await upgradeIfUnchanged( - ['app/views/layout.html'], - 'app/views/layouts/main.html', - () => deleteIfUnchanged([ - 'app/views/includes/head.html', - 'app/views/includes/scripts.html' - ])) - return results.flat() -} - async function migrate () { await logger.setup() @@ -143,8 +133,8 @@ async function migrate () { prepareSass('app/assets/sass/application.scss'), deleteUnusedFiles(filesToDelete), deleteUnusedDirectories(directoriesToDelete), - upgradeIfUnchanged(filesToUpdateIfUnchanged, '', upgradeIfPossible), - upgradeLayoutIfUnchanged(), + upgradeIfUnchanged(filesToUpdateIfUnchanged, upgradeIfPossible), + upgradeLayoutIfUnchanged('app/views/layout.html', 'app/views/layouts/main.html'), updateUnbrandedLayouts('app/views'), deleteIfUnchanged(filesToDeleteIfUnchanged), deleteIfUnchanged(patternsToDeleteIfUnchanged) diff --git a/migrator/migration-steps.js b/migrator/migration-steps.js index f1786ebfb2..dcde7c554a 100644 --- a/migrator/migration-steps.js +++ b/migrator/migration-steps.js @@ -206,8 +206,29 @@ async function deleteIfUnchanged (filePaths) { })) } -async function upgradeIfUnchanged (filePaths, starterFilePath, additionalStep) { - return Promise.all(filePaths.map(async filePath => { +// Special case app/views/layout.html, as it has moved in prototype +// starter files, but we don't want to move for existing users +async function upgradeLayoutIfUnchanged (filePath, starterFilePath) { + const matchFound = await matchAgainstOldVersions(filePath) + const reporter = await addReporter(`Overwrite ${filePath}`) + + let result = false + try { + if (matchFound) { + await copyFileFromStarter(starterFilePath, filePath) + result = true + } + } catch (e) { + await verboseLog(e.message) + await verboseLog(e.stack) + } + + await reporter(result) + return result +} + +async function upgradeIfUnchanged (filePaths, additionalStep) { + const results = await Promise.all(filePaths.map(async filePath => { const matchFound = await matchAgainstOldVersions(filePath) const reporter = await addReporter(`Overwrite ${filePath}`) @@ -215,7 +236,7 @@ async function upgradeIfUnchanged (filePaths, starterFilePath, additionalStep) { let result = false try { if (matchFound) { - await copyFileFromStarter(starterFilePath || filePath, filePath) + await copyFileFromStarter(filePath) } if (additionalStep) { result = await additionalStep(filePath, matchFound) @@ -230,6 +251,7 @@ async function upgradeIfUnchanged (filePaths, starterFilePath, additionalStep) { await reporter(result) return result })) + return results } async function updateUnbrandedLayouts (dir) { @@ -252,6 +274,7 @@ module.exports = { deleteUnusedDirectories, deleteEmptyDirectories, deleteIfUnchanged, + upgradeLayoutIfUnchanged, upgradeIfUnchanged, updateUnbrandedLayouts, upgradeIfPossible diff --git a/migrator/migration-steps.spec.js b/migrator/migration-steps.spec.js index d84ad9a0c1..24dd540fbf 100644 --- a/migrator/migration-steps.spec.js +++ b/migrator/migration-steps.spec.js @@ -49,14 +49,17 @@ const config = require('../lib/config') const { projectDir, starterDir, appDir } = require('../lib/utils/paths') const migrationSteps = require('./migration-steps') -const { preflightChecks, deleteIfUnchanged, removeOldPatternIncludesFromSassFile } = require('./migration-steps') const { + preflightChecks, + deleteIfUnchanged, + removeOldPatternIncludesFromSassFile, + upgradeIfUnchanged, + upgradeLayoutIfUnchanged, migrateConfig, prepareAppRoutes, prepareSass, deleteUnusedFiles, - deleteUnusedDirectories, - upgradeIfUnchanged + deleteUnusedDirectories } = migrationSteps describe('migration steps', () => { @@ -277,20 +280,46 @@ describe('migration steps', () => { expect(mockReporter).toHaveBeenNthCalledWith(3, true) }) + it('upgrade unchanged application.js and fail changed filters.js', async () => { + const appFile = 'application.js' + const filtersFile = 'filters.js' + const additionalStep = jest.fn() + .mockImplementationOnce(async () => true) + .mockImplementationOnce(async () => false) + const result = await upgradeIfUnchanged([appFile, filtersFile], additionalStep) + + expect(result).toEqual([true, false]) + + expect(fileHelpers.matchAgainstOldVersions).toHaveBeenCalledTimes(2) + expect(fileHelpers.matchAgainstOldVersions.mock.calls).toEqual([ + [appFile], // First call + [filtersFile] // Second call + ]) + + expect(reporter.addReporter).toHaveBeenCalledTimes(2) + expect(reporter.addReporter.mock.calls).toEqual([ + [`Overwrite ${appFile}`], // First call + [`Overwrite ${filtersFile}`] // Second call + ]) + + expect(mockReporter).toHaveBeenCalledTimes(2) + expect(mockReporter.mock.calls).toEqual([ + [true], // First call + [false] // Second call + ]) + }) + it('upgrade if unchanged layout', async () => { const layout = 'app/views/layout.html' - const additionalStep = jest.fn().mockResolvedValue(true) + const starterLayout = 'app/views/layouts/main.html' - const result = await upgradeIfUnchanged([layout], layout, additionalStep) + const result = await upgradeLayoutIfUnchanged(layout, starterLayout) expect(result).toBeTruthy() expect(fileHelpers.matchAgainstOldVersions).toHaveBeenCalledTimes(1) expect(fileHelpers.matchAgainstOldVersions).toHaveBeenCalledWith(layout) - expect(additionalStep).toHaveBeenCalledTimes(1) - expect(additionalStep).toHaveBeenCalled() - expect(reporter.addReporter).toHaveBeenCalledTimes(1) expect(reporter.addReporter).toHaveBeenCalledWith(`Overwrite ${layout}`) @@ -300,25 +329,22 @@ describe('migration steps', () => { it('report if changed layout', async () => { const layout = 'app/views/layout.html' - const additionalStep = jest.fn().mockResolvedValue(true) + const starterLayout = 'app/views/layouts/main.html' fileHelpers.matchAgainstOldVersions.mockReturnValue(false) - const result = await upgradeIfUnchanged([layout], layout, additionalStep) + const result = await upgradeLayoutIfUnchanged(layout, starterLayout) - expect(result).toBeTruthy() + expect(result).toBeFalsy() expect(fileHelpers.matchAgainstOldVersions).toHaveBeenCalledTimes(1) expect(fileHelpers.matchAgainstOldVersions).toHaveBeenCalledWith(layout) - expect(additionalStep).toHaveBeenCalledTimes(1) - expect(additionalStep).toHaveBeenCalled() - expect(reporter.addReporter).toHaveBeenCalledTimes(1) expect(reporter.addReporter).toHaveBeenCalledWith(`Overwrite ${layout}`) expect(mockReporter).toHaveBeenCalledTimes(1) - expect(mockReporter).toHaveBeenCalledWith(true) + expect(mockReporter).toHaveBeenCalledWith(false) }) it('delete if unchanged unbranded layout', async () => { diff --git a/migrator/migrator.spec.js b/migrator/migrator.spec.js index fb3820d66b..f05f82934d 100644 --- a/migrator/migrator.spec.js +++ b/migrator/migrator.spec.js @@ -21,6 +21,7 @@ jest.mock('./migration-steps', () => { deleteUnusedDirectories: jest.fn().mockResolvedValue(true), deleteEmptyDirectories: jest.fn().mockResolvedValue([true]), upgradeIfUnchanged: jest.fn(), + upgradeLayoutIfUnchanged: jest.fn().mockResolvedValue(true), upgradeIfPossible: jest.fn(), updateUnbrandedLayouts: jest.fn().mockResolvedValue(true), deleteIfUnchanged: jest.fn().mockResolvedValue([true, true, true]),