Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MLPAB-1552: Remove optional flags from ATTORNEY_SIGN email and channel #181

Merged
merged 4 commits into from
Apr 29, 2024
Merged

MLPAB-1552: Remove optional flags from ATTORNEY_SIGN email and channel #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dfdb6a8
enforce matching on existing and remove optional flags from ATTORNEY_…
acsauk Apr 25, 2024
4720a7d
cover multiple attorneys with single changeset
acsauk Apr 25, 2024
17a6449
stop passing lpa via Changes
acsauk Apr 29, 2024
3b2c54f
make mobile optional
acsauk Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions lambda/update/attorney_sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (a AttorneySign) Apply(lpa *shared.Lpa) []shared.FieldError {
return nil
}

func validateAttorneySign(changes []shared.Change) (AttorneySign, []shared.FieldError) {
func validateAttorneySign(changes []shared.Change, lpa *shared.Lpa) (AttorneySign, []shared.FieldError) {
var data AttorneySign

errors := parse.Changes(changes).
Expand All @@ -43,18 +43,27 @@ func validateAttorneySign(changes []shared.Change) (AttorneySign, []shared.Field
}

data.Index = &i
data.Mobile = lpa.Attorneys[i].Mobile
data.ContactLanguagePreference = lpa.Attorneys[i].ContactLanguagePreference
data.Channel = lpa.Attorneys[i].Channel
data.Email = lpa.Attorneys[i].Email

if lpa.Attorneys[i].SignedAt != nil {
data.SignedAt = *lpa.Attorneys[i].SignedAt
}

return p.
Field("/mobile", &data.Mobile).
Field("/mobile", &data.Mobile, parse.MustMatchExisting()).
Field("/signedAt", &data.SignedAt, parse.Validate(func() []shared.FieldError {
return validate.Time("", data.SignedAt)
})).
}), parse.MustMatchExisting()).
Field("/contactLanguagePreference", &data.ContactLanguagePreference, parse.Validate(func() []shared.FieldError {
return validate.IsValid("", data.ContactLanguagePreference)
})).
}), parse.MustMatchExisting()).
Field("/channel", &data.Channel, parse.Validate(func() []shared.FieldError {
return validate.IsValid("", data.Channel)
}), parse.Optional()).
Field("/email", &data.Email, parse.Optional()).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you'll want to keep this optional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will always expect an email via the online channel...are you referring to this event listener being used for paper attorneys via scanning?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/ministryofjustice/opg-modernising-lpa/pull/1200/files#diff-75a2c79a05d59597c29f665f8d24662c63e9ac5e83bdae3eff8de38a0067920dR150 but it will only be sent when it changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah 😅

}), parse.MustMatchExisting()).
Field("/email", &data.Email, parse.MustMatchExisting(), parse.Optional()).
Consumed()
}).
Consumed()
Expand Down
114 changes: 85 additions & 29 deletions lambda/update/attorney_sign_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,71 +50,73 @@ func TestValidateUpdateAttorneySign(t *testing.T) {

testcases := map[string]struct {
update shared.Update
lpa *shared.Lpa
errors []shared.FieldError
}{
"valid - no previous values": {
update: shared.Update{
Type: "ATTORNEY_SIGN",
Changes: []shared.Change{
{
Key: "/attorneys/1/mobile",
Key: "/attorneys/0/mobile",
New: json.RawMessage(`"07777"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/signedAt",
New: json.RawMessage(`"` + time.Now().Format(time.RFC3339) + `"`),
Key: "/attorneys/0/signedAt",
New: json.RawMessage(`"` + time.Now().Format(time.RFC3339Nano) + `"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/contactLanguagePreference",
Key: "/attorneys/0/contactLanguagePreference",
New: json.RawMessage(`"cy"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/channel",
Key: "/attorneys/0/channel",
New: json.RawMessage(`"online"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/email",
New: json.RawMessage(`"[email protected]"`),
Old: jsonNull,
},
},
},
lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{
{},
}}},
},
"valid - with previous values": {
update: shared.Update{
Type: "ATTORNEY_SIGN",
Changes: []shared.Change{
{
Key: "/attorneys/1/mobile",
Key: "/attorneys/0/mobile",
New: json.RawMessage(`"07777"`),
Old: json.RawMessage(`"06666"`),
},
{
Key: "/attorneys/1/signedAt",
New: json.RawMessage(`"` + now.Format(time.RFC3339) + `"`),
Old: json.RawMessage(`"` + yesterday.Format(time.RFC3339) + `"`),
Key: "/attorneys/0/signedAt",
New: json.RawMessage(`"` + now.Format(time.RFC3339Nano) + `"`),
Old: json.RawMessage(`"` + yesterday.Format(time.RFC3339Nano) + `"`),
},
{
Key: "/attorneys/1/contactLanguagePreference",
Key: "/attorneys/0/contactLanguagePreference",
New: json.RawMessage(`"cy"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/channel",
Key: "/attorneys/0/channel",
New: json.RawMessage(`"online"`),
Old: json.RawMessage(`"paper"`),
},
{
Key: "/attorneys/1/email",
Key: "/attorneys/0/email",
New: json.RawMessage(`"[email protected]"`),
Old: json.RawMessage(`"[email protected]"`),
},
},
},
lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{
{Channel: shared.ChannelPaper, Email: "[email protected]", Mobile: "06666", SignedAt: &yesterday},
}}},
},
"missing all": {
update: shared.Update{Type: "ATTORNEY_SIGN"},
Expand All @@ -127,17 +129,17 @@ func TestValidateUpdateAttorneySign(t *testing.T) {
Type: "ATTORNEY_SIGN",
Changes: []shared.Change{
{
Key: "/attorneys/1/mobile",
Key: "/attorneys/0/mobile",
New: json.RawMessage(`"0777"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/signedAt",
Key: "/attorneys/0/signedAt",
New: json.RawMessage(`"` + time.Now().Format(time.RFC3339) + `"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/contactLanguagePreference",
Key: "/attorneys/0/contactLanguagePreference",
New: json.RawMessage(`"` + shared.LangCy + `"`),
Old: jsonNull,
},
Expand All @@ -147,17 +149,25 @@ func TestValidateUpdateAttorneySign(t *testing.T) {
Old: jsonNull,
},
{
Key: "/attorneys/1/firstNames",
Key: "/attorneys/0/firstNames",
New: json.RawMessage(`"John"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/email",
Key: "/attorneys/0/email",
New: json.RawMessage(`"[email protected]"`),
Old: jsonNull,
},
{
Key: "/attorneys/0/channel",
New: json.RawMessage(`"paper"`),
Old: jsonNull,
},
},
},
lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{
{},
}}},
errors: []shared.FieldError{
{Source: "/changes/3", Detail: "unexpected change provided"},
{Source: "/changes/4", Detail: "unexpected change provided"},
Expand All @@ -168,38 +178,41 @@ func TestValidateUpdateAttorneySign(t *testing.T) {
Type: "ATTORNEY_SIGN",
Changes: []shared.Change{
{
Key: "/attorneys/1/mobile",
Key: "/attorneys/0/mobile",
New: json.RawMessage(`"07777"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/signedAt",
Key: "/attorneys/0/signedAt",
New: json.RawMessage(`"` + time.Now().Format(time.RFC3339) + `"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/contactLanguagePreference",
Key: "/attorneys/0/contactLanguagePreference",
New: json.RawMessage(`"xy"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/channel",
Key: "/attorneys/0/channel",
New: json.RawMessage(`"digital"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/email",
Key: "/attorneys/0/email",
New: json.RawMessage(`"[email protected]"`),
Old: jsonNull,
},
},
},
lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{
{},
}}},
errors: []shared.FieldError{
{Source: "/changes/2/new", Detail: "invalid value"},
{Source: "/changes/3/new", Detail: "invalid value"},
},
},
"multiple attorneys": {
"multiple attorneys - multiple attorney changes": {
update: shared.Update{
Type: "ATTORNEY_SIGN",
Changes: []shared.Change{
Expand All @@ -223,18 +236,61 @@ func TestValidateUpdateAttorneySign(t *testing.T) {
New: json.RawMessage(`"[email protected]"`),
Old: jsonNull,
},
{
Key: "/attorneys/0/channel",
New: json.RawMessage(`"online"`),
Old: jsonNull,
},
},
},
lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{
{}, {},
}}},
errors: []shared.FieldError{
{Source: "/changes/1/key", Detail: "index out of range"},
{Source: "/changes", Detail: "missing /attorneys/0/signedAt"},
},
},
"multiple attorneys - single attorney change": {
update: shared.Update{
Type: "ATTORNEY_SIGN",
Changes: []shared.Change{
{
Key: "/attorneys/1/mobile",
New: json.RawMessage(`"07777"`),
Old: json.RawMessage(`"06666"`),
},
{
Key: "/attorneys/1/signedAt",
New: json.RawMessage(`"` + now.Format(time.RFC3339Nano) + `"`),
Old: json.RawMessage(`"` + yesterday.Format(time.RFC3339Nano) + `"`),
},
{
Key: "/attorneys/1/contactLanguagePreference",
New: json.RawMessage(`"cy"`),
Old: jsonNull,
},
{
Key: "/attorneys/1/channel",
New: json.RawMessage(`"online"`),
Old: json.RawMessage(`"paper"`),
},
{
Key: "/attorneys/1/email",
New: json.RawMessage(`"[email protected]"`),
Old: json.RawMessage(`"[email protected]"`),
},
},
},
lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{
{}, {Channel: shared.ChannelPaper, Email: "[email protected]", Mobile: "06666", SignedAt: &yesterday},
}}},
},
}

