From 614fb9700e3db8e6fc53c386e98682892c69b4b9 Mon Sep 17 00:00:00 2001 From: Ben Surgison Date: Fri, 6 Oct 2023 17:13:34 +0100 Subject: [PATCH 1/2] Only allow plugin update functionality when installed from npm --- CHANGELOG.md | 1 + .../install-available-plugin.cypress.js | 7 +++++++ lib/manage-prototype-handlers.js | 5 +++-- lib/nunjucks/views/manage-prototype/plugins.njk | 1 + lib/plugins/packages.js | 4 ++++ lib/plugins/packages.spec.js | 4 ++++ 6 files changed, 20 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f0c7890e9..7883511ab3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - [#2355: Prevent management pages using "plugin" GOV.UK Frontend views](https://github.com/alphagov/govuk-prototype-kit/pull/2355) +- [#2356: Only allow plugin update functionality when installed from npm](https://github.com/alphagov/govuk-prototype-kit/pull/2356) - [#2358: Suppress Sass warnings for `$legacy` deprecated colour palette](https://github.com/alphagov/govuk-prototype-kit/pull/2358) ## 13.13.4 diff --git a/cypress/e2e/plugins/1-available-plugins-tests/install-available-plugin.cypress.js b/cypress/e2e/plugins/1-available-plugins-tests/install-available-plugin.cypress.js index 526e518610..e4d4b6987b 100644 --- a/cypress/e2e/plugins/1-available-plugins-tests/install-available-plugin.cypress.js +++ b/cypress/e2e/plugins/1-available-plugins-tests/install-available-plugin.cypress.js @@ -80,18 +80,23 @@ describe('Management plugins: ', () => { loadPluginsPage() + cy.get('#plugins-updates-available-message').should('not.exist') + cy.visit(`${managePluginsPagePath}/install?package=${encodeURIComponent(plugin)}&version=${version1}`) cy.get('#plugin-action-button').click() performPluginAction('install', plugin, pluginName) + cy.get('#plugins-updates-available-message').contains('1 UPDATE AVAILABLE') + // ------------------------ log(`Update the ${plugin}@${version1} plugin to ${plugin}@${version2}`) installPlugin(plugin, version1) loadInstalledPluginsPage() + log(`Update the ${plugin} plugin`) cy.get(`[data-plugin-package-name="${plugin}"]`) @@ -101,6 +106,8 @@ describe('Management plugins: ', () => { .click() performPluginAction('update', plugin, pluginName) + + cy.get('#plugins-updates-available-message').should('not.exist') }) it(`Create a page using a template from the ${plugin} plugin`, () => { diff --git a/lib/manage-prototype-handlers.js b/lib/manage-prototype-handlers.js index 0e8286c74e..7adedd3719 100644 --- a/lib/manage-prototype-handlers.js +++ b/lib/manage-prototype-handlers.js @@ -412,6 +412,7 @@ function buildPluginData (pluginData) { packageName, installed, installedLocally, + updateAvailable, latestVersion, installedVersion, required, @@ -427,7 +428,7 @@ function buildPluginData (pluginData) { installedLocally, installLink: `${contextPath}/plugins/install?package=${encodeURIComponent(packageName)}`, installCommand: `npm install ${packageName}`, - updateLink: installed && !installedLocally && latestVersion !== installedVersion ? `${contextPath}/plugins/update?package=${encodeURIComponent(packageName)}` : undefined, + updateLink: updateAvailable ? `${contextPath}/plugins/update?package=${encodeURIComponent(packageName)}` : undefined, updateCommand: latestVersion && `npm install ${packageName}@${latestVersion}`, uninstallLink: installed && !required ? `${contextPath}/plugins/uninstall?package=${encodeURIComponent(packageName)}${installedLocally ? `&version=${encodeURIComponent(localVersion)}` : ''}` : undefined, uninstallCommand: `npm uninstall ${packageName}`, @@ -450,7 +451,7 @@ async function prepareForPluginPage (isInstalledPage, search) { status: isInstalledPage ? 'installed' : 'search', plugins: plugins.map(buildPluginData), found: plugins.length, - updates: installedPlugins.filter(plugin => plugin.installedVersion !== plugin.latestVersion).length + updates: installedPlugins.filter(plugin => plugin.updateAvailable).length } } diff --git a/lib/nunjucks/views/manage-prototype/plugins.njk b/lib/nunjucks/views/manage-prototype/plugins.njk index 1cb2129512..afc0f36d9e 100644 --- a/lib/nunjucks/views/manage-prototype/plugins.njk +++ b/lib/nunjucks/views/manage-prototype/plugins.njk @@ -38,6 +38,7 @@ {% endif %} {% if updatesMessage %} {{ govukTag({ + attributes: {id: "plugins-updates-available-message"}, text: updatesMessage }) }} {% endif %} diff --git a/lib/plugins/packages.js b/lib/plugins/packages.js index 17ec890edf..622cff7c74 100644 --- a/lib/plugins/packages.js +++ b/lib/plugins/packages.js @@ -87,6 +87,7 @@ async function refreshPackageInfo (packageName, version) { const installedPackageVersion = packageJson && projectPackage.dependencies[packageName] const installed = !!installedPackageVersion const installedLocally = installedPackageVersion?.startsWith('file:') + const installedFromGithub = installedPackageVersion?.startsWith('github') const installedVersion = installed ? packageJson?.version : undefined let localVersion @@ -118,6 +119,8 @@ async function refreshPackageInfo (packageName, version) { localVersion = path.resolve(installedPackageVersion.replace('file:', '')) } + const updateAvailable = installed && !installedLocally && !installedFromGithub && installedVersion !== latestVersion + const pluginDependencies = pluginConfig?.pluginDependencies ? normaliseDependencies(pluginConfig.pluginDependencies) : undefined const packageInfo = { @@ -133,6 +136,7 @@ async function refreshPackageInfo (packageName, version) { pluginConfig, pluginDependencies, localVersion, + updateAvailable, installedPackageVersion } diff --git a/lib/plugins/packages.spec.js b/lib/plugins/packages.spec.js index 3fbfbdb218..87ab98a9a9 100644 --- a/lib/plugins/packages.spec.js +++ b/lib/plugins/packages.spec.js @@ -184,6 +184,7 @@ describe('packages', () => { installedPackageVersion: '1.0.0', installedVersion: '1.0.0', latestVersion: '1.0.1', + updateAvailable: true, required: false, packageJson: { local: true, @@ -211,6 +212,7 @@ describe('packages', () => { installed: false, latestVersion: '1.0.0', required: false, + updateAvailable: false, packageJson: { version: '1.0.0' }, @@ -232,6 +234,7 @@ describe('packages', () => { installed: false, latestVersion: '2.0.0', required: false, + updateAvailable: false, packageJson: { version: '2.0.0' }, @@ -266,6 +269,7 @@ describe('packages', () => { available: false, installed: false, localVersion: version, + updateAvailable: false, packageJson: { local: true, version: '1.0.0' From 4fe28057458d31c2c601b3b471040fe54b50577a Mon Sep 17 00:00:00 2001 From: Ben Surgison Date: Mon, 9 Oct 2023 16:12:39 +0100 Subject: [PATCH 2/2] Make github consistent with file --- lib/plugins/packages.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/packages.js b/lib/plugins/packages.js index 622cff7c74..95b80327b8 100644 --- a/lib/plugins/packages.js +++ b/lib/plugins/packages.js @@ -87,7 +87,7 @@ async function refreshPackageInfo (packageName, version) { const installedPackageVersion = packageJson && projectPackage.dependencies[packageName] const installed = !!installedPackageVersion const installedLocally = installedPackageVersion?.startsWith('file:') - const installedFromGithub = installedPackageVersion?.startsWith('github') + const installedFromGithub = installedPackageVersion?.startsWith('github:') const installedVersion = installed ? packageJson?.version : undefined let localVersion