From abc2402b04d6387eca61e2de100f112ba41d14dc Mon Sep 17 00:00:00 2001 From: Andrew Hosgood Date: Thu, 16 Nov 2023 16:39:52 +0000 Subject: [PATCH 1/3] Abstract CookieEventHandler, add more tests (#52) --- .storybook/preview.js | 1 - .../cookie-banner/cookie-banner.stories.js | 1 - src/nationalarchives/lib/cookies.mjs | 101 ++++++---- src/nationalarchives/tests/cookies.test.js | 188 ++++++++++++++---- src/nationalarchives/tests/uuid.test.js | 17 ++ 5 files changed, 226 insertions(+), 82 deletions(-) create mode 100644 src/nationalarchives/tests/uuid.test.js diff --git a/.storybook/preview.js b/.storybook/preview.js index 7d217d41..cf2cb800 100644 --- a/.storybook/preview.js +++ b/.storybook/preview.js @@ -42,7 +42,6 @@ export const decorators = [ }; const cookies = new Cookies(); cookies.deleteAll(); - cookies.destroy(); return Story(); }, ]; diff --git a/src/nationalarchives/components/cookie-banner/cookie-banner.stories.js b/src/nationalarchives/components/cookie-banner/cookie-banner.stories.js index 406110aa..8fd6334a 100644 --- a/src/nationalarchives/components/cookie-banner/cookie-banner.stories.js +++ b/src/nationalarchives/components/cookie-banner/cookie-banner.stories.js @@ -174,7 +174,6 @@ Existing.decorators = [ (Story) => { const cookies = new Cookies(); cookies.set("cookie_preferences_set", true); - cookies.destroy(); return Story(); }, ]; diff --git a/src/nationalarchives/lib/cookies.mjs b/src/nationalarchives/lib/cookies.mjs index 2b3a6821..079975bd 100644 --- a/src/nationalarchives/lib/cookies.mjs +++ b/src/nationalarchives/lib/cookies.mjs @@ -1,3 +1,35 @@ +export class CookieEventHandler { + events = {}; + + constructor() { + if (CookieEventHandler._instance) { + return CookieEventHandler._instance; + } + CookieEventHandler._instance = this; + } + + /** + * Add an event listener. + * @param {string} event - The event to add a listener for. + * @param {function} callback - The callback function to call when the event is triggered. + */ + on(event, callback) { + if (!Object.prototype.hasOwnProperty.call(this.events, event)) { + this.events[event] = []; + } + this.events[event] = [...this.events[event], callback]; + } + + /** @protected */ + trigger(event, data = {}) { + if (Object.prototype.hasOwnProperty.call(this.events, event)) { + this.events[event].forEach((eventToTrigger) => + eventToTrigger.call(this, data), + ); + } + } +} + /** * Class to handle cookies. * @class Cookies @@ -12,8 +44,6 @@ export default class Cookies { /** @protected */ secure = true; /** @protected */ - events = {}; - /** @protected */ policiesKey = ""; /** @@ -29,15 +59,11 @@ export default class Cookies { secure = true, policiesKey = "cookies_policy", } = options; - if (Cookies._instance && Cookies._instance.policiesKey === policiesKey) { - return Cookies._instance; - } - Cookies._instance = this; this.extraPolicies = extraPolicies; this.domain = domain; this.secure = secure; - this.events = {}; this.policiesKey = policiesKey; + this.events = new CookieEventHandler(); this.init(); } @@ -54,15 +80,10 @@ export default class Cookies { }); } - destroy() { - Cookies._instance = null; - this.trigger("destroy"); - } - get all() { const deserialised = {}; document.cookie - .split(";") + .split("; ") .filter((x) => x) .forEach((cookie) => { const parts = cookie.trim().split("="); @@ -74,7 +95,11 @@ export default class Cookies { } get policies() { - return JSON.parse(this.get(this.policiesKey) || "{}"); + try { + return JSON.parse(this.get(this.policiesKey) || "{}"); + } catch (e) { + return {}; + } } /** @@ -128,12 +153,12 @@ export default class Cookies { return; } const cookie = `${encodeURIComponent(key)}=${encodeURIComponent(value)};${ - domain ? ` domain=${domain};` : "" + domain ? ` domain=${domain}; ` : "" } samesite=${sameSite}; path=${path}; max-age=${maxAge}${ secure ? "; secure" : "" }`; document.cookie = cookie; - this.trigger("setCookie", { + this.events.trigger("setCookie", { key, value, maxAge, @@ -153,17 +178,17 @@ export default class Cookies { delete(key, path = "/", domain = null) { const options = { maxAge: -1, path, domain: domain || undefined }; this.set(key, "", options); - this.trigger("deleteCookie", { key, ...options }); + this.events.trigger("deleteCookie", { key, ...options }); } /** * Delete all cookies. */ deleteAll(path = "/", domain = null) { - Object.keys(this.all).forEach((cookie) => - this.delete(cookie, path, domain), - ); - this.trigger("deleteAllCookies", { path, domain }); + Object.keys(this.all).forEach((cookie) => { + this.delete(cookie, path, domain); + }); + this.events.trigger("deleteAllCookies", { path, domain }); } /** @@ -172,8 +197,8 @@ export default class Cookies { */ acceptPolicy(policy) { this.setPolicy(policy, true); - this.trigger("acceptPolicy", policy); - this.trigger("changePolicy", { [policy]: true }); + this.events.trigger("acceptPolicy", policy); + this.events.trigger("changePolicy", { [policy]: true }); } /** @@ -182,8 +207,8 @@ export default class Cookies { */ rejectPolicy(policy) { this.setPolicy(policy, false); - this.trigger("rejectPolicy", policy); - this.trigger("changePolicy", { [policy]: false }); + this.events.trigger("rejectPolicy", policy); + this.events.trigger("changePolicy", { [policy]: false }); } /** @@ -200,7 +225,7 @@ export default class Cookies { [policy]: accepted, essential: true, }); - this.trigger("changePolicy", { [policy]: accepted }); + this.events.trigger("changePolicy", { [policy]: accepted }); } /** @@ -211,8 +236,8 @@ export default class Cookies { Object.keys(this.policies).map((k) => [k.toLowerCase(), true]), ); this.savePolicies(allPolicies); - this.trigger("acceptAllPolicies"); - this.trigger("changePolicy", allPolicies); + this.events.trigger("acceptAllPolicies"); + this.events.trigger("changePolicy", allPolicies); } /** @@ -226,8 +251,8 @@ export default class Cookies { essential: true, }; this.savePolicies(allPolicies); - this.trigger("rejectAllPolicies"); - this.trigger("changePolicy", allPolicies); + this.events.trigger("rejectAllPolicies"); + this.events.trigger("changePolicy", allPolicies); } /** @@ -250,23 +275,11 @@ export default class Cookies { } /** - * Accept a policy. + * Add an event listener. * @param {string} event - The event to add a listener for. * @param {function} callback - The callback function to call when the event is triggered. */ on(event, callback) { - if (!Object.prototype.hasOwnProperty.call(this.events, event)) { - this.events[event] = []; - } - this.events[event] = [...this.events[event], callback]; - } - - /** @protected */ - trigger(event, data = {}) { - if (Object.prototype.hasOwnProperty.call(this.events, event)) { - this.events[event].forEach((eventToTrigger) => - eventToTrigger.call(this, data), - ); - } + this.events.on(event, callback); } } diff --git a/src/nationalarchives/tests/cookies.test.js b/src/nationalarchives/tests/cookies.test.js index ca1e7fe5..361278d7 100644 --- a/src/nationalarchives/tests/cookies.test.js +++ b/src/nationalarchives/tests/cookies.test.js @@ -1,4 +1,11 @@ -import { jest, describe, expect, test, afterEach } from "@jest/globals"; +import { + jest, + describe, + expect, + test, + beforeEach, + afterEach, +} from "@jest/globals"; import { TextEncoder, TextDecoder, store, options } from "util"; import Cookies from "../lib/cookies.mjs"; @@ -12,12 +19,13 @@ const addCookiesToDocument = (document) => { document.__defineGetter__("cookie", () => { return Object.keys(_cookies) .map((key) => `${key}=${_cookies[key]}`) - .join(";"); + .join("; "); }); document.__defineSetter__("cookie", (s) => { - const indexOfSeparator = s.indexOf("="); - const key = s.substr(0, indexOfSeparator); - const value = s.substring(indexOfSeparator + 1); + const keyValue = s.trim().split("="); + const key = keyValue[0].trim(); + const values = keyValue[1].trim().split(";"); + const value = values[0]; _cookies[key] = value; return `${key}=${value}`; }); @@ -30,8 +38,6 @@ addCookiesToDocument(document); describe("No existing cookies", () => { afterEach(() => { - const cookies = new Cookies(); - cookies.destroy(); document.clearAllCookies(); }); @@ -45,30 +51,6 @@ describe("No existing cookies", () => { expect(document.cookie).not.toEqual(""); }); - test("Destruction", async () => { - const cookies = new Cookies(); - - expect(Cookies).toHaveProperty("_instance"); - expect(Cookies._instance).not.toEqual(null); - - cookies.destroy(); - - expect(Cookies).toHaveProperty("_instance"); - expect(Cookies._instance).toEqual(null); - }); - - test("Singleton", async () => { - const cookies1 = new Cookies(); - const cookies2 = new Cookies(); - - expect(cookies1).toBe(cookies2); - - cookies1.destroy(); - const cookies3 = new Cookies(); - - expect(cookies1).not.toBe(cookies3); - }); - test("Getting/setting", async () => { const cookies = new Cookies(); expect(cookies).toHaveProperty("get"); @@ -76,6 +58,8 @@ describe("No existing cookies", () => { expect(cookies).toHaveProperty("exists"); expect(cookies).toHaveProperty("hasValue"); + expect(Object.keys(cookies.all)).toHaveLength(1); + const testKey = "foo"; const testValue = "bar"; @@ -84,6 +68,8 @@ describe("No existing cookies", () => { cookies.set(testKey, testValue); + expect(Object.keys(cookies.all)).toHaveLength(2); + expect(cookies.all).toHaveProperty(testKey); expect(cookies.all[testKey]).toEqual(testValue); expect(cookies.exists(testKey)).toEqual(true); @@ -271,7 +257,7 @@ describe("No existing cookies", () => { expect(cookies.isPolicyAccepted("essential")).toEqual(true); }); - test("Events", async () => { + test("Basic events", async () => { const cookies = new Cookies(); expect(cookies).toHaveProperty("on"); @@ -291,21 +277,151 @@ describe("No existing cookies", () => { sameSite: "Lax", secure: true, maxAge: 31536000, - cookie: `foo=bar; samesite=Lax; path=/; max-age=31536000; secure`, + cookie: `${testKey}=${testValue}; samesite=Lax; path=/; max-age=31536000; secure`, }); - cookies.set(testKey, ""); + cookies.set(testKey, testValue); expect(mockCallback.mock.calls).toHaveLength(2); expect(mockCallback.mock.calls[1][0]).toStrictEqual({ key: testKey, - value: "", + value: testValue, domain: "", path: "/", sameSite: "Lax", secure: true, maxAge: 31536000, - cookie: `foo=; samesite=Lax; path=/; max-age=31536000; secure`, + cookie: `${testKey}=${testValue}; samesite=Lax; path=/; max-age=31536000; secure`, }); }); + + test("All events", async () => { + const cookies = new Cookies(); + + const mockSetCookieCallback = jest.fn(); + cookies.on("setCookie", mockSetCookieCallback); + const mockDeleteCookieCallback = jest.fn(); + cookies.on("deleteCookie", mockDeleteCookieCallback); + const mockDeleteAllCookiesCallback = jest.fn(); + cookies.on("deleteAllCookies", mockDeleteAllCookiesCallback); + const mockAcceptPolicyCallback = jest.fn(); + cookies.on("acceptPolicy", mockAcceptPolicyCallback); + const mockRejectPolicyCallback = jest.fn(); + cookies.on("rejectPolicy", mockRejectPolicyCallback); + const mockAcceptAllPoliciesCallback = jest.fn(); + cookies.on("acceptAllPolicies", mockAcceptAllPoliciesCallback); + const mockRejectAllPoliciesCallback = jest.fn(); + cookies.on("rejectAllPolicies", mockRejectAllPoliciesCallback); + const mockChangePolicyCallback = jest.fn(); + cookies.on("changePolicy", mockChangePolicyCallback); + + const testKey = "foo"; + const testValue = "bar"; + cookies.set(testKey, testValue); + cookies.delete(testKey); + cookies.acceptPolicy("settings"); + cookies.rejectPolicy("settings"); + cookies.setPolicy("settings", true); + cookies.acceptAllPolicies(); + cookies.rejectAllPolicies(); + cookies.deleteAll(); + + expect(mockSetCookieCallback.mock.calls).toHaveLength(9); + expect(mockDeleteCookieCallback.mock.calls).toHaveLength(3); + expect(mockDeleteAllCookiesCallback.mock.calls).toHaveLength(1); + expect(mockAcceptPolicyCallback.mock.calls).toHaveLength(1); + expect(mockRejectPolicyCallback.mock.calls).toHaveLength(1); + expect(mockAcceptAllPoliciesCallback.mock.calls).toHaveLength(1); + expect(mockRejectAllPoliciesCallback.mock.calls).toHaveLength(1); + expect(mockChangePolicyCallback.mock.calls).toHaveLength(7); + }); + + test("Shared events", async () => { + const mockCallback = jest.fn(); + + const cookies1 = new Cookies(); + + const cookies2 = new Cookies(); + cookies2.on("setCookie", mockCallback); + + const testKey = "foo"; + const testValue = "bar"; + + cookies1.set(testKey, testValue); + expect(mockCallback.mock.calls).toHaveLength(1); + + cookies1.set(testKey, testValue); + expect(mockCallback.mock.calls).toHaveLength(2); + + cookies2.set(testKey, testValue); + expect(mockCallback.mock.calls).toHaveLength(3); + }); +}); + +describe("Existing cookies", () => { + beforeEach(() => { + document.clearAllCookies(); + document.cookie = + "cookies_policy=%7B%22usage%22%3Afalse%2C%22settings%22%3Atrue%2C%22essential%22%3Atrue%7D"; + }); + + test("Initialisation", async () => { + const cookies = new Cookies(); + + expect(cookies.all).toHaveProperty("cookies_policy"); + expect(cookies.policies).toHaveProperty("essential"); + expect(cookies.isPolicyAccepted("essential")).toEqual(true); + expect(cookies.policies).toHaveProperty("settings"); + expect(cookies.isPolicyAccepted("settings")).toEqual(true); + expect(cookies.policies).toHaveProperty("usage"); + expect(cookies.isPolicyAccepted("usage")).toEqual(false); + }); + + test("Update policies", async () => { + const cookies = new Cookies(); + cookies.acceptPolicy("usage"); + cookies.rejectPolicy("settings"); + + expect(cookies.isPolicyAccepted("essential")).toEqual(true); + expect(cookies.isPolicyAccepted("settings")).toEqual(false); + expect(cookies.isPolicyAccepted("usage")).toEqual(true); + }); +}); + +describe("Existing empty cookie policies", () => { + beforeEach(() => { + document.clearAllCookies(); + document.cookie = "cookies_policy=%7B%7D"; + }); + + test("Initialisation", async () => { + const cookies = new Cookies(); + + expect(cookies.all).toHaveProperty("cookies_policy"); + expect(cookies.policies).toHaveProperty("essential"); + expect(cookies.isPolicyAccepted("essential")).toEqual(true); + expect(cookies.policies).toHaveProperty("settings"); + expect(cookies.isPolicyAccepted("settings")).toEqual(false); + expect(cookies.policies).toHaveProperty("usage"); + expect(cookies.isPolicyAccepted("usage")).toEqual(false); + }); +}); + +describe("Existing malformed cookie policies", () => { + beforeEach(() => { + document.clearAllCookies(); + document.cookie = "cookies_policy=foobar"; + }); + + test("Initialisation", async () => { + const cookies = new Cookies(); + + expect(cookies.all).toHaveProperty("cookies_policy"); + expect(cookies.policies).toHaveProperty("essential"); + expect(cookies.isPolicyAccepted("essential")).toEqual(true); + expect(cookies.policies).toHaveProperty("settings"); + expect(cookies.isPolicyAccepted("settings")).toEqual(false); + expect(cookies.policies).toHaveProperty("usage"); + expect(cookies.isPolicyAccepted("usage")).toEqual(false); + }); }); diff --git a/src/nationalarchives/tests/uuid.test.js b/src/nationalarchives/tests/uuid.test.js new file mode 100644 index 00000000..70bd3e5b --- /dev/null +++ b/src/nationalarchives/tests/uuid.test.js @@ -0,0 +1,17 @@ +import { describe, expect, test } from "@jest/globals"; +import { TextEncoder, TextDecoder, store, options } from "util"; +import uuidv4 from "../lib/uuid.mjs"; + +global.TextEncoder = TextEncoder; +global.TextDecoder = TextDecoder; +global.store = store; +global.options = options; + +describe("UUID", () => { + test("Initialisation", async () => { + const uuid1 = uuidv4(); + const uuid2 = uuidv4(); + + expect(uuid1).not.toEqual(uuid2); + }); +}); From 2879e7b2683a54e38ff2088e2397b2eef529b3bc Mon Sep 17 00:00:00 2001 From: Andrew Hosgood Date: Thu, 16 Nov 2023 16:43:17 +0000 Subject: [PATCH 2/3] v0.1.25-prerelease --- CHANGELOG.md | 14 ++++++++++---- package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 06bd205f..5618a2b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,16 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased](https://github.com/nationalarchives/tna-frontend/compare/v0.1.24-prerelease...HEAD) +## [Unreleased](https://github.com/nationalarchives/tna-frontend/compare/v0.1.25-prerelease...HEAD) + +### Added +### Changed +### Deprecated +### Removed +### Fixed +### Security + +## [0.1.25-prerelease](https://github.com/nationalarchives/tna-frontend/compare/v0.1.24-prerelease...v0.1.25-prerelease) - 2023-11-16 ### Added @@ -24,7 +33,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The standard cookie policies are always `essential`, `usage` and `settings` - other custom policies can be added - Focus outline on dark themes has changed from blue to orange to avoid colour conflict with links -### Deprecated ### Removed - Removed CSS to counter conflicting GOV.UK paragraph styling @@ -40,8 +48,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Accepting or declining individual cookie policies now works - Removed extra space from nested links -### Security - ## [0.1.24-prerelease](https://github.com/nationalarchives/tna-frontend/compare/v0.1.23-prerelease...v0.1.24-prerelease) - 2023-11-06 ### Added diff --git a/package-lock.json b/package-lock.json index 8d595b9e..fbe73645 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@nationalarchives/frontend", - "version": "0.1.24-prerelease", + "version": "0.1.25-prerelease", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@nationalarchives/frontend", - "version": "0.1.24-prerelease", + "version": "0.1.25-prerelease", "license": "MIT", "devDependencies": { "@babel/core": "^7.23.2", diff --git a/package.json b/package.json index abff8278..a8067634 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@nationalarchives/frontend", - "version": "0.1.24-prerelease", + "version": "0.1.25-prerelease", "description": "The National Archives frontend styles", "scripts": { "start": "storybook dev -p 6006", From 61310d9bd8c0ccff711d3407cb172467b5dbe9c1 Mon Sep 17 00:00:00 2001 From: Andrew Hosgood Date: Thu, 16 Nov 2023 16:52:21 +0000 Subject: [PATCH 3/3] Fix release workflow --- .github/workflows/npm-publish.yml | 9 +++------ .gitignore | 3 ++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.github/workflows/npm-publish.yml b/.github/workflows/npm-publish.yml index 0cdacec0..1eb9dc94 100644 --- a/.github/workflows/npm-publish.yml +++ b/.github/workflows/npm-publish.yml @@ -1,4 +1,4 @@ -name: Publish new release +name: Publish release on: release: @@ -99,14 +99,11 @@ jobs: - uses: actions/checkout@v3 - name: Get release notes id: get-release-notes - run: | - RELEASE_NOTES=$(./tasks/get-release-notes.sh "${{ needs.check.outputs.version }}") - echo $RELEASE_NOTES - echo "RELEASE_NOTES=$RELEASE_NOTES" >> "$GITHUB_OUTPUT" + run: ./tasks/get-release-notes.sh "${{ needs.check.outputs.version }}" > RELEASE_NOTES.txt - uses: rtCamp/action-slack-notify@v2 env: SLACK_TITLE: "`v${{ needs.check.outputs.version }}` of `tna-frontend` has just been published" SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }} SLACK_ICON: "https://raw.githubusercontent.com/nationalarchives/tna-frontend/main/src/nationalarchives/assets/images/apple-touch-icon.png" MSG_MINIMAL: true - SLACK_MESSAGE: ${{ env.RELEASE_NOTES }} + SLACK_MESSAGE: $(cat RELEASE_NOTES.txt) diff --git a/.gitignore b/.gitignore index bc409558..b4688170 100644 --- a/.gitignore +++ b/.gitignore @@ -8,4 +8,5 @@ storybook *.tgz build-storybook.log chromatic.config.json -chromatic.log \ No newline at end of file +chromatic.log +RELEASE_NOTES.txt \ No newline at end of file