Merge pull request #1752 from alphagov/ldeb-layouts-folder

Move default user template to app/views/layouts/main.html
alphagov · Nov 8, 2022 · 84230ad · 84230ad
2 parents b35eacd + 20ce4ec
commit 84230ad
Show file tree

Hide file tree

Showing 24 changed files with 82 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Unreleased
 
 ### Breaking changes
+- [#1752: Move default user template to app/views/layouts/main.html](https://github.com/alphagov/govuk-prototype-kit/pull/1752)
 - [#1640: Fixing govuk frontend until they release update](https://github.com/alphagov/govuk-prototype-kit/pull/1640) We have made the plugin framework powerful enough to handle everything govuk-frontend needs but they haven't yet implemented their side of this, this change allows current and previous versions of govuk-frontend to work with the kit.  You'll need to import components before you can use them until govuk-frontend release a version which imports them automatically.  
 - [#1648: Change default session store to cookie store when in production mode](https://github.com/alphagov/govuk-prototype-kit/pull/1648) When hosted online the kit will now preserve user session data between server restarts by default.
 - [#1658: Use file store for session data](https://github.com/alphagov/govuk-prototype-kit/pull/1658)

diff --git a/__tests__/spec/migrate.js b/__tests__/spec/migrate.js
@@ -32,10 +32,6 @@ describe('migrate test prototype', () => {
     })
   }, 120000)
 
-  afterAll(() => {
-    fse.removeSync(projectDirectory)
-  })
-
   it('config.js to config.json', () => {
     const config = fse.readJsonSync(path.join(appDirectory, 'config.json'))
 

diff --git a/cypress/e2e/dev/5-extension-tests/install-extension-test.cypress.js b/cypress/e2e/dev/5-extension-tests/install-extension-test.cypress.js
@@ -11,7 +11,7 @@ const CYAN = 'rgb(0, 255, 255)'
 const MAGENTA = 'rgb(255, 0, 255)'
 
 const extensionBazViewMarkup = `
-{% extends "layout.html" %}
+{% extends "layouts/main.html" %}
 
 {% block content %}
 {% include "baz.njk" %}

diff --git a/cypress/e2e/dev/5-extension-tests/multi-combined-extension-test.cypress.js b/cypress/e2e/dev/5-extension-tests/multi-combined-extension-test.cypress.js
@@ -10,7 +10,7 @@ const YELLOW = 'rgb(255, 255, 0)'
 const BLUE = 'rgb(0, 0, 255)'
 
 const extensionFooBarCombinedViewMarkup = `
-{% extends "layout.html" %}
+{% extends "layouts/main.html" %}
 
 {% block content %}
 {% set testVar="Hello" %}

diff --git a/cypress/e2e/dev/5-extension-tests/multi-extension-test.cypress.js b/cypress/e2e/dev/5-extension-tests/multi-extension-test.cypress.js
@@ -11,7 +11,7 @@ const YELLOW = 'rgb(255, 255, 0)'
 const BLUE = 'rgb(0, 0, 255)'
 
 const extensionFooBarSeparatedViewMarkup = `
-{% extends "layout.html" %}
+{% extends "layouts/main.html" %}
 
 {% block content %}
 {% include "bar.njk" %}

diff --git a/cypress/e2e/dev/5-extension-tests/single-extension-test.cypress.js b/cypress/e2e/dev/5-extension-tests/single-extension-test.cypress.js
@@ -9,7 +9,7 @@ const YELLOW = 'rgb(255, 255, 0)'
 const BLUE = 'rgb(0, 0, 255)'
 
 const extensionFooViewMarkup = `
-{% extends "layout.html" %}
+{% extends "layouts/main.html" %}
 
 {% block content %}
 {% include "foo.njk" %}

diff --git a/cypress/e2e/dev/7-layout-tests/default-layout.cypress.js b/cypress/e2e/dev/7-layout-tests/default-layout.cypress.js
@@ -0,0 +1,34 @@
+const path = require('path')
+
+const { waitForApplication } = require('../../utils')
+
+const defaultLayoutFilePath = path.join('app', 'views', 'layouts', 'main.html')
+const backupLayoutComment = '<!-- could not find layouts/main.html in prototype, using backup default template -->'
+
+const comments = el => cy.wrap(
+  [...el.childNodes]
+    .filter(node => node.nodeName === '#comment')
+    .map(commentNode => '<!--' + commentNode.data + '-->')
+)
+
+after(() => {
+  cy.task('copyFromStarterFiles', { filename: defaultLayoutFilePath })
+  waitForApplication()
+})
+
+specify('deleting default layout does not cause pages to fail to render', () => {
+  cy.visit('/')
+
+  cy.document().then(doc =>
+    comments(doc.head).should('not.contain', backupLayoutComment)
+  )
+
+  cy.task('deleteFile', { filename: path.join(Cypress.env('projectFolder'), defaultLayoutFilePath) })
+
+  cy.visit('/', { failOnStatusCode: false })
+  cy.get('body').should('not.contains.text', 'Error: template not found')
+
+  cy.document().then(doc =>
+    comments(doc.head).should('contain', backupLayoutComment)
+  )
+})
diff --git a/cypress/fixtures/views/larry-the-cat.html b/cypress/fixtures/views/larry-the-cat.html
@@ -1,4 +1,4 @@
-{% extends "layout.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   Larry the cat – {{ serviceName }} – GOV.UK Prototype Kit

diff --git a/cypress/plugins/index.js b/cypress/plugins/index.js
@@ -19,6 +19,7 @@ const waitOn = require('wait-on')
 const extract = require('extract-zip')
 const https = require('https')
 
+const { starterDir } = require('../../lib/path-utils')
 const { sleep } = require('../e2e/utils')
 
 const log = (message) => console.log(`${new Date().toLocaleTimeString()} => ${message}`)
@@ -193,6 +194,17 @@ module.exports = (on, config) => {
       .then(() => sleep(2000)) // pause after the copy
       .then(makeSureCypressCanInterpretTheResult),
 
+    copyFromStarterFiles: ({ starterFilename = undefined, filename }) => {
+      const src = path.join(starterDir, starterFilename || filename)
+      const dest = path.join(config.env.projectFolder, filename)
+      return createFolderForFile(dest)
+        .then(() => fsp.copyFile(src, dest))
+        // The sleep of 2 seconds allows for the file to be copied completely to prevent
+        // it from not existing when the file is needed in a subsequent step
+        .then(() => sleep(2000)) // pause after the copy
+        .then(makeSureCypressCanInterpretTheResult)
+    },
+
     createFile: ({ filename, data, replace = false }) => createFolderForFile(filename)
       .then(() => fsp.writeFile(filename, data, {
         flag: replace ? 'w' : '' // Flag of w will overwrite

diff --git a/lib/migrator/fileHelpers.js b/lib/migrator/fileHelpers.js
@@ -98,11 +98,11 @@ const deleteDirectoryIfEmpty = async (partialPath) => {
   return false
 }
 
-const copyFileFromStarter = async (partialPath) => {
-  const src = path.join(packageDir, 'prototype-starter', partialPath)
-  const dest = path.join(projectDir, partialPath)
+const copyFileFromStarter = async (starterPath, newPath) => {
+  const src = path.join(packageDir, 'prototype-starter', starterPath)
+  const dest = path.join(projectDir, newPath || starterPath)
   await verboseLog(`copying from [${src}] to [${dest}]`)
-  return fs.copyFile(src, dest)
+  return fse.copy(src, dest)
     .then(successOutput)
     .catch(async e => {
       await log('error caught')

diff --git a/lib/migrator/index.js b/lib/migrator/index.js
@@ -62,8 +62,7 @@ const directoriesToDeleteIfEmpty = [
 
 const filesToUpdateIfUnchanged = [
   'app/assets/javascripts/application.js',
-  'app/filters.js',
-  'app/views/layout.html'
+  'app/filters.js'
 ]
 
 const filesToDeleteIfUnchanged = [
@@ -125,6 +124,11 @@ const migrate = async () => {
       upgradeIfUnchanged([
         ...filesToUpdateIfUnchanged.map(filePath => ({ filePath, action: 'copyFromKitStarter' })),
         ...filesToDeleteIfUnchanged.map(filePath => ({ filePath, action: 'delete' }))
+      ]),
+      upgradeIfUnchanged([
+        // Special case app/views/layout.html, as it has moved in prototype
+        // starter files, but we don't want to move for existing users
+        { filePath: 'app/views/layout.html', starterFilePath: 'app/views/layouts/main.html', action: 'copyFromKitStarter' }
       ])
     ])
 

diff --git a/lib/migrator/migrationSteps.js b/lib/migrator/migrationSteps.js
@@ -134,7 +134,7 @@ module.exports.upgradeIfUnchanged = (configs) => Promise.all(configs.map(async c
       return
     }
     try {
-      await copyFileFromStarter(config.filePath)
+      await copyFileFromStarter(config.starterFilePath || config.filePath, config.filePath)
       if (config.filePath === 'app/views/layout.html') {
         await module.exports.upgradeIfUnchanged([
           {

diff --git a/lib/nunjucks/layouts/main.html b/lib/nunjucks/layouts/main.html
@@ -0,0 +1,8 @@
+{# this file exists as a backup in case the user has deleted their `app/views/layouts/main.html` template #}
+
+{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+
+{% block meta %}
+  <!-- could not find layouts/main.html in prototype, using backup default template -->
+  {{ super() }}
+{% endblock %}
diff --git a/lib/templates/blank-govuk.html b/lib/templates/blank-govuk.html
@@ -1,4 +1,4 @@
-{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   GOV.UK page template – {{ serviceName }} – GOV.UK Prototype Kit

diff --git a/lib/templates/check-answers.html b/lib/templates/check-answers.html
@@ -1,4 +1,4 @@
-{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   Check your answers template – {{ serviceName }} – GOV.UK Prototype Kit

diff --git a/lib/templates/confirmation.html b/lib/templates/confirmation.html
@@ -1,4 +1,4 @@
-{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   Confirmation page template – {{ serviceName }} – GOV.UK Prototype Kit

diff --git a/lib/templates/content.html b/lib/templates/content.html
@@ -1,4 +1,4 @@
-{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   Content page template – {{ serviceName }} – GOV.UK Prototype Kit

diff --git a/lib/templates/mainstream-guide.html b/lib/templates/mainstream-guide.html
@@ -1,4 +1,4 @@
-{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   Mainstream guide example

diff --git a/lib/templates/question.html b/lib/templates/question.html
@@ -1,4 +1,4 @@
-{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   Question page template – {{ serviceName }} – GOV.UK Prototype Kit

diff --git a/lib/templates/start.html b/lib/templates/start.html
@@ -1,4 +1,4 @@
-{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   Start page template – {{ serviceName }} – GOV.UK Prototype Kit

diff --git a/lib/templates/task-list.html b/lib/templates/task-list.html
@@ -1,4 +1,4 @@
-{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
   Task list template – {{ serviceName }} – GOV.UK Prototype Kit

diff --git a/prototype-starter/app/views/index.html b/prototype-starter/app/views/index.html
@@ -1,4 +1,4 @@
-{% extends "layout.html" %}
+{% extends "layouts/main.html" %}
 
 {% block pageTitle %}
 Home – {{serviceName}} – GOV.UK Prototype Kit

diff --git a/prototype-starter/app/views/layout.html → ...otype-starter/app/views/layouts/main.html b/prototype-starter/app/views/layout.html → ...otype-starter/app/views/layouts/main.html
diff --git a/server.js b/server.js
@@ -77,9 +77,9 @@ middlewareFunctions.push(sessionUtils.getSessionMiddleware())
 middlewareFunctions.forEach(func => app.use(func))
 
 // Set up App
-var appViews = extensions.getAppViews([
+var appViews = [
   path.join(projectDir, '/app/views/')
-])
+].concat(extensions.getAppViews())
 
 var nunjucksConfig = {
   autoescape: true,