From dfdb6a847a99a3d400111ce699886e9b99aeea75 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Thu, 25 Apr 2024 17:32:08 +0100 Subject: [PATCH 1/4] enforce matching on existing and remove optional flags from ATTORNEY_SIGN email and channel --- lambda/update/attorney_sign.go | 23 +++++-- lambda/update/attorney_sign_test.go | 74 +++++++++++++++------- lambda/update/certificate_provider_sign.go | 2 +- lambda/update/parse/changes.go | 9 +-- lambda/update/parse/changes_test.go | 50 +++++++-------- lambda/update/trust_corporation_sign.go | 4 +- lambda/update/validate.go | 4 +- 7 files changed, 101 insertions(+), 65 deletions(-) diff --git a/lambda/update/attorney_sign.go b/lambda/update/attorney_sign.go index 84285ead..d7b115d2 100644 --- a/lambda/update/attorney_sign.go +++ b/lambda/update/attorney_sign.go @@ -31,10 +31,10 @@ 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). + errors := parse.Changes(changes, lpa). Prefix("/attorneys", func(p *parse.Parser) []shared.FieldError { return p. Each(func(i int, p *parse.Parser) []shared.FieldError { @@ -43,18 +43,27 @@ func validateAttorneySign(changes []shared.Change) (AttorneySign, []shared.Field } data.Index = &i + data.Mobile = p.Lpa.Attorneys[i].Mobile + data.ContactLanguagePreference = p.Lpa.Attorneys[i].ContactLanguagePreference + data.Channel = p.Lpa.Attorneys[i].Channel + data.Email = p.Lpa.Attorneys[i].Email + + if p.Lpa.Attorneys[i].SignedAt != nil { + data.SignedAt = *p.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()). + }), parse.MustMatchExisting()). + Field("/email", &data.Email, parse.MustMatchExisting()). Consumed() }). Consumed() diff --git a/lambda/update/attorney_sign_test.go b/lambda/update/attorney_sign_test.go index 27820786..69e4fb39 100644 --- a/lambda/update/attorney_sign_test.go +++ b/lambda/update/attorney_sign_test.go @@ -50,6 +50,7 @@ func TestValidateUpdateAttorneySign(t *testing.T) { testcases := map[string]struct { update shared.Update + lpa *shared.Lpa errors []shared.FieldError }{ "valid - no previous values": { @@ -57,64 +58,70 @@ 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", - 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", + Key: "/attorneys/0/email", New: json.RawMessage(`"a@example.com"`), 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(`"b@example.com"`), Old: json.RawMessage(`"a@example.com"`), }, }, }, + lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{ + {Channel: shared.ChannelPaper, Email: "a@example.com", Mobile: "06666", SignedAt: &yesterday}, + }}}, }, "missing all": { update: shared.Update{Type: "ATTORNEY_SIGN"}, @@ -127,17 +134,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, }, @@ -147,17 +154,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(`"a@example.com"`), 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"}, @@ -168,32 +183,35 @@ 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(`"b@example.com"`), 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"}, @@ -223,8 +241,16 @@ func TestValidateUpdateAttorneySign(t *testing.T) { New: json.RawMessage(`"a@example.com"`), 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"}, @@ -234,7 +260,7 @@ func TestValidateUpdateAttorneySign(t *testing.T) { 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) }) } diff --git a/lambda/update/certificate_provider_sign.go b/lambda/update/certificate_provider_sign.go index f163c9ef..35b3270f 100644 --- a/lambda/update/certificate_provider_sign.go +++ b/lambda/update/certificate_provider_sign.go @@ -42,7 +42,7 @@ func validateCertificateProviderSign(changes []shared.Change, lpa *shared.Lpa) ( data.SignedAt = *lpa.LpaInit.CertificateProvider.SignedAt } - errors := parse.Changes(changes). + errors := parse.Changes(changes, lpa). Prefix("/certificateProvider/address", func(p *parse.Parser) []shared.FieldError { return p. Field("/line1", &data.Address.Line1, parse.MustMatchExisting()). diff --git a/lambda/update/parse/changes.go b/lambda/update/parse/changes.go index fed78a81..4a59e7ee 100644 --- a/lambda/update/parse/changes.go +++ b/lambda/update/parse/changes.go @@ -24,16 +24,17 @@ type Parser struct { root string changes []changeWithPosition errors []shared.FieldError + Lpa *shared.Lpa } // Changes constructs a new [Parser] for a set of changes. -func Changes(changes []shared.Change) *Parser { +func Changes(changes []shared.Change, lpa *shared.Lpa) *Parser { cs := make([]changeWithPosition, len(changes)) for i, change := range changes { cs[i] = changeWithPosition{Change: change, pos: i} } - return &Parser{changes: cs} + return &Parser{changes: cs, Lpa: lpa} } // Consumed checks the [Parser] has used all of the changes. It adds an error for any unparsed changes. @@ -236,7 +237,7 @@ func (p *Parser) Each(fn func(int, *Parser) []shared.FieldError, required ...int for _, idx := range indexes { changes := indexedChanges[idx] - subParser := &Parser{root: p.root + "/" + strconv.Itoa(idx), changes: changes} + subParser := &Parser{root: p.root + "/" + strconv.Itoa(idx), changes: changes, Lpa: p.Lpa} fn(idx, subParser) p.errors = append(p.errors, subParser.errors...) } @@ -286,7 +287,7 @@ func (p *Parser) Prefix(prefix string, fn func(*Parser) []shared.FieldError, opt p.errors = append(p.errors, shared.FieldError{Source: "/changes", Detail: "missing " + p.root + prefix + "/..."}) } } else { - subParser := &Parser{root: p.root + prefix, changes: matching} + subParser := &Parser{root: p.root + prefix, changes: matching, Lpa: p.Lpa} fn(subParser) p.errors = append(p.errors, subParser.errors...) } diff --git a/lambda/update/parse/changes_test.go b/lambda/update/parse/changes_test.go index d38273fd..ee3d2487 100644 --- a/lambda/update/parse/changes_test.go +++ b/lambda/update/parse/changes_test.go @@ -17,7 +17,7 @@ func TestField(t *testing.T) { } var v string - Changes(changes).Field("/thing", &v) + Changes(changes, &shared.Lpa{}).Field("/thing", &v) assert.Equal(t, "val", v) } @@ -26,7 +26,7 @@ func TestFieldWhenMissing(t *testing.T) { changes := []shared.Change{} var v string - errors := Changes(changes).Field("/thing", &v).Errors() + errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v).Errors() assert.Equal(t, []shared.FieldError{{Source: "/changes", Detail: "missing /thing"}}, errors) } @@ -37,7 +37,7 @@ func TestFieldWhenWrongType(t *testing.T) { } var v int - errors := Changes(changes).Field("/thing", &v).Errors() + errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v).Errors() assert.Equal(t, []shared.FieldError{{Source: "/changes/0/new", Detail: "unexpected type"}}, errors) } @@ -48,7 +48,7 @@ func TestFieldOptional(t *testing.T) { } var v string - Changes(changes).Field("/thing", &v, Optional()) + Changes(changes, &shared.Lpa{}).Field("/thing", &v, Optional()) assert.Equal(t, "val", v) } @@ -57,7 +57,7 @@ func TestFieldOptionalWhenMissing(t *testing.T) { changes := []shared.Change{} var v string - errors := Changes(changes).Field("/thing", &v, Optional()).Errors() + errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v, Optional()).Errors() assert.Empty(t, errors) } @@ -68,7 +68,7 @@ func TestFieldValidate(t *testing.T) { } var v string - Changes(changes).Field("/thing", &v, Validate(func() []shared.FieldError { + Changes(changes, &shared.Lpa{}).Field("/thing", &v, Validate(func() []shared.FieldError { return nil })) @@ -81,7 +81,7 @@ func TestFieldValidateWhenInvalid(t *testing.T) { } v := "why" - errors := Changes(changes).Field("/thing", &v, Validate(func() []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v, Validate(func() []shared.FieldError { return []shared.FieldError{{Source: "/rewritten", Detail: "invalid"}} })).Errors() @@ -94,7 +94,7 @@ func TestFieldMustMatchExistingString(t *testing.T) { } v := "old val" - Changes(changes).Field("/thing", &v, MustMatchExisting()) + Changes(changes, &shared.Lpa{}).Field("/thing", &v, MustMatchExisting()) assert.Equal(t, "new val", v) } @@ -107,7 +107,7 @@ func TestFieldMustMatchExistingTime(t *testing.T) { {Key: "/thing", New: json.RawMessage(`"` + now.Format(time.RFC3339Nano) + `"`), Old: json.RawMessage(`"` + yesterday.Format(time.RFC3339Nano) + `"`)}, } - Changes(changes).Field("/thing", &yesterday, MustMatchExisting()) + Changes(changes, &shared.Lpa{}).Field("/thing", &yesterday, MustMatchExisting()) assert.WithinDuration(t, now, yesterday, time.Second) } @@ -118,7 +118,7 @@ func TestFieldMustMatchExistingLang(t *testing.T) { } v := shared.LangEn - Changes(changes).Field("/thing", &v, MustMatchExisting()) + Changes(changes, &shared.Lpa{}).Field("/thing", &v, MustMatchExisting()) assert.Equal(t, shared.LangCy, v) } @@ -129,7 +129,7 @@ func TestFieldMustMatchExistingChannel(t *testing.T) { } v := shared.ChannelPaper - Changes(changes).Field("/thing", &v, MustMatchExisting()) + Changes(changes, &shared.Lpa{}).Field("/thing", &v, MustMatchExisting()) assert.Equal(t, shared.ChannelOnline, v) } @@ -147,7 +147,7 @@ func TestFieldWhenOldDoesNotMatchExisting(t *testing.T) { } v := "existing" - errors := Changes(changes).Field("/thing", &v, MustMatchExisting()).Errors() + errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v, MustMatchExisting()).Errors() assert.Equal(t, []shared.FieldError{{Source: "/changes/0/old", Detail: "does not match existing value"}}, errors) }) @@ -156,7 +156,7 @@ func TestFieldWhenOldDoesNotMatchExisting(t *testing.T) { func TestConsumed(t *testing.T) { changes := []shared.Change{} - errors := Changes(changes).Consumed() + errors := Changes(changes, &shared.Lpa{}).Consumed() assert.Empty(t, errors) } @@ -166,7 +166,7 @@ func TestConsumedWhenNot(t *testing.T) { {Key: "/thing", New: json.RawMessage(`"val"`), Old: jsonNull}, } - errors := Changes(changes).Consumed() + errors := Changes(changes, &shared.Lpa{}).Consumed() assert.Equal(t, []shared.FieldError{{Source: "/changes/0", Detail: "unexpected change provided"}}, errors) } @@ -177,7 +177,7 @@ func TestConsumedWhenConsumed(t *testing.T) { } var v string - errors := Changes(changes).Field("/thing", &v).Consumed() + errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v).Consumed() assert.Empty(t, errors) } @@ -189,7 +189,7 @@ func TestEach(t *testing.T) { } var v, w string - errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { if i == 0 { p.Field("/thing", &v) } else if i == 1 { @@ -209,7 +209,7 @@ func TestEachWhenNonIndexedKey(t *testing.T) { {Key: "/-/other", New: json.RawMessage(`"other"`), Old: jsonNull}, } - errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { var v any p.Field("/thing", v) return p.Errors() @@ -224,7 +224,7 @@ func TestEachWhenNonIndexedKey(t *testing.T) { func TestEachWhenRequired(t *testing.T) { changes := []shared.Change{} - errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { var v any p.Field("/thing", v) return p.Errors() @@ -242,7 +242,7 @@ func TestEachWhenOutOfRange(t *testing.T) { {Key: "/2/thing", New: json.RawMessage(`"val"`), Old: jsonNull}, } - errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { if i > 0 { return p.OutOfRange() } @@ -263,7 +263,7 @@ func TestEachWhenNotConsumed(t *testing.T) { {Key: "/2/thing", New: json.RawMessage(`"val"`), Old: jsonNull}, } - errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { if i == 0 { var v string p.Field("/thing", &v) @@ -284,7 +284,7 @@ func TestPrefix(t *testing.T) { } var v string - errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { return p. Field("/thing", &v). Consumed() @@ -302,7 +302,7 @@ func TestPrefixWhenNotConsumed(t *testing.T) { } var v string - errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { return p. Field("/thing", &v). Consumed() @@ -318,7 +318,7 @@ func TestPrefixWhenNotConsumed(t *testing.T) { func TestPrefixWhenMissing(t *testing.T) { changes := []shared.Change{} - errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { return nil }).Consumed() + errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { return nil }).Consumed() assert.ElementsMatch(t, []shared.FieldError{ {Source: "/changes", Detail: "missing /a/..."}, @@ -331,7 +331,7 @@ func TestPrefixOptional(t *testing.T) { } var v string - errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { + errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { return p. Field("/thing", &v). Consumed() @@ -343,7 +343,7 @@ func TestPrefixOptional(t *testing.T) { func TestOptionalPrefixWhenMissing(t *testing.T) { changes := []shared.Change{} - errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { return nil }, Optional()).Consumed() + errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { return nil }, Optional()).Consumed() assert.Empty(t, errors) } diff --git a/lambda/update/trust_corporation_sign.go b/lambda/update/trust_corporation_sign.go index 6c1ccd8b..96a7503e 100644 --- a/lambda/update/trust_corporation_sign.go +++ b/lambda/update/trust_corporation_sign.go @@ -29,10 +29,10 @@ 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). + errors := parse.Changes(changes, lpa). Prefix("/trustCorporations", func(prefix *parse.Parser) []shared.FieldError { return prefix. Each(func(i int, each *parse.Parser) []shared.FieldError { diff --git a/lambda/update/validate.go b/lambda/update/validate.go index 00578348..bdd998ab 100644 --- a/lambda/update/validate.go +++ b/lambda/update/validate.go @@ -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": From 4720a7de07abb1fc877279e752112a84b899da37 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Thu, 25 Apr 2024 17:39:14 +0100 Subject: [PATCH 2/4] cover multiple attorneys with single changeset --- lambda/update/attorney_sign_test.go | 37 ++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/lambda/update/attorney_sign_test.go b/lambda/update/attorney_sign_test.go index 69e4fb39..20063773 100644 --- a/lambda/update/attorney_sign_test.go +++ b/lambda/update/attorney_sign_test.go @@ -217,7 +217,7 @@ func TestValidateUpdateAttorneySign(t *testing.T) { {Source: "/changes/3/new", Detail: "invalid value"}, }, }, - "multiple attorneys": { + "multiple attorneys - multiple attorney changes": { update: shared.Update{ Type: "ATTORNEY_SIGN", Changes: []shared.Change{ @@ -256,6 +256,41 @@ func TestValidateUpdateAttorneySign(t *testing.T) { {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(`"b@example.com"`), + Old: json.RawMessage(`"a@example.com"`), + }, + }, + }, + lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{ + {}, {Channel: shared.ChannelPaper, Email: "a@example.com", Mobile: "06666", SignedAt: &yesterday}, + }}}, + }, } for name, tc := range testcases { From 17a6449c8060f9710f39f02462f658a90150f234 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Mon, 29 Apr 2024 13:41:14 +0100 Subject: [PATCH 3/4] stop passing lpa via Changes --- lambda/update/attorney_sign.go | 14 +++--- lambda/update/certificate_provider_sign.go | 2 +- lambda/update/parse/changes.go | 9 ++-- lambda/update/parse/changes_test.go | 50 +++++++++++----------- lambda/update/trust_corporation_sign.go | 2 +- 5 files changed, 38 insertions(+), 39 deletions(-) diff --git a/lambda/update/attorney_sign.go b/lambda/update/attorney_sign.go index d7b115d2..db29fb91 100644 --- a/lambda/update/attorney_sign.go +++ b/lambda/update/attorney_sign.go @@ -34,7 +34,7 @@ func (a AttorneySign) Apply(lpa *shared.Lpa) []shared.FieldError { func validateAttorneySign(changes []shared.Change, lpa *shared.Lpa) (AttorneySign, []shared.FieldError) { var data AttorneySign - errors := parse.Changes(changes, lpa). + errors := parse.Changes(changes). Prefix("/attorneys", func(p *parse.Parser) []shared.FieldError { return p. Each(func(i int, p *parse.Parser) []shared.FieldError { @@ -43,13 +43,13 @@ func validateAttorneySign(changes []shared.Change, lpa *shared.Lpa) (AttorneySig } data.Index = &i - data.Mobile = p.Lpa.Attorneys[i].Mobile - data.ContactLanguagePreference = p.Lpa.Attorneys[i].ContactLanguagePreference - data.Channel = p.Lpa.Attorneys[i].Channel - data.Email = p.Lpa.Attorneys[i].Email + 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 p.Lpa.Attorneys[i].SignedAt != nil { - data.SignedAt = *p.Lpa.Attorneys[i].SignedAt + if lpa.Attorneys[i].SignedAt != nil { + data.SignedAt = *lpa.Attorneys[i].SignedAt } return p. diff --git a/lambda/update/certificate_provider_sign.go b/lambda/update/certificate_provider_sign.go index 35b3270f..f163c9ef 100644 --- a/lambda/update/certificate_provider_sign.go +++ b/lambda/update/certificate_provider_sign.go @@ -42,7 +42,7 @@ func validateCertificateProviderSign(changes []shared.Change, lpa *shared.Lpa) ( data.SignedAt = *lpa.LpaInit.CertificateProvider.SignedAt } - errors := parse.Changes(changes, lpa). + errors := parse.Changes(changes). Prefix("/certificateProvider/address", func(p *parse.Parser) []shared.FieldError { return p. Field("/line1", &data.Address.Line1, parse.MustMatchExisting()). diff --git a/lambda/update/parse/changes.go b/lambda/update/parse/changes.go index 4a59e7ee..fed78a81 100644 --- a/lambda/update/parse/changes.go +++ b/lambda/update/parse/changes.go @@ -24,17 +24,16 @@ type Parser struct { root string changes []changeWithPosition errors []shared.FieldError - Lpa *shared.Lpa } // Changes constructs a new [Parser] for a set of changes. -func Changes(changes []shared.Change, lpa *shared.Lpa) *Parser { +func Changes(changes []shared.Change) *Parser { cs := make([]changeWithPosition, len(changes)) for i, change := range changes { cs[i] = changeWithPosition{Change: change, pos: i} } - return &Parser{changes: cs, Lpa: lpa} + return &Parser{changes: cs} } // Consumed checks the [Parser] has used all of the changes. It adds an error for any unparsed changes. @@ -237,7 +236,7 @@ func (p *Parser) Each(fn func(int, *Parser) []shared.FieldError, required ...int for _, idx := range indexes { changes := indexedChanges[idx] - subParser := &Parser{root: p.root + "/" + strconv.Itoa(idx), changes: changes, Lpa: p.Lpa} + subParser := &Parser{root: p.root + "/" + strconv.Itoa(idx), changes: changes} fn(idx, subParser) p.errors = append(p.errors, subParser.errors...) } @@ -287,7 +286,7 @@ func (p *Parser) Prefix(prefix string, fn func(*Parser) []shared.FieldError, opt p.errors = append(p.errors, shared.FieldError{Source: "/changes", Detail: "missing " + p.root + prefix + "/..."}) } } else { - subParser := &Parser{root: p.root + prefix, changes: matching, Lpa: p.Lpa} + subParser := &Parser{root: p.root + prefix, changes: matching} fn(subParser) p.errors = append(p.errors, subParser.errors...) } diff --git a/lambda/update/parse/changes_test.go b/lambda/update/parse/changes_test.go index ee3d2487..d38273fd 100644 --- a/lambda/update/parse/changes_test.go +++ b/lambda/update/parse/changes_test.go @@ -17,7 +17,7 @@ func TestField(t *testing.T) { } var v string - Changes(changes, &shared.Lpa{}).Field("/thing", &v) + Changes(changes).Field("/thing", &v) assert.Equal(t, "val", v) } @@ -26,7 +26,7 @@ func TestFieldWhenMissing(t *testing.T) { changes := []shared.Change{} var v string - errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v).Errors() + errors := Changes(changes).Field("/thing", &v).Errors() assert.Equal(t, []shared.FieldError{{Source: "/changes", Detail: "missing /thing"}}, errors) } @@ -37,7 +37,7 @@ func TestFieldWhenWrongType(t *testing.T) { } var v int - errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v).Errors() + errors := Changes(changes).Field("/thing", &v).Errors() assert.Equal(t, []shared.FieldError{{Source: "/changes/0/new", Detail: "unexpected type"}}, errors) } @@ -48,7 +48,7 @@ func TestFieldOptional(t *testing.T) { } var v string - Changes(changes, &shared.Lpa{}).Field("/thing", &v, Optional()) + Changes(changes).Field("/thing", &v, Optional()) assert.Equal(t, "val", v) } @@ -57,7 +57,7 @@ func TestFieldOptionalWhenMissing(t *testing.T) { changes := []shared.Change{} var v string - errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v, Optional()).Errors() + errors := Changes(changes).Field("/thing", &v, Optional()).Errors() assert.Empty(t, errors) } @@ -68,7 +68,7 @@ func TestFieldValidate(t *testing.T) { } var v string - Changes(changes, &shared.Lpa{}).Field("/thing", &v, Validate(func() []shared.FieldError { + Changes(changes).Field("/thing", &v, Validate(func() []shared.FieldError { return nil })) @@ -81,7 +81,7 @@ func TestFieldValidateWhenInvalid(t *testing.T) { } v := "why" - errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v, Validate(func() []shared.FieldError { + errors := Changes(changes).Field("/thing", &v, Validate(func() []shared.FieldError { return []shared.FieldError{{Source: "/rewritten", Detail: "invalid"}} })).Errors() @@ -94,7 +94,7 @@ func TestFieldMustMatchExistingString(t *testing.T) { } v := "old val" - Changes(changes, &shared.Lpa{}).Field("/thing", &v, MustMatchExisting()) + Changes(changes).Field("/thing", &v, MustMatchExisting()) assert.Equal(t, "new val", v) } @@ -107,7 +107,7 @@ func TestFieldMustMatchExistingTime(t *testing.T) { {Key: "/thing", New: json.RawMessage(`"` + now.Format(time.RFC3339Nano) + `"`), Old: json.RawMessage(`"` + yesterday.Format(time.RFC3339Nano) + `"`)}, } - Changes(changes, &shared.Lpa{}).Field("/thing", &yesterday, MustMatchExisting()) + Changes(changes).Field("/thing", &yesterday, MustMatchExisting()) assert.WithinDuration(t, now, yesterday, time.Second) } @@ -118,7 +118,7 @@ func TestFieldMustMatchExistingLang(t *testing.T) { } v := shared.LangEn - Changes(changes, &shared.Lpa{}).Field("/thing", &v, MustMatchExisting()) + Changes(changes).Field("/thing", &v, MustMatchExisting()) assert.Equal(t, shared.LangCy, v) } @@ -129,7 +129,7 @@ func TestFieldMustMatchExistingChannel(t *testing.T) { } v := shared.ChannelPaper - Changes(changes, &shared.Lpa{}).Field("/thing", &v, MustMatchExisting()) + Changes(changes).Field("/thing", &v, MustMatchExisting()) assert.Equal(t, shared.ChannelOnline, v) } @@ -147,7 +147,7 @@ func TestFieldWhenOldDoesNotMatchExisting(t *testing.T) { } v := "existing" - errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v, MustMatchExisting()).Errors() + errors := Changes(changes).Field("/thing", &v, MustMatchExisting()).Errors() assert.Equal(t, []shared.FieldError{{Source: "/changes/0/old", Detail: "does not match existing value"}}, errors) }) @@ -156,7 +156,7 @@ func TestFieldWhenOldDoesNotMatchExisting(t *testing.T) { func TestConsumed(t *testing.T) { changes := []shared.Change{} - errors := Changes(changes, &shared.Lpa{}).Consumed() + errors := Changes(changes).Consumed() assert.Empty(t, errors) } @@ -166,7 +166,7 @@ func TestConsumedWhenNot(t *testing.T) { {Key: "/thing", New: json.RawMessage(`"val"`), Old: jsonNull}, } - errors := Changes(changes, &shared.Lpa{}).Consumed() + errors := Changes(changes).Consumed() assert.Equal(t, []shared.FieldError{{Source: "/changes/0", Detail: "unexpected change provided"}}, errors) } @@ -177,7 +177,7 @@ func TestConsumedWhenConsumed(t *testing.T) { } var v string - errors := Changes(changes, &shared.Lpa{}).Field("/thing", &v).Consumed() + errors := Changes(changes).Field("/thing", &v).Consumed() assert.Empty(t, errors) } @@ -189,7 +189,7 @@ func TestEach(t *testing.T) { } var v, w string - errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { if i == 0 { p.Field("/thing", &v) } else if i == 1 { @@ -209,7 +209,7 @@ func TestEachWhenNonIndexedKey(t *testing.T) { {Key: "/-/other", New: json.RawMessage(`"other"`), Old: jsonNull}, } - errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { var v any p.Field("/thing", v) return p.Errors() @@ -224,7 +224,7 @@ func TestEachWhenNonIndexedKey(t *testing.T) { func TestEachWhenRequired(t *testing.T) { changes := []shared.Change{} - errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { var v any p.Field("/thing", v) return p.Errors() @@ -242,7 +242,7 @@ func TestEachWhenOutOfRange(t *testing.T) { {Key: "/2/thing", New: json.RawMessage(`"val"`), Old: jsonNull}, } - errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { if i > 0 { return p.OutOfRange() } @@ -263,7 +263,7 @@ func TestEachWhenNotConsumed(t *testing.T) { {Key: "/2/thing", New: json.RawMessage(`"val"`), Old: jsonNull}, } - errors := Changes(changes, &shared.Lpa{}).Each(func(i int, p *Parser) []shared.FieldError { + errors := Changes(changes).Each(func(i int, p *Parser) []shared.FieldError { if i == 0 { var v string p.Field("/thing", &v) @@ -284,7 +284,7 @@ func TestPrefix(t *testing.T) { } var v string - errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { + errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { return p. Field("/thing", &v). Consumed() @@ -302,7 +302,7 @@ func TestPrefixWhenNotConsumed(t *testing.T) { } var v string - errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { + errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { return p. Field("/thing", &v). Consumed() @@ -318,7 +318,7 @@ func TestPrefixWhenNotConsumed(t *testing.T) { func TestPrefixWhenMissing(t *testing.T) { changes := []shared.Change{} - errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { return nil }).Consumed() + errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { return nil }).Consumed() assert.ElementsMatch(t, []shared.FieldError{ {Source: "/changes", Detail: "missing /a/..."}, @@ -331,7 +331,7 @@ func TestPrefixOptional(t *testing.T) { } var v string - errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { + errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { return p. Field("/thing", &v). Consumed() @@ -343,7 +343,7 @@ func TestPrefixOptional(t *testing.T) { func TestOptionalPrefixWhenMissing(t *testing.T) { changes := []shared.Change{} - errors := Changes(changes, &shared.Lpa{}).Prefix("/a", func(p *Parser) []shared.FieldError { return nil }, Optional()).Consumed() + errors := Changes(changes).Prefix("/a", func(p *Parser) []shared.FieldError { return nil }, Optional()).Consumed() assert.Empty(t, errors) } diff --git a/lambda/update/trust_corporation_sign.go b/lambda/update/trust_corporation_sign.go index 96a7503e..d314de0f 100644 --- a/lambda/update/trust_corporation_sign.go +++ b/lambda/update/trust_corporation_sign.go @@ -32,7 +32,7 @@ func (a TrustCorporationSign) Apply(lpa *shared.Lpa) []shared.FieldError { func validateTrustCorporationSign(changes []shared.Change, lpa *shared.Lpa) (TrustCorporationSign, []shared.FieldError) { var data TrustCorporationSign - errors := parse.Changes(changes, lpa). + errors := parse.Changes(changes). Prefix("/trustCorporations", func(prefix *parse.Parser) []shared.FieldError { return prefix. Each(func(i int, each *parse.Parser) []shared.FieldError { From 3b2c54f1e53e8183a88c33b1b1b4712d58affc17 Mon Sep 17 00:00:00 2001 From: Alex Saunders Date: Mon, 29 Apr 2024 13:45:12 +0100 Subject: [PATCH 4/4] make mobile optional --- lambda/update/attorney_sign.go | 2 +- lambda/update/attorney_sign_test.go | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/lambda/update/attorney_sign.go b/lambda/update/attorney_sign.go index db29fb91..505fc58e 100644 --- a/lambda/update/attorney_sign.go +++ b/lambda/update/attorney_sign.go @@ -63,7 +63,7 @@ func validateAttorneySign(changes []shared.Change, lpa *shared.Lpa) (AttorneySig Field("/channel", &data.Channel, parse.Validate(func() []shared.FieldError { return validate.IsValid("", data.Channel) }), parse.MustMatchExisting()). - Field("/email", &data.Email, parse.MustMatchExisting()). + Field("/email", &data.Email, parse.MustMatchExisting(), parse.Optional()). Consumed() }). Consumed() diff --git a/lambda/update/attorney_sign_test.go b/lambda/update/attorney_sign_test.go index 20063773..da63e1fa 100644 --- a/lambda/update/attorney_sign_test.go +++ b/lambda/update/attorney_sign_test.go @@ -77,11 +77,6 @@ func TestValidateUpdateAttorneySign(t *testing.T) { New: json.RawMessage(`"online"`), Old: jsonNull, }, - { - Key: "/attorneys/0/email", - New: json.RawMessage(`"a@example.com"`), - Old: jsonNull, - }, }, }, lpa: &shared.Lpa{LpaInit: shared.LpaInit{Attorneys: []shared.Attorney{