From 1fbda8faf274f317023037a4ee1346c85e480a98 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 27 Feb 2024 13:37:56 +0000 Subject: [PATCH 1/4] add local stop only hook --- .pre-commit-config.yaml | 8 +++ package.json | 3 +- yarn.lock | 114 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 115 insertions(+), 10 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c7592194e8..49a5551c06 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -53,6 +53,14 @@ repos: rev: 37.150.1 hooks: - id: renovate-config-validator + - repo: local + hooks: + - id: stop-only + name: stop-only + entry: yarn run stop-only + language: node + args: [--folder, cypress] + # - repo: https://github.com/Yelp/detect-secrets # rev: v1.4.0 # hooks: diff --git a/package.json b/package.json index 957518dc47..d0eeb75d03 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "cypress-file-upload": "^5.0.8", "cypress-multi-reporters": "^1.6.1", "cypress-parallel": "^0.14.0", - "cypress-real-events": "^1.8.1" + "cypress-real-events": "^1.8.1", + "stop-only": "^3.3.1" } } diff --git a/yarn.lock b/yarn.lock index c2444595ae..e0fcf1d7cc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -766,6 +766,17 @@ core-util-is@1.0.2: resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.2.tgz#b5fd54220aa2bc5ab57aab7140c940754503c1a7" integrity sha512-3lqz5YjWTYnW6dlDa5TLaTCcShfar1e40rmcJVwCBJC6mWlFuj0eCHIElmG1g5kyuJ/GD+8Wn4FFCcz4gJPfaQ== +cross-spawn@^6.0.0: + version "6.0.5" + resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4" + integrity sha512-eTVLrBSt7fjbDygz805pMnstIs2VTBNkRm0qxZd+M7A5XDdxVRWO5MxGBXZhjY4cqLYLdtrGqRf8mBPmzwSpWQ== + dependencies: + nice-try "^1.0.4" + path-key "^2.0.1" + semver "^5.5.0" + shebang-command "^1.2.0" + which "^1.2.9" + cross-spawn@^7.0.0, cross-spawn@^7.0.3: version "7.0.3" resolved "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz" @@ -880,6 +891,13 @@ debug@4.3.3: dependencies: ms "2.1.2" +debug@4.3.4, debug@^4.1.1, debug@^4.3.4: + version "4.3.4" + resolved "https://registry.yarnpkg.com/debug/-/debug-4.3.4.tgz#1319f6579357f2338d3337d2cdd4914bb5dcc865" + integrity sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ== + dependencies: + ms "2.1.2" + debug@^3.1.0: version "3.2.7" resolved "https://registry.yarnpkg.com/debug/-/debug-3.2.7.tgz#72580b7e9145fb39b6676f9c5e5fb100b934179a" @@ -887,13 +905,6 @@ debug@^3.1.0: dependencies: ms "^2.1.1" -debug@^4.1.1, debug@^4.3.4: - version "4.3.4" - resolved "https://registry.yarnpkg.com/debug/-/debug-4.3.4.tgz#1319f6579357f2338d3337d2cdd4914bb5dcc865" - integrity sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ== - dependencies: - ms "2.1.2" - decamelize@^1.2.0: version "1.2.0" resolved "https://registry.npmjs.org/decamelize/-/decamelize-1.2.0.tgz" @@ -1012,6 +1023,19 @@ eventemitter2@6.4.7: resolved "https://registry.yarnpkg.com/eventemitter2/-/eventemitter2-6.4.7.tgz#a7f6c4d7abf28a14c1ef3442f21cb306a054271d" integrity sha512-tYUSVOGeQPKt/eC1ABfhHy5Xd96N3oIijJvN3O9+TsC28T5V9yX9oEfEK5faP0EFSNVOG97qtAS68GBrQB2hDg== +execa@0.11.0: + version "0.11.0" + resolved "https://registry.yarnpkg.com/execa/-/execa-0.11.0.tgz#0b3c71daf9b9159c252a863cd981af1b4410d97a" + integrity sha512-k5AR22vCt1DcfeiRixW46U5tMLtBg44ssdJM9PiXw3D8Bn5qyxFCSnKY/eR22y+ctFDGPqafpaXg2G4Emyua4A== + dependencies: + cross-spawn "^6.0.0" + get-stream "^4.0.0" + is-stream "^1.1.0" + npm-run-path "^2.0.0" + p-finally "^1.0.0" + signal-exit "^3.0.0" + strip-eof "^1.0.0" + execa@4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/execa/-/execa-4.1.0.tgz#4e5491ad1572f2f17a77d388c6c857135b22847a" @@ -1166,6 +1190,13 @@ get-intrinsic@^1.1.3, get-intrinsic@^1.2.3, get-intrinsic@^1.2.4: has-symbols "^1.0.3" hasown "^2.0.0" +get-stream@^4.0.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/get-stream/-/get-stream-4.1.0.tgz#c1b255575f3dc21d59bfc79cd3d2b46b1c3a54b5" + integrity sha512-GMat4EJ5161kIy2HevLlr4luNjBgvmj413KaQA7jt4V8B4RDsfpHk7WQ9GVqfYyyx8OS/L66Kox+rJRNklLK7w== + dependencies: + pump "^3.0.0" + get-stream@^5.0.0, get-stream@^5.1.0: version "5.2.0" resolved "https://registry.yarnpkg.com/get-stream/-/get-stream-5.2.0.tgz#4966a1795ee5ace65e706c4b7beb71257d6e22d3" @@ -1397,6 +1428,11 @@ is-plain-obj@^2.1.0: resolved "https://registry.npmjs.org/is-plain-obj/-/is-plain-obj-2.1.0.tgz" integrity sha512-YWnfyRwxL/+SsrWYfOpUtz5b3YD+nyfkHvjbcanzk8zgyO4ASD67uVMRt8k5bM4lLMDnXfriRhOpemw+NfT1eA== +is-stream@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/is-stream/-/is-stream-1.1.0.tgz#12d4a3dd4e68e0b79ceb8dbc84173ae80d91ca44" + integrity sha512-uQPm8kcs47jx38atAcWTVxyltQYoPT68y9aWYdV6yWXSyW8mzSat0TL6CiWdZeCdF3KrAvpVtnHbTv4RN+rqdQ== + is-stream@^2.0.0: version "2.0.1" resolved "https://registry.yarnpkg.com/is-stream/-/is-stream-2.0.1.tgz#fac1e3d53b97ad5a9d0ae9cef2389f5810a5c077" @@ -1577,7 +1613,7 @@ minimatch@^3.0.4, minimatch@^3.1.1: dependencies: brace-expansion "^1.1.7" -minimist@^1.2.8: +minimist@1.2.8, minimist@^1.2.8: version "1.2.8" resolved "https://registry.yarnpkg.com/minimist/-/minimist-1.2.8.tgz#c1a464e7693302e082a075cee0c057741ac4772c" integrity sha512-2yyAR8qBkN3YuheJanUpWC5U3bb5osDywNB8RzDVlDwDHbocAJveqqj1u8+SVD7jkWT4yvsHCpWqqWqAxb0zCA== @@ -1632,11 +1668,23 @@ nanoid@3.3.1: resolved "https://registry.npmjs.org/nanoid/-/nanoid-3.3.1.tgz" integrity sha512-n6Vs/3KGyxPQd6uO0eH4Bv0ojGSUvuLlIHtC3Y0kEO23YRge8H9x1GCzLn28YX0H66pMkxuaeESFq4tKISKwdw== +nice-try@^1.0.4: + version "1.0.5" + resolved "https://registry.yarnpkg.com/nice-try/-/nice-try-1.0.5.tgz#a3378a7696ce7d223e88fc9b764bd7ef1089e366" + integrity sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ== + normalize-path@^3.0.0, normalize-path@~3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/normalize-path/-/normalize-path-3.0.0.tgz#0dcd69ff23a1c9b11fd0978316644a0388216a65" integrity sha512-6eZs5Ls3WtCisHWp9S2GUy8dqkpGi4BVSz3GaqiE6ezub0512ESztXUwUB6C6IKbQkY2Pnb/mD4WYojCRwcwLA== +npm-run-path@^2.0.0: + version "2.0.2" + resolved "https://registry.yarnpkg.com/npm-run-path/-/npm-run-path-2.0.2.tgz#35a9232dfa35d7067b4cb2ddf2357b1871536c5f" + integrity sha512-lJxZYlT4DW/bRUtFh1MQIWqmLwQfAxnqWG4HhEdjMlkrJYnJn0Jrr2u3mgxqaWsdiBc76TYkTG/mhrnYTuzfHw== + dependencies: + path-key "^2.0.0" + npm-run-path@^4.0.0: version "4.0.1" resolved "https://registry.yarnpkg.com/npm-run-path/-/npm-run-path-4.0.1.tgz#b7ecd1e5ed53da8e37a55e1c2269e0b97ed748ea" @@ -1668,6 +1716,11 @@ ospath@^1.2.2: resolved "https://registry.yarnpkg.com/ospath/-/ospath-1.2.2.tgz#1276639774a3f8ef2572f7fe4280e0ea4550c07b" integrity sha512-o6E5qJV5zkAbIDNhGSIlyOhScKXgQrSRMilfph0clDfM0nEnBOlKlH4sWDmG95BW/CvwNz0vmm7dJVtU2KlMiA== +p-finally@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/p-finally/-/p-finally-1.0.0.tgz#3fbcfb15b899a44123b34b6dcc18b724336a2cae" + integrity sha512-LICb2p9CB7FS+0eR1oqWnHhp0FljGLZCWBE9aix0Uye9W8LTQPwMTYVGWQWIw9RdQiDg4+epXQODwIYJtSJaow== + p-limit@^2.2.0: version "2.3.0" resolved "https://registry.npmjs.org/p-limit/-/p-limit-2.3.0.tgz" @@ -1718,6 +1771,11 @@ path-is-absolute@^1.0.0: resolved "https://registry.yarnpkg.com/path-is-absolute/-/path-is-absolute-1.0.1.tgz#174b9268735534ffbc7ace6bf53a5a9e1b5c5f5f" integrity sha512-AVbw3UJ2e9bq64vSaS9Am0fje1Pa8pbGqTTsmXfaIiMpnr5DlDhfJOuLj9Sf95ZPVDAUerDfEk88MPmPe7UCQg== +path-key@^2.0.0, path-key@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/path-key/-/path-key-2.0.1.tgz#411cadb574c5a140d3a4b1910d40d80cc9f40b40" + integrity sha512-fEHGKCSmUSDPv4uoj8AlD+joPlq3peND+HRYyxFz4KPw4z926S/b8rIuFs2FYJg3BwsxJf6A9/3eIdLaYC+9Dw== + path-key@^3.0.0, path-key@^3.1.0: version "3.1.1" resolved "https://registry.yarnpkg.com/path-key/-/path-key-3.1.1.tgz#581f6ade658cbba65a0d3380de7753295054f375" @@ -1875,6 +1933,11 @@ sass@^1.56.1: immutable "^4.0.0" source-map-js ">=0.6.2 <2.0.0" +semver@^5.5.0: + version "5.7.2" + resolved "https://registry.yarnpkg.com/semver/-/semver-5.7.2.tgz#48d55db737c3287cd4835e17fa13feace1c41ef8" + integrity sha512-cBznnQ9KjJqU67B52RMC65CMarK2600WFnbkcaiwWq3xy/5haFJlshgnpjovMVJ+Hff49d8GEn0b87C5pDQ10g== + semver@^7.5.3: version "7.6.0" resolved "https://registry.yarnpkg.com/semver/-/semver-7.6.0.tgz#1a46a4db4bffcccd97b743b5005c8325f23d4e2d" @@ -1906,6 +1969,13 @@ set-function-length@^1.2.1: gopd "^1.0.1" has-property-descriptors "^1.0.1" +shebang-command@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/shebang-command/-/shebang-command-1.2.0.tgz#44aac65b695b03398968c39f363fee5deafdf1ea" + integrity sha512-EV3L1+UQWGor21OmnvojK36mhg+TyIKDh3iFBKBohr5xeXIhNBcx8oWdgkTEEQ+BEFFYdLRuqMfd5L84N1V5Vg== + dependencies: + shebang-regex "^1.0.0" + shebang-command@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/shebang-command/-/shebang-command-2.0.0.tgz#ccd0af4f8835fbdc265b82461aaf0c36663f34ea" @@ -1913,6 +1983,11 @@ shebang-command@^2.0.0: dependencies: shebang-regex "^3.0.0" +shebang-regex@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/shebang-regex/-/shebang-regex-1.0.0.tgz#da42f49740c0b42db2ca9728571cb190c98efea3" + integrity sha512-wpoSFAxys6b2a2wHZ1XpDSgD7N9iVjg29Ph9uV/uaP9Ex/KXlkTZTeddxDPSYQpgvzKLGJke2UU0AzoGCjNIvQ== + shebang-regex@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/shebang-regex/-/shebang-regex-3.0.0.tgz#ae16f1644d873ecad843b0307b143362d4c42172" @@ -1933,7 +2008,7 @@ side-channel@^1.0.4: get-intrinsic "^1.2.4" object-inspect "^1.13.1" -signal-exit@^3.0.2: +signal-exit@^3.0.0, signal-exit@^3.0.2: version "3.0.7" resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.7.tgz#a9a1767f8af84155114eaabd73f99273c8f59ad9" integrity sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ== @@ -1976,6 +2051,15 @@ sshpk@^1.14.1: safer-buffer "^2.0.2" tweetnacl "~0.14.0" +stop-only@^3.3.1: + version "3.3.1" + resolved "https://registry.yarnpkg.com/stop-only/-/stop-only-3.3.1.tgz#0fc5126d556ba624641e655069abacd2f6c1d121" + integrity sha512-G8x+lRj6RU9pAm+gxaK7wIHmQ2ws6YlXwC2e/WgIwDdrMkr5iPWakLxD5BVOTVyw/mA578N8EBxSXjL4tDQu0A== + dependencies: + debug "4.3.4" + execa "0.11.0" + minimist "1.2.8" + string-width@^4.1.0, string-width@^4.2.0: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" @@ -1992,6 +2076,11 @@ strip-ansi@^6.0.0, strip-ansi@^6.0.1: dependencies: ansi-regex "^5.0.1" +strip-eof@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/strip-eof/-/strip-eof-1.0.0.tgz#bb43ff5598a6eb05d89b59fcd129c983313606bf" + integrity sha512-7FCwGGmx8mD5xQd3RPUvnSpUXHM3BWuzjtpD4TXsfcZ9EL4azvVVUscFYwD9nx8Kh+uCBC00XBtAykoMHwTh8Q== + strip-final-newline@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/strip-final-newline/-/strip-final-newline-2.0.0.tgz#89b852fb2fcbe936f6f4b3187afb0a12c1ab58ad" @@ -2146,6 +2235,13 @@ which@2.0.2, which@^2.0.1: dependencies: isexe "^2.0.0" +which@^1.2.9: + version "1.3.1" + resolved "https://registry.yarnpkg.com/which/-/which-1.3.1.tgz#a45043d54f5805316da8d62f9f50918d3da70b0a" + integrity sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ== + dependencies: + isexe "^2.0.0" + workerpool@6.2.0: version "6.2.0" resolved "https://registry.npmjs.org/workerpool/-/workerpool-6.2.0.tgz" From 8881805011e77e5904c27d3dcd3f8c754f870429 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 27 Feb 2024 16:31:05 +0000 Subject: [PATCH 2/4] add resending invites once expired --- .../e2e/supporter/manage-team-members.cy.js | 21 ++- internal/app/app.go | 2 +- internal/app/member_store.go | 12 +- internal/app/member_store_test.go | 4 +- internal/page/fixtures/donor.go | 1 + internal/page/fixtures/supporter.go | 36 +++- .../page/supporter/manage_team_members.go | 63 ++++++- .../supporter/manage_team_members_test.go | 159 +++++++++++++++++- .../page/supporter/mock_MemberStore_test.go | 48 ++++++ internal/page/supporter/register.go | 9 +- .../supporter/manage_team_members.gohtml | 16 +- 11 files changed, 345 insertions(+), 26 deletions(-) diff --git a/cypress/e2e/supporter/manage-team-members.cy.js b/cypress/e2e/supporter/manage-team-members.cy.js index f6ddd11f8b..a6fee64d82 100644 --- a/cypress/e2e/supporter/manage-team-members.cy.js +++ b/cypress/e2e/supporter/manage-team-members.cy.js @@ -10,13 +10,11 @@ describe('Organisation details', () => { cy.contains("td", "kamal-singh@example.org").parent().within(() => { cy.contains("Kamal Singh") cy.contains("Invite pending") - cy.contains("a", "Resend invite") }) cy.contains("td", "jo-alessi@example.org").parent().within(() => { cy.contains("Jo Alessi") cy.contains("Invite pending") - cy.contains("a", "Resend invite") }) cy.contains("Team members") @@ -33,4 +31,23 @@ describe('Organisation details', () => { cy.contains("Active") }) }); + + it('shows resend invite when expired', () => { + cy.visit('/fixtures/supporter?organisation=1&redirect=/manage-organisation/manage-team-members&invitedMembers=1&permission=admin&expireInvites=1'); + + cy.checkA11yApp(); + cy.contains("a", "Manage team members").click() + + cy.contains("Invited team members") + + cy.contains("td", "kamal-singh@example.org").parent().within(() => { + cy.contains("button", "Resend invite").click({force: true}) + }) + + cy.url().should("contain", "/manage-organisation/manage-team-members"); + cy.checkA11yApp(); + + cy.contains(".govuk-notification-banner--success", "kamal-singh@example.org"); + cy.get("main").should("not.contain", "Resend invite"); + }); }); diff --git a/internal/app/app.go b/internal/app/app.go index 63257c34f4..da3767428f 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -124,7 +124,7 @@ func App( handleRoot(page.Paths.AttorneyFixtures, None, fixtures.Attorney(tmpls.Get("attorney_fixtures.gohtml"), sessionStore, shareCodeSender, donorStore, certificateProviderStore, attorneyStore)) handleRoot(page.Paths.SupporterFixtures, None, - fixtures.Supporter(sessionStore, organisationStore, donorStore, memberStore)) + fixtures.Supporter(sessionStore, organisationStore, donorStore, memberStore, lpaDynamoClient)) handleRoot(page.Paths.DashboardFixtures, None, fixtures.Dashboard(tmpls.Get("dashboard_fixtures.gohtml"), sessionStore, shareCodeSender, donorStore, certificateProviderStore, attorneyStore)) handleRoot(page.Paths.YourLegalRightsAndResponsibilities, None, diff --git a/internal/app/member_store.go b/internal/app/member_store.go index 2993a043ab..44b139fc1e 100644 --- a/internal/app/member_store.go +++ b/internal/app/member_store.go @@ -47,6 +47,14 @@ func (s *memberStore) CreateMemberInvite(ctx context.Context, organisation *acto return nil } +func (s *memberStore) DeleteMemberInvite(ctx context.Context, organisationID, email string) error { + if err := s.dynamoClient.DeleteOne(ctx, organisationKey(organisationID), memberInviteKey(email)); err != nil { + return fmt.Errorf("error deleting member invite: %w", err) + } + + return nil +} + func (s *memberStore) Create(ctx context.Context, invite *actor.MemberInvite) error { data, err := page.SessionDataFromContext(ctx) if err != nil { @@ -73,8 +81,8 @@ func (s *memberStore) Create(ctx context.Context, invite *actor.MemberInvite) er return fmt.Errorf("error creating organisation member: %w", err) } - if err := s.dynamoClient.DeleteOne(ctx, invite.PK, invite.SK); err != nil { - return fmt.Errorf("error deleting member invite: %w", err) + if err := s.DeleteMemberInvite(ctx, invite.OrganisationID, invite.Email); err != nil { + return err } link := &organisationLink{ diff --git a/internal/app/member_store_test.go b/internal/app/member_store_test.go index 006ead68c7..a9a54e6bd5 100644 --- a/internal/app/member_store_test.go +++ b/internal/app/member_store_test.go @@ -292,7 +292,7 @@ func TestMemberStoreCreate(t *testing.T) { invite := &actor.MemberInvite{ PK: "pk", SK: "sk", - Email: "ab@example.ord", + Email: "ab@example.org", FirstNames: "a", LastName: "b", Permission: actor.Admin, @@ -316,7 +316,7 @@ func TestMemberStoreCreate(t *testing.T) { Return(nil) dynamoClient.EXPECT(). - DeleteOne(ctx, "pk", "sk"). + DeleteOne(ctx, "ORGANISATION#org-id", "MEMBERINVITE#YWJAZXhhbXBsZS5vcmc="). Return(nil) dynamoClient.EXPECT(). diff --git a/internal/page/fixtures/donor.go b/internal/page/fixtures/donor.go index 4d39548e8b..d7d6f242c0 100644 --- a/internal/page/fixtures/donor.go +++ b/internal/page/fixtures/donor.go @@ -25,6 +25,7 @@ import ( type DynamoClient interface { OneByUID(ctx context.Context, uid string, v interface{}) error + Create(ctx context.Context, v interface{}) error } type DocumentStore interface { diff --git a/internal/page/fixtures/supporter.go b/internal/page/fixtures/supporter.go index 047718b688..d9a5bc4fb8 100644 --- a/internal/page/fixtures/supporter.go +++ b/internal/page/fixtures/supporter.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "fmt" + "log" "net/http" "strconv" "strings" @@ -25,7 +26,7 @@ type MemberStore interface { CreateMemberInvite(ctx context.Context, organisation *actor.Organisation, firstNames, lastname, email, code string, permission actor.Permission) error } -func Supporter(sessionStore sesh.Store, organisationStore OrganisationStore, donorStore DonorStore, memberStore MemberStore) page.Handler { +func Supporter(sessionStore sesh.Store, organisationStore OrganisationStore, donorStore DonorStore, memberStore MemberStore, dynamoClient DynamoClient) page.Handler { return func(appData page.AppData, w http.ResponseWriter, r *http.Request) error { var ( invitedMembers = r.FormValue("invitedMembers") @@ -35,6 +36,7 @@ func Supporter(sessionStore sesh.Store, organisationStore OrganisationStore, don redirect = r.FormValue("redirect") asMember = r.FormValue("asMember") permission = r.FormValue("permission") + expireInvites = r.FormValue("expireInvites") == "1" supporterSub = random.String(16) supporterSessionID = base64.StdEncoding.EncodeToString([]byte(supporterSub)) @@ -75,15 +77,43 @@ func Supporter(sessionStore sesh.Store, organisationStore OrganisationStore, don if invitedMembers != "" { n, err := strconv.Atoi(invitedMembers) + if err != nil { + return fmt.Errorf("invitedMembers should be a number") + } for i, member := range invitedOrgMemberNames { if i == n { break } - if err = memberStore.CreateMemberInvite(page.ContextWithSessionData(r.Context(), &page.SessionData{OrganisationID: org.ID}), org, member.Firstnames, member.Lastname, strings.ToLower(fmt.Sprintf("%s-%s@example.org", member.Firstnames, member.Lastname)), random.String(12), actor.Admin); err != nil { - return err + email := strings.ToLower(fmt.Sprintf("%s-%s@example.org", member.Firstnames, member.Lastname)) + + now := time.Now() + if expireInvites { + now = now.Add(time.Hour * -time.Duration(48)) + log.Println(now) } + + invite := &actor.MemberInvite{ + PK: "ORGANISATION#" + org.ID, + SK: "MEMBERINVITE#" + base64.StdEncoding.EncodeToString([]byte(email)), + CreatedAt: now, + OrganisationID: org.ID, + OrganisationName: org.Name, + Email: email, + FirstNames: member.Firstnames, + LastName: member.Lastname, + Permission: actor.Admin, + ReferenceNumber: random.String(12), + } + + if err := dynamoClient.Create(page.ContextWithSessionData(r.Context(), &page.SessionData{OrganisationID: org.ID}), invite); err != nil { + return fmt.Errorf("error creating member invite: %w", err) + } + + //if err = memberStore.CreateMemberInvite(page.ContextWithSessionData(r.Context(), &page.SessionData{OrganisationID: org.ID}), org, member.Firstnames, member.Lastname, strings.ToLower(fmt.Sprintf("%s-%s@example.org", member.Firstnames, member.Lastname)), random.String(12), actor.Admin); err != nil { + // return err + //} } } diff --git a/internal/page/supporter/manage_team_members.go b/internal/page/supporter/manage_team_members.go index f65e968fb5..bc30e02e17 100644 --- a/internal/page/supporter/manage_team_members.go +++ b/internal/page/supporter/manage_team_members.go @@ -1,10 +1,13 @@ package supporter import ( + "errors" "net/http" + "net/url" "github.com/ministryofjustice/opg-go-common/template" "github.com/ministryofjustice/opg-modernising-lpa/internal/actor" + "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" "github.com/ministryofjustice/opg-modernising-lpa/internal/page" "github.com/ministryofjustice/opg-modernising-lpa/internal/validation" ) @@ -15,10 +18,58 @@ type manageTeamMembersData struct { Organisation *actor.Organisation InvitedMembers []*actor.MemberInvite Members []*actor.Member + Form *inviteMemberForm } -func ManageTeamMembers(tmpl template.Template, memberStore MemberStore) Handler { +func ManageTeamMembers(tmpl template.Template, memberStore MemberStore, randomString func(int) string, notifyClient NotifyClient, appPublicURL string) Handler { return func(appData page.AppData, w http.ResponseWriter, r *http.Request, organisation *actor.Organisation) error { + data := &manageTeamMembersData{ + App: appData, + Organisation: organisation, + } + + if r.Method == http.MethodPost { + data.Form = readInviteMemberForm(r) + data.Errors = data.Form.Validate() + + if data.Errors.None() { + session, err := page.SessionDataFromContext(r.Context()) + if err != nil { + return err + } + + if err := memberStore.DeleteMemberInvite(r.Context(), organisation.ID, data.Form.Email); err != nil { + return err + } + + inviteCode := randomString(12) + if err := memberStore.CreateMemberInvite( + r.Context(), + organisation, + data.Form.FirstNames, + data.Form.LastName, + data.Form.Email, + inviteCode, + data.Form.Permission, + ); err != nil { + return err + } + + if err := notifyClient.SendEmail(r.Context(), data.Form.Email, notify.OrganisationMemberInviteEmail{ + OrganisationName: organisation.Name, + InviterEmail: session.Email, + InviteCode: inviteCode, + JoinAnOrganisationURL: appPublicURL + page.Paths.Supporter.Start.Format(), + }); err != nil { + return err + } + + return page.Paths.Supporter.ManageTeamMembers.RedirectQuery(w, r, appData, url.Values{"inviteSent": {data.Form.Email}}) + } + + return errors.New("unable to resend invite") + } + invitedMembers, err := memberStore.InvitedMembers(r.Context()) if err != nil { return err @@ -29,11 +80,9 @@ func ManageTeamMembers(tmpl template.Template, memberStore MemberStore) Handler return err } - return tmpl(w, &manageTeamMembersData{ - App: appData, - Organisation: organisation, - InvitedMembers: invitedMembers, - Members: members, - }) + data.InvitedMembers = invitedMembers + data.Members = members + + return tmpl(w, data) } } diff --git a/internal/page/supporter/manage_team_members_test.go b/internal/page/supporter/manage_team_members_test.go index c4340149af..994c8ce929 100644 --- a/internal/page/supporter/manage_team_members_test.go +++ b/internal/page/supporter/manage_team_members_test.go @@ -1,11 +1,16 @@ package supporter import ( + "context" "net/http" "net/http/httptest" + "net/url" + "strings" "testing" "github.com/ministryofjustice/opg-modernising-lpa/internal/actor" + "github.com/ministryofjustice/opg-modernising-lpa/internal/notify" + "github.com/ministryofjustice/opg-modernising-lpa/internal/page" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) @@ -32,7 +37,7 @@ func TestGetManageTeamMembers(t *testing.T) { }). Return(nil) - err := ManageTeamMembers(template.Execute, memberStore)(testAppData, w, r, &actor.Organisation{ID: "org-id"}) + err := ManageTeamMembers(template.Execute, memberStore, nil, nil, "")(testAppData, w, r, &actor.Organisation{ID: "org-id"}) resp := w.Result() @@ -40,7 +45,7 @@ func TestGetManageTeamMembers(t *testing.T) { assert.Equal(t, http.StatusOK, resp.StatusCode) } -func TestGetManageTeamMembersWhenOrganisationStoreErrors(t *testing.T) { +func TestGetManageTeamMembersWhenMemberStoreErrors(t *testing.T) { testcases := map[string]struct { invitedMembersError error membersError error @@ -69,7 +74,7 @@ func TestGetManageTeamMembersWhenOrganisationStoreErrors(t *testing.T) { Return([]*actor.Member{}, tc.membersError) } - err := ManageTeamMembers(nil, memberStore)(testAppData, w, r, &actor.Organisation{}) + err := ManageTeamMembers(nil, memberStore, nil, nil, "")(testAppData, w, r, &actor.Organisation{}) resp := w.Result() @@ -96,10 +101,156 @@ func TestGetManageTeamMembersWhenTemplateError(t *testing.T) { Execute(w, mock.Anything). Return(expectedError) - err := ManageTeamMembers(template.Execute, memberStore)(testAppData, w, r, &actor.Organisation{}) + err := ManageTeamMembers(template.Execute, memberStore, nil, nil, "")(testAppData, w, r, &actor.Organisation{}) resp := w.Result() assert.Equal(t, expectedError, err) assert.Equal(t, http.StatusOK, resp.StatusCode) } + +func TestPostManageTeamMembers(t *testing.T) { + form := url.Values{ + "email": {"email@example.com"}, + "first-names": {"a"}, + "last-name": {"b"}, + "permission": {"admin"}, + } + + ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{Email: "inviter@example.com"}) + w := httptest.NewRecorder() + r, _ := http.NewRequestWithContext(ctx, http.MethodPost, "/", strings.NewReader(form.Encode())) + r.Header.Add("Content-Type", page.FormUrlEncoded) + + organisation := &actor.Organisation{Name: "My organisation", ID: "org-id"} + + memberStore := newMockMemberStore(t) + memberStore.EXPECT(). + DeleteMemberInvite(r.Context(), organisation.ID, "email@example.com"). + Return(nil) + memberStore.EXPECT(). + CreateMemberInvite(r.Context(), organisation, "a", "b", "email@example.com", "abcde", actor.Admin). + Return(nil) + + notifyClient := newMockNotifyClient(t) + notifyClient.EXPECT(). + SendEmail(r.Context(), "email@example.com", notify.OrganisationMemberInviteEmail{ + OrganisationName: "My organisation", + InviterEmail: "inviter@example.com", + InviteCode: "abcde", + JoinAnOrganisationURL: "http://base" + page.Paths.Supporter.Start.Format(), + }). + Return(nil) + + err := ManageTeamMembers(nil, memberStore, func(int) string { return "abcde" }, notifyClient, "http://base")(testAppData, w, r, organisation) + + resp := w.Result() + + assert.Nil(t, err) + assert.Equal(t, http.StatusFound, resp.StatusCode) + assert.Equal(t, page.Paths.Supporter.ManageTeamMembers.Format()+"?inviteSent=email%40example.com", resp.Header.Get("Location")) +} + +func TestPostManageTeamMembersWhenValidationErrors(t *testing.T) { + form := url.Values{ + "email": {"not an email"}, + "first-names": {"a"}, + "last-name": {"b"}, + "permission": {"admin"}, + } + + ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{Email: "inviter@example.com"}) + w := httptest.NewRecorder() + r, _ := http.NewRequestWithContext(ctx, http.MethodPost, "/", strings.NewReader(form.Encode())) + r.Header.Add("Content-Type", page.FormUrlEncoded) + + err := ManageTeamMembers(nil, nil, func(int) string { return "abcde" }, nil, "http://base")(testAppData, w, r, &actor.Organisation{ID: "org-id", Name: "My organisation"}) + + resp := w.Result() + + assert.Error(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) +} + +func TestPostManageTeamMembersWhenMemberStoreErrors(t *testing.T) { + testcases := map[string]struct { + deleteMembersError error + createMemberInvite error + }{ + "DeleteMemberInvite error": { + deleteMembersError: expectedError, + }, + "CreateMemberInvite error": { + createMemberInvite: expectedError, + }, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + form := url.Values{ + "email": {"email@example.com"}, + "first-names": {"a"}, + "last-name": {"b"}, + "permission": {"admin"}, + } + + ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{Email: "inviter@example.com"}) + w := httptest.NewRecorder() + r, _ := http.NewRequestWithContext(ctx, http.MethodPost, "/", strings.NewReader(form.Encode())) + r.Header.Add("Content-Type", page.FormUrlEncoded) + + memberStore := newMockMemberStore(t) + memberStore.EXPECT(). + DeleteMemberInvite(mock.Anything, mock.Anything, mock.Anything). + Return(tc.deleteMembersError) + + if tc.createMemberInvite != nil { + memberStore.EXPECT(). + CreateMemberInvite(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(tc.createMemberInvite) + } + + err := ManageTeamMembers(nil, memberStore, func(int) string { return "abcde" }, nil, "")(testAppData, w, r, &actor.Organisation{}) + + resp := w.Result() + + assert.Equal(t, expectedError, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + }) + } +} + +func TestPostManageTeamMembersWhenNotifyClientError(t *testing.T) { + form := url.Values{ + "email": {"email@example.com"}, + "first-names": {"a"}, + "last-name": {"b"}, + "permission": {"admin"}, + } + + ctx := page.ContextWithSessionData(context.Background(), &page.SessionData{Email: "inviter@example.com"}) + w := httptest.NewRecorder() + r, _ := http.NewRequestWithContext(ctx, http.MethodPost, "/", strings.NewReader(form.Encode())) + r.Header.Add("Content-Type", page.FormUrlEncoded) + + memberStore := newMockMemberStore(t) + memberStore.EXPECT(). + DeleteMemberInvite(mock.Anything, mock.Anything, mock.Anything). + Return(nil) + memberStore.EXPECT(). + CreateMemberInvite(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(nil) + + notifyClient := newMockNotifyClient(t) + notifyClient.EXPECT(). + SendEmail(mock.Anything, mock.Anything, mock.Anything). + Return(expectedError) + + err := ManageTeamMembers(nil, memberStore, func(int) string { return "abcde" }, notifyClient, "")(testAppData, w, r, &actor.Organisation{}) + + resp := w.Result() + + assert.Equal(t, expectedError, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + +} diff --git a/internal/page/supporter/mock_MemberStore_test.go b/internal/page/supporter/mock_MemberStore_test.go index 02923e3840..6d488c9d4f 100644 --- a/internal/page/supporter/mock_MemberStore_test.go +++ b/internal/page/supporter/mock_MemberStore_test.go @@ -122,6 +122,54 @@ func (_c *mockMemberStore_CreateMemberInvite_Call) RunAndReturn(run func(context return _c } +// DeleteMemberInvite provides a mock function with given fields: ctx, organisationID, email +func (_m *mockMemberStore) DeleteMemberInvite(ctx context.Context, organisationID string, email string) error { + ret := _m.Called(ctx, organisationID, email) + + if len(ret) == 0 { + panic("no return value specified for DeleteMemberInvite") + } + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) error); ok { + r0 = rf(ctx, organisationID, email) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// mockMemberStore_DeleteMemberInvite_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteMemberInvite' +type mockMemberStore_DeleteMemberInvite_Call struct { + *mock.Call +} + +// DeleteMemberInvite is a helper method to define mock.On call +// - ctx context.Context +// - organisationID string +// - email string +func (_e *mockMemberStore_Expecter) DeleteMemberInvite(ctx interface{}, organisationID interface{}, email interface{}) *mockMemberStore_DeleteMemberInvite_Call { + return &mockMemberStore_DeleteMemberInvite_Call{Call: _e.mock.On("DeleteMemberInvite", ctx, organisationID, email)} +} + +func (_c *mockMemberStore_DeleteMemberInvite_Call) Run(run func(ctx context.Context, organisationID string, email string)) *mockMemberStore_DeleteMemberInvite_Call { + _c.Call.Run(func(args mock.Arguments) { + run(args[0].(context.Context), args[1].(string), args[2].(string)) + }) + return _c +} + +func (_c *mockMemberStore_DeleteMemberInvite_Call) Return(_a0 error) *mockMemberStore_DeleteMemberInvite_Call { + _c.Call.Return(_a0) + return _c +} + +func (_c *mockMemberStore_DeleteMemberInvite_Call) RunAndReturn(run func(context.Context, string, string) error) *mockMemberStore_DeleteMemberInvite_Call { + _c.Call.Return(run) + return _c +} + // Get provides a mock function with given fields: ctx func (_m *mockMemberStore) Get(ctx context.Context) (*actor.Member, error) { ret := _m.Called(ctx) diff --git a/internal/page/supporter/register.go b/internal/page/supporter/register.go index 84a3d2917d..8f89f13f88 100644 --- a/internal/page/supporter/register.go +++ b/internal/page/supporter/register.go @@ -27,11 +27,12 @@ type OrganisationStore interface { type MemberStore interface { Create(ctx context.Context, invite *actor.MemberInvite) error CreateMemberInvite(ctx context.Context, organisation *actor.Organisation, firstNames, lastname, email, code string, permission actor.Permission) error - InvitedMember(ctx context.Context) (*actor.MemberInvite, error) - InvitedMembers(ctx context.Context) ([]*actor.MemberInvite, error) + DeleteMemberInvite(ctx context.Context, organisationID, email string) error Get(ctx context.Context) (*actor.Member, error) - GetByID(ctx context.Context, memberID string) (*actor.Member, error) GetAll(ctx context.Context) ([]*actor.Member, error) + GetByID(ctx context.Context, memberID string) (*actor.Member, error) + InvitedMember(ctx context.Context) (*actor.MemberInvite, error) + InvitedMembers(ctx context.Context) ([]*actor.MemberInvite, error) Put(ctx context.Context, member *actor.Member) error } @@ -100,7 +101,7 @@ func Register( handleWithSupporter(paths.EditOrganisationName, page.None, EditOrganisationName(tmpls.Get("edit_organisation_name.gohtml"), organisationStore)) handleWithSupporter(paths.ManageTeamMembers, page.None, - ManageTeamMembers(tmpls.Get("manage_team_members.gohtml"), memberStore)) + ManageTeamMembers(tmpls.Get("manage_team_members.gohtml"), memberStore, random.String, notifyClient, appPublicURL)) handleWithSupporter(paths.InviteMember, page.CanGoBack, InviteMember(tmpls.Get("invite_member.gohtml"), memberStore, notifyClient, random.String, appPublicURL)) handleWithSupporter(paths.EditMember, page.CanGoBack, diff --git a/web/template/supporter/manage_team_members.gohtml b/web/template/supporter/manage_team_members.gohtml index b300eb2bf0..3792e10fa5 100644 --- a/web/template/supporter/manage_team_members.gohtml +++ b/web/template/supporter/manage_team_members.gohtml @@ -114,7 +114,21 @@ {{ tr $.App "invitePending" }} - {{ tr $.App "resendInvite" }} {{ trFormat $.App "toMemberName" "MemberName" .FullName }} + {{ if .HasExpired }} +
+ + + + + + + + {{ template "csrf-field" $ }} +
+ {{ end }} {{ end }} From 07e7317b0b4239cc543be31c554b4435395af710 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 27 Feb 2024 16:38:08 +0000 Subject: [PATCH 3/4] tidy --- internal/page/fixtures/supporter.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/page/fixtures/supporter.go b/internal/page/fixtures/supporter.go index d9a5bc4fb8..46a8d65bd0 100644 --- a/internal/page/fixtures/supporter.go +++ b/internal/page/fixtures/supporter.go @@ -110,10 +110,6 @@ func Supporter(sessionStore sesh.Store, organisationStore OrganisationStore, don if err := dynamoClient.Create(page.ContextWithSessionData(r.Context(), &page.SessionData{OrganisationID: org.ID}), invite); err != nil { return fmt.Errorf("error creating member invite: %w", err) } - - //if err = memberStore.CreateMemberInvite(page.ContextWithSessionData(r.Context(), &page.SessionData{OrganisationID: org.ID}), org, member.Firstnames, member.Lastname, strings.ToLower(fmt.Sprintf("%s-%s@example.org", member.Firstnames, member.Lastname)), random.String(12), actor.Admin); err != nil { - // return err - //} } } From 0128211db0527fc3f0a4bba220c131eacaf3410e Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Tue, 27 Feb 2024 17:11:30 +0000 Subject: [PATCH 4/4] cover missing session --- .../supporter/manage_team_members_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/internal/page/supporter/manage_team_members_test.go b/internal/page/supporter/manage_team_members_test.go index 994c8ce929..17c492099a 100644 --- a/internal/page/supporter/manage_team_members_test.go +++ b/internal/page/supporter/manage_team_members_test.go @@ -172,6 +172,26 @@ func TestPostManageTeamMembersWhenValidationErrors(t *testing.T) { assert.Equal(t, http.StatusOK, resp.StatusCode) } +func TestPostManageTeamMembersWhenSessionMissing(t *testing.T) { + form := url.Values{ + "email": {"a@b.com"}, + "first-names": {"a"}, + "last-name": {"b"}, + "permission": {"admin"}, + } + + w := httptest.NewRecorder() + r, _ := http.NewRequest(http.MethodPost, "/", strings.NewReader(form.Encode())) + r.Header.Add("Content-Type", page.FormUrlEncoded) + + err := ManageTeamMembers(nil, nil, func(int) string { return "abcde" }, nil, "http://base")(testAppData, w, r, &actor.Organisation{ID: "org-id", Name: "My organisation"}) + + resp := w.Result() + + assert.Error(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) +} + func TestPostManageTeamMembersWhenMemberStoreErrors(t *testing.T) { testcases := map[string]struct { deleteMembersError error