diff --git a/cypress/e2e/plugins/2-prototype-kit-plugin-tests/handle-plugin-update-when-a-dependency-is-now-required.cypress.js b/cypress/e2e/plugins/2-prototype-kit-plugin-tests/handle-plugin-update-when-a-dependency-is-now-required.cypress.js new file mode 100644 index 0000000000..58b62147f6 --- /dev/null +++ b/cypress/e2e/plugins/2-prototype-kit-plugin-tests/handle-plugin-update-when-a-dependency-is-now-required.cypress.js @@ -0,0 +1,48 @@ +import { installPlugin, uninstallPlugin, waitForApplication } from '../../utils' + +const plugin = '@govuk-prototype-kit/common-templates' +const pluginVersion = '1.1.1' +const pluginsPage = '/manage-prototype/plugins' + +const dependencyPlugin = 'govuk-frontend' +const dependencyPluginName = 'GOV.UK Frontend' + +describe('Handle a plugin update', () => { + it('when a dependency is now required', () => { + installPlugin(plugin, pluginVersion) + uninstallPlugin(dependencyPlugin) + + waitForApplication(pluginsPage) + + cy.get('[data-plugin-group-status="available"]') + .find(`[data-plugin-package-name="${dependencyPlugin}"]`) + .find('button') + .contains('Install') + + cy.get('[data-plugin-group-status="installed"]') + .find(`[data-plugin-package-name="${plugin}"]`) + .find('button') + .contains('Update') + .click() + + cy.get('#plugin-action-confirmation') + .find('ul') + .contains(dependencyPluginName) + + cy.get('#plugin-action-button').click() + + cy.get('#panel-complete', { timeout: 20000 }) + .should('be.visible') + .contains('Update complete') + + cy.get('#instructions-complete a') + .contains('Back to plugins') + .click() + + cy.get('[data-plugin-group-status="installed"]') + .find(`[data-plugin-package-name="${dependencyPlugin}"]`) + + cy.get('[data-plugin-group-status="installed"]') + .find(`[data-plugin-package-name="${plugin}"]`) + }) +}) diff --git a/lib/manage-prototype-handlers.js b/lib/manage-prototype-handlers.js index 10416f0406..03bab87ffb 100644 --- a/lib/manage-prototype-handlers.js +++ b/lib/manage-prototype-handlers.js @@ -444,7 +444,10 @@ function getCommand (mode, chosenPlugin) { dependentPlugins } = chosenPlugin const dependents = dependentPlugins?.map(({ packageName }) => packageName).join(' ') - const dependencies = dependencyPlugins?.map(({ packageName }) => packageName).join(' ') + const dependencies = dependencyPlugins?.map(({ + packageName, + latestVersion + }) => packageName + '@' + latestVersion).join(' ') if (version && installCommand) { installCommand += `@${version}` @@ -474,19 +477,22 @@ const verbs = { title: 'Update', para: 'update', status: 'updated', - progressive: 'updating' + progressive: 'updating', + dependencyPara: 'install' }, install: { title: 'Install', para: 'install', status: 'installed', - progressive: 'installing' + progressive: 'installing', + dependencyPara: 'install' }, uninstall: { title: 'Uninstall', para: 'uninstall', status: 'uninstalled', - progressive: 'uninstalling' + progressive: 'uninstalling', + dependencyPara: 'uninstall' } } @@ -533,21 +539,15 @@ async function getPluginForRequest (req) { } } - const dependentPlugins = mode === 'uninstall' - ? (await getDependentPackages(chosenPlugin.packageName)) - .filter(({ installed }) => installed) - .map(buildPluginData) - : [] + const dependentPlugins = (await getDependentPackages(chosenPlugin.packageName, version, mode)) + .filter(({ installed }) => installed || mode !== 'uninstall') + .map(buildPluginData) if (dependentPlugins.length) { chosenPlugin.dependentPlugins = dependentPlugins } - const dependencyPlugins = mode !== 'uninstall' - ? (await getDependencyPackages(chosenPlugin.packageName, version)) - .filter(({ installed }) => !installed) - .map(buildPluginData) - : [] + const dependencyPlugins = (await getDependencyPackages(chosenPlugin.packageName, version, mode)).map(buildPluginData) if (dependencyPlugins.length) { chosenPlugin.dependencyPlugins = dependencyPlugins diff --git a/lib/nunjucks/views/manage-prototype/plugin-install-or-uninstall.njk b/lib/nunjucks/views/manage-prototype/plugin-install-or-uninstall.njk index a01a082f3b..866ddbe1cb 100644 --- a/lib/nunjucks/views/manage-prototype/plugin-install-or-uninstall.njk +++ b/lib/nunjucks/views/manage-prototype/plugin-install-or-uninstall.njk @@ -47,7 +47,7 @@

- To {{ verb.para }} {{ chosenPlugin.name }} you also need to {{ verb.para }} {% if chosenPlugin.dependentPlugins | length > 1 %}these plugins{% else %}this plugin{% endif %}. + To {{ verb.para }} {{ chosenPlugin.name }} you also need to {{ verb.dependencyPara }} {% if chosenPlugin.dependentPlugins | length > 1 %}these plugins{% else %}this plugin{% endif %}.

{{ govukButton({ diff --git a/lib/plugins/packages.js b/lib/plugins/packages.js index e52959509f..a24e735f36 100644 --- a/lib/plugins/packages.js +++ b/lib/plugins/packages.js @@ -118,7 +118,7 @@ async function refreshPackageInfo (packageName, version) { localVersion = path.resolve(installedPackageVersion.replace('file:', '')) } - const pluginDependencies = pluginConfig?.pluginDependencies + const pluginDependencies = pluginConfig?.pluginDependencies ? normaliseDependencies(pluginConfig.pluginDependencies) : undefined const packageInfo = { packageName, @@ -206,7 +206,21 @@ async function waitForPackagesCache () { } } -async function getDependentPackages (packageName) { +function normaliseDependencies (dependencies) { + return dependencies.map((dependency) => { + if (typeof dependency === 'string') { + dependency = { + packageName: dependency + } + } + return dependency + }) +} + +async function getDependentPackages (packageName, version, mode) { + if (mode !== 'uninstall') { + return [] + } if (!Object.keys(packagesCache).length) { await startPackageTracker() } @@ -215,17 +229,29 @@ async function getDependentPackages (packageName) { .filter(({ pluginDependencies }) => pluginDependencies?.some((pluginDependency) => pluginDependency === packageName || pluginDependency.packageName === packageName)) } -async function getDependencyPackages (packageName, version) { +async function getDependencyPackages (packageName, version, mode) { if (!Object.keys(packagesCache).length) { await startPackageTracker() } await waitForPackagesCache() const pkg = await lookupPackageInfo(packageName, version) - return !pkg?.pluginDependencies + let pluginDependencies = pkg?.pluginDependencies + if (version || mode === 'update') { + const targetVersion = version || pkg.latestVersion + if (targetVersion !== pkg.installedVersion) { + const latestPluginConfig = await getConfigForPackage(pkg.packageName, version) + if (latestPluginConfig) { + pluginDependencies = latestPluginConfig.pluginDependencies + } + } + } + const dependencyPlugins = !pluginDependencies ? [] - : await Promise.all(pkg.pluginDependencies.map((pluginDependency) => { - return typeof pluginDependency === 'string' ? lookupPackageInfo(pluginDependency) : lookupPackageInfo(pluginDependency.packageName) + : await Promise.all(normaliseDependencies(pluginDependencies).map((pluginDependency) => { + return lookupPackageInfo(pluginDependency.packageName) })) + + return dependencyPlugins.filter(({ installed }) => !installed) } if (!config.getConfig().isTest) { diff --git a/lib/plugins/packages.spec.js b/lib/plugins/packages.spec.js index 468db75538..d69179929b 100644 --- a/lib/plugins/packages.spec.js +++ b/lib/plugins/packages.spec.js @@ -43,23 +43,29 @@ describe('packages', () => { const availableInstalledPackage = { packageName: 'available-installed-package', installed: true, - available: true + available: true, + installedVersion: '1.0.0', + latestVersion: '1.0.0' } const availableUninstalledPackage = { packageName: 'available-uninstalled-package', installed: false, - available: true + available: true, + latestVersion: '1.0.0' } const unavailableInstalledPackage = { packageName: 'unavailable-installed-package', installed: true, available: false, + installedVersion: '1.0.0', + latestVersion: '1.0.0', pluginDependencies: [availableInstalledPackage] } const unavailableUninstalledPackage = { packageName: 'unavailable-uninstalled-package', installed: false, available: false, + latestVersion: '1.0.0', pluginDependencies: [availableUninstalledPackage] } @@ -86,10 +92,15 @@ describe('packages', () => { }) describe('getDependentPackages', () => { - it('', async () => { - const dependentPackages = await getDependentPackages(availableInstalledPackage.packageName) + it('when mode is uninstall', async () => { + const dependentPackages = await getDependentPackages(availableInstalledPackage.packageName, undefined, 'uninstall') expect(dependentPackages).toEqual([unavailableInstalledPackage]) }) + + it('when mode is update', async () => { + const dependentPackages = await getDependentPackages(availableInstalledPackage.packageName, '1.1.1', 'update') + expect(dependentPackages).toEqual([]) + }) }) describe('getDependencyPackages', () => { diff --git a/lib/utils/requestHttps.js b/lib/utils/requestHttps.js index 16720fd2f9..3afe27ad17 100644 --- a/lib/utils/requestHttps.js +++ b/lib/utils/requestHttps.js @@ -7,7 +7,7 @@ const { tmpDir } = require('./paths') const { exists, readJson, ensureDir, writeJson } = require('fs-extra') const { verboseLog } = require('./verboseLogger') -async function getConfigForPackage (packageName) { +async function getConfigForPackage (packageName, version) { const timer = startPerformanceTimer() const cacheFileDirectory = path.join(tmpDir, 'caches') @@ -32,19 +32,19 @@ async function getConfigForPackage (packageName) { return undefined } - const latestTag = registry['dist-tags']?.latest + const targetVersion = version || registry['dist-tags']?.latest - if (cache[packageName] && cache[packageName][latestTag]) { + if (cache[packageName] && cache[packageName][targetVersion]) { endPerformanceTimer('getConfigForPackage (from cache)', timer) - return cache[packageName][latestTag] + return cache[packageName][targetVersion] } - if (!latestTag) { - endPerformanceTimer('getConfigForPackage (no latest tag)', timer) + if (!targetVersion || !registry.versions[targetVersion]) { + endPerformanceTimer(`getConfigForPackage (no ${targetVersion ? 'version ' + targetVersion : 'latest tag'})`, timer) return } - const url = registry.versions[latestTag].dist.tarball + const url = registry.versions[targetVersion].dist.tarball try { const result = await findFileInHttpsTgz(url, { fileToFind: 'package/govuk-prototype-kit.config.json', @@ -52,7 +52,7 @@ async function getConfigForPackage (packageName) { if (str && str.startsWith('{')) { const result = JSON.parse(str) cache[packageName] = cache[packageName] || {} - cache[packageName][latestTag] = result + cache[packageName][targetVersion] = result writeJson(cacheFileReference, cache) return result }