Skip to content

Commit

Permalink
MLPAB-2213: Fix attorney and replacements duplicates bug (#1376)
Browse files Browse the repository at this point in the history
  • Loading branch information
acsauk authored Jul 29, 2024
1 parent 4d78c9f commit d3f577f
Show file tree
Hide file tree
Showing 29 changed files with 300 additions and 160 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/ui_test_job.yml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ jobs:
- name: Setup node
uses: actions/setup-node@v4
if: inputs.skip != true
with:
node-version-file: "package.json"

- name: Install dependencies
if: inputs.skip != true
Expand Down
3 changes: 3 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
default_language_version:
node: system

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
Expand Down
2 changes: 1 addition & 1 deletion .tool-versions
Original file line number Diff line number Diff line change
@@ -1 +1 @@
nodejs 20.15.1
nodejs 20.16.0
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ make up
make run-cypress
```

Cypress is run locally rather than in docker. To ensure version parity of nodeJS use a version manager, such as asdf, to parse `.tool-versions`:

```shell
asdf install
```

### Local development

To run the app in dev mode on amd64/intel:
Expand Down
6 changes: 4 additions & 2 deletions cypress/e2e/accessibility/data-loss-warning.cy.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
describe('Data loss warnings', () => {
describe('Return to task list', () => {
it('locks focus to data loss warning dialog', () => {
cy.visit('/fixtures?redirect=/choose-attorneys&progress=provideYourDetails');
cy.visit('/fixtures?redirect=/choose-attorneys-guidance&progress=provideYourDetails');
cy.contains('button', 'Continue').click()

cy.get('#f-first-names').type('John');
cy.contains('a', 'Return to task list').click()
Expand Down Expand Up @@ -60,7 +61,8 @@ describe('Data loss warnings', () => {

describe('Change language', () => {
it('locks focus to data loss warning dialog', () => {
cy.visit('/fixtures?redirect=/choose-attorneys&progress=provideYourDetails');
cy.visit('/fixtures?redirect=/choose-attorneys-guidance&progress=provideYourDetails');
cy.contains('button', 'Continue').click()

cy.get('#f-first-names').type('John');
cy.contains('a', 'Cymraeg').click()
Expand Down
10 changes: 5 additions & 5 deletions cypress/e2e/donor/choose-attorneys-task.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('Choose attorneys task', () => {
it('is in progress if I start adding an attorney', () => {
cy.visit('/fixtures?redirect=/task-list');
cy.contains('a', 'Choose your attorneys').click();
cy.contains('a', 'Continue').click();
cy.contains('button', 'Continue').click();

cy.get('#f-first-names').type('John');
cy.get('#f-last-name').type('Doe');
Expand All @@ -30,7 +30,7 @@ describe('Choose attorneys task', () => {
it('is completed if enter an attorneys details using address', () => {
cy.visit('/fixtures?redirect=/task-list');
cy.contains('a', 'Choose your attorneys').click();
cy.contains('a', 'Continue').click();
cy.contains('button', 'Continue').click();

cy.get('#f-first-names').type('John');
cy.get('#f-last-name').type('Doe');
Expand Down Expand Up @@ -61,7 +61,7 @@ describe('Choose attorneys task', () => {
it('is in progress if I enter multiple attorneys details', () => {
cy.visit('/fixtures?redirect=/task-list&progress=chooseYourAttorneys&attorneys=single');
cy.contains('a', 'Choose your attorneys').click();
cy.contains('a', 'Continue').click();
cy.contains('button', 'Continue').click();

cy.contains('label', 'Yes').click();
cy.contains('button', 'Continue').click();
Expand All @@ -84,7 +84,7 @@ describe('Choose attorneys task', () => {
it('is completed if I enter multiple attorneys details with how they act', () => {
cy.visit('/fixtures?redirect=/task-list&progress=chooseYourAttorneys&attorneys=single');
cy.contains('a', 'Choose your attorneys').click();
cy.contains('a', 'Continue').click();
cy.contains('button', 'Continue').click();

cy.contains('label', 'Yes').click();
cy.contains('button', 'Continue').click();
Expand Down Expand Up @@ -114,7 +114,7 @@ describe('Choose attorneys task', () => {
it('is completed if I enter multiple attorneys details when jointly', () => {
cy.visit('/fixtures?redirect=/task-list&progress=chooseYourAttorneys&attorneys=single');
cy.contains('a', 'Choose your attorneys').click();
cy.contains('a', 'Continue').click();
cy.contains('button', 'Continue').click();

cy.contains('label', 'Yes').click();
cy.contains('button', 'Continue').click();
Expand Down
26 changes: 15 additions & 11 deletions cypress/e2e/donor/choose-attorneys.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { TestEmail } from "../../support/e2e";

describe('Choose attorneys', () => {
beforeEach(() => {
cy.visit('/fixtures?redirect=/choose-attorneys');
cy.visit('/fixtures?redirect=/choose-attorneys-guidance&progress=provideYourDetails');
cy.contains('button', 'Continue').click();
});

it('can be submitted', () => {
Expand Down Expand Up @@ -85,8 +86,6 @@ describe('Choose attorneys', () => {
});

it('warns when name shared with other actor', () => {
cy.visit('/fixtures?redirect=/choose-attorneys&progress=provideYourDetails');

cy.get('#f-first-names').type('Sam');
cy.get('#f-last-name').type('Smith');
cy.get('#f-date-of-birth').type('1');
Expand All @@ -102,31 +101,27 @@ describe('Choose attorneys', () => {
});

it('permanently warns when date of birth is under 18', () => {
cy.visit('/fixtures?redirect=/choose-replacement-attorneys&progress=provideYourDetails');

cy.get('#f-first-names').type('John');
cy.get('#f-last-name').type('Doe');
cy.get('#f-date-of-birth').type('1');
cy.get('#f-date-of-birth-month').type('2');
cy.get('#f-date-of-birth-year').type(new Date().getFullYear() - 1);
cy.contains('button', 'Save and continue').click();
cy.url().should('contain', '/choose-replacement-attorneys');
cy.url().should('contain', '/choose-attorneys');

cy.contains('This attorney is under 18 years old. You can continue making your LPA but you will not be able to sign it until they are 18.');

cy.contains('button', 'Save and continue').click();
cy.url().should('contain', '/choose-replacement-attorneys-address');
cy.url().should('contain', '/choose-attorneys-address');

cy.visitLpa("/choose-replacement-attorneys-summary")
cy.visitLpa("/choose-attorneys-summary")
cy.contains('a', 'Change').click()
cy.url().should('contain', '/choose-replacement-attorneys');
cy.url().should('contain', '/choose-attorneys');

cy.contains('This attorney is under 18 years old. You can continue making your LPA but you will not be able to sign it until they are 18.');
});

it('warns when date of birth is over 100', () => {
cy.visit('/fixtures?redirect=/choose-attorneys&progress=provideYourDetails');

cy.get('#f-first-names').type('John');
cy.get('#f-last-name').type('Doe');
cy.get('#f-date-of-birth').type('1');
Expand All @@ -139,5 +134,14 @@ describe('Choose attorneys', () => {

cy.contains('button', 'Save and continue').click();
cy.url().should('contain', '/choose-attorneys-address');

// TODO workout why cypress with later versions of node breaks when using back links
// cy.contains('a', 'Back').click()
// cy.url().should('contain', '/choose-attorneys');
//
// cy.get('#f-date-of-birth-year').clear().type(new Date().getFullYear() - 20);
// cy.contains('button', 'Save and continue').click();
//
// cy.url().should('contain', '/choose-attorneys-address');
});
});
2 changes: 1 addition & 1 deletion cypress/e2e/donor/choose-replacement-attorneys-task.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('Choose replacement attorneys task', () => {
cy.visitLpa('/task-list');

cy.contains('a', 'Choose your attorneys').click();
cy.contains('a', 'Continue').click();
cy.contains('button', 'Continue').click();

cy.contains('label', 'Yes').click();
cy.contains('button', 'Continue').click();
Expand Down
22 changes: 15 additions & 7 deletions cypress/e2e/donor/choose-replacement-attorneys.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import { TestEmail } from "../../support/e2e";

describe('Choose replacement attorneys', () => {
beforeEach(() => {
cy.visit('/fixtures?redirect=/choose-replacement-attorneys&progress=chooseYourAttorneys');
cy.visit('/fixtures?redirect=/do-you-want-replacement-attorneys&progress=chooseYourAttorneys');

cy.get('input[name="yes-no"]').check('yes', { force: true })
cy.contains('button', 'Save and continue').click();

cy.url().should('contain', '/choose-replacement-attorneys');
});

it('can be submitted', () => {
Expand Down Expand Up @@ -84,8 +89,6 @@ describe('Choose replacement attorneys', () => {
});

it('warns when name shared with other actor', () => {
cy.visit('/fixtures?redirect=/choose-replacement-attorneys&progress=chooseYourAttorneys');

cy.get('#f-first-names').type('Sam');
cy.get('#f-last-name').type('Smith');
cy.get('#f-date-of-birth').type('1');
Expand All @@ -101,8 +104,6 @@ describe('Choose replacement attorneys', () => {
});

it('permanently warns when date of birth is under 18', () => {
cy.visit('/fixtures?redirect=/choose-replacement-attorneys&progress=chooseYourAttorneys');

cy.get('#f-first-names').type('John');
cy.get('#f-last-name').type('Doe');
cy.get('#f-date-of-birth').type('1');
Expand All @@ -124,8 +125,6 @@ describe('Choose replacement attorneys', () => {
});

it('warns when date of birth is over 100', () => {
cy.visit('/fixtures?redirect=/choose-replacement-attorneys&progress=chooseYourAttorneys');

cy.get('#f-first-names').type('John');
cy.get('#f-last-name').type('Doe');
cy.get('#f-date-of-birth').type('1');
Expand All @@ -138,5 +137,14 @@ describe('Choose replacement attorneys', () => {

cy.contains('button', 'Save and continue').click();
cy.url().should('contain', '/choose-replacement-attorneys-address');

// TODO workout why cypress with later versions of node breaks when using back links
// cy.contains('a', 'Back').click()
// cy.url().should('contain', '/choose-replacement-attorneys');
//
// cy.get('#f-date-of-birth-year').clear().type(new Date().getFullYear() - 20);
// cy.contains('button', 'Save and continue').click();
//
// cy.url().should('contain', '/choose-replacement-attorneys-address');
});
});
7 changes: 2 additions & 5 deletions cypress/e2e/error-pages.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,13 @@ describe('Error pages', () => {
});

it('shows for invalid CSRF tokens', () => {
cy.visit('/fixtures?redirect=/your-details');
cy.visit('/fixtures?redirect=/your-name');
cy.clearCookie('csrf');

cy.get('#f-first-names').type('John');
cy.get('#f-last-name').type('Doe');
cy.get('#f-date-of-birth').type('1');
cy.get('#f-date-of-birth-month').type('2');
cy.get('#f-date-of-birth-year').type('1990');

cy.contains('button', 'Continue').click();
cy.contains('button', 'Save and continue').click();

cy.contains('Sorry, there is a problem with the service');
});
Expand Down
2 changes: 1 addition & 1 deletion docker/mlpa/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ FROM golang:1.22.5-alpine AS base

WORKDIR /app

FROM node:20.2.0-alpine3.16 AS asset-env
FROM node:20.16.0-alpine3.20 AS asset-env

WORKDIR /app

Expand Down
14 changes: 8 additions & 6 deletions internal/page/donor/choose_attorneys.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,17 @@ type chooseAttorneysData struct {
ShowTrustCorporationLink bool
}

func ChooseAttorneys(tmpl template.Template, donorStore DonorStore, newUID func() actoruid.UID) Handler {
func ChooseAttorneys(tmpl template.Template, donorStore DonorStore) Handler {
return func(appData page.AppData, w http.ResponseWriter, r *http.Request, donor *actor.DonorProvidedDetails) error {
addAnother := r.FormValue("addAnother") == "1"
attorney, attorneyFound := donor.Attorneys.Get(actoruid.FromRequest(r))
uid := actoruid.FromRequest(r)

if r.Method == http.MethodGet && len(donor.Attorneys.Attorneys) > 0 && !attorneyFound && !addAnother {
return page.Paths.ChooseAttorneysSummary.Redirect(w, r, appData, donor)
if uid.IsZero() {
return page.Paths.TaskList.Redirect(w, r, appData, donor)
}

addAnother := r.FormValue("addAnother") == "1"
attorney, attorneyFound := donor.Attorneys.Get(uid)

data := &chooseAttorneysData{
App: appData,
Donor: donor,
Expand Down Expand Up @@ -68,7 +70,7 @@ func ChooseAttorneys(tmpl template.Template, donorStore DonorStore, newUID func(

if data.Errors.None() && data.DobWarning == "" && data.NameWarning == nil {
if attorneyFound == false {
attorney = actor.Attorney{UID: newUID()}
attorney = actor.Attorney{UID: uid}
}

attorney.FirstNames = data.Form.FirstNames
Expand Down
33 changes: 33 additions & 0 deletions internal/page/donor/choose_attorneys_guidance.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package donor

import (
"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/actor/actoruid"
"github.com/ministryofjustice/opg-modernising-lpa/internal/page"
"github.com/ministryofjustice/opg-modernising-lpa/internal/validation"
)

type chooseAttorneysGuidanceData struct {
App page.AppData
Errors validation.List
Donor *actor.DonorProvidedDetails
}

func ChooseAttorneysGuidance(tmpl template.Template, newUID func() actoruid.UID) Handler {
return func(appData page.AppData, w http.ResponseWriter, r *http.Request, donor *actor.DonorProvidedDetails) error {
data := &chooseAttorneysGuidanceData{
App: appData,
Donor: donor,
}

if r.Method == http.MethodPost {
return page.Paths.ChooseAttorneys.RedirectQuery(w, r, appData, donor, url.Values{"id": {newUID().String()}})
}

return tmpl(w, data)
}
}
60 changes: 60 additions & 0 deletions internal/page/donor/choose_attorneys_guidance_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package donor

import (
"net/http"
"net/http/httptest"
"testing"

"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
"github.com/ministryofjustice/opg-modernising-lpa/internal/page"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)

func TestGetChooseAttorneysGuidance(t *testing.T) {
w := httptest.NewRecorder()
r, _ := http.NewRequest(http.MethodGet, "/", nil)

template := newMockTemplate(t)
template.EXPECT().
Execute(w, &chooseAttorneysGuidanceData{
App: testAppData,
Donor: &actor.DonorProvidedDetails{},
}).
Return(nil)

err := ChooseAttorneysGuidance(template.Execute, nil)(testAppData, w, r, &actor.DonorProvidedDetails{})
resp := w.Result()

assert.Nil(t, err)
assert.Equal(t, http.StatusOK, resp.StatusCode)
}

func TestGetChooseAttorneysGuidanceWhenTemplateErrors(t *testing.T) {
w := httptest.NewRecorder()
r, _ := http.NewRequest(http.MethodGet, "/", nil)

template := newMockTemplate(t)
template.EXPECT().
Execute(w, mock.Anything).
Return(expectedError)

err := ChooseAttorneysGuidance(template.Execute, nil)(testAppData, w, r, &actor.DonorProvidedDetails{})
resp := w.Result()

assert.Equal(t, expectedError, err)
assert.Equal(t, http.StatusOK, resp.StatusCode)
}

func TestPostChooseAttorneysGuidance(t *testing.T) {
w := httptest.NewRecorder()
r, _ := http.NewRequest(http.MethodPost, "/", nil)
r.Header.Add("Content-Type", page.FormUrlEncoded)

err := ChooseAttorneysGuidance(nil, testUIDFn)(testAppData, w, r, &actor.DonorProvidedDetails{LpaID: "lpa-id"})
resp := w.Result()

assert.Nil(t, err)
assert.Equal(t, http.StatusFound, resp.StatusCode)
assert.Equal(t, page.Paths.ChooseAttorneys.Format("lpa-id")+"?id="+testUID.String(), resp.Header.Get("Location"))
}
Loading

0 comments on commit d3f577f

Please sign in to comment.