for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
_, errors := validateUpdate(tc.update, &shared.Lpa{})
_, errors := validateUpdate(tc.update, tc.lpa)
assert.ElementsMatch(t, tc.errors, errors)
})
}
Expand Down
2 changes: 1 addition & 1 deletion lambda/update/trust_corporation_sign.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (a TrustCorporationSign) Apply(lpa *shared.Lpa) []shared.FieldError {
return nil
}

func validateTrustCorporationSign(changes []shared.Change) (TrustCorporationSign, []shared.FieldError) {
func validateTrustCorporationSign(changes []shared.Change, lpa *shared.Lpa) (TrustCorporationSign, []shared.FieldError) {
var data TrustCorporationSign

errors := parse.Changes(changes).
Expand Down
4 changes: 2 additions & 2 deletions lambda/update/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ func validateUpdate(update shared.Update, lpa *shared.Lpa) (Applyable, []shared.
case "CERTIFICATE_PROVIDER_SIGN":
return validateCertificateProviderSign(update.Changes, lpa)
case "ATTORNEY_SIGN":
return validateAttorneySign(update.Changes)
return validateAttorneySign(update.Changes, lpa)
case "TRUST_CORPORATION_SIGN":
return validateTrustCorporationSign(update.Changes)
return validateTrustCorporationSign(update.Changes, lpa)
case "PERFECT":
return validatePerfect(update.Changes)
case "REGISTER":
Expand Down