Skip to content

Commit

Permalink
Merge 6c58b1e into bb5ee08
Browse files Browse the repository at this point in the history
  • Loading branch information
acsauk authored Nov 23, 2023
2 parents bb5ee08 + 6c58b1e commit 09dd7dd
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 53 deletions.
14 changes: 14 additions & 0 deletions docs/runbooks/changes_to_existing_notify_templates.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Changes to existing GOV.UK Notify SMS and email templates

Email and SMS notifications are sent via [GOV.UK Notify](https://www.notifications.service.gov.uk). If a change to an existing template involves removing or adding a personalisation variable we should take a staged approach to making the change to avoid missing personalisation error responses from Notify in production or ephemeral environments.

- Copy the existing template in Notify:
- `Templates -> New Template -> Copy an existing template`
- Make required changes, move the template to a suitable folder and copy the new template ID
- Update the [template ID](../../internal/notify/client.go) in code as part of the PR
- Merge the PR
- Remove the old template from Notify

If there is an existing template in the OPG Test Notify account that needs to be duplicated you can make a copy of it directly in to the Office of the Public Guardian account by switching to the Office of the Public Guardian account and then following the steps above but selecting the template from OPG Test when choosing `Copy an existing template`.

This process should be followed in both OPG Test and Office of the Public Guardian Notify accounts.
1 change: 0 additions & 1 deletion internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ func App(
attorneyStore,
notifyClient,
evidenceReceivedStore,
s3Client,
documentStore,
eventClient,
)
Expand Down
4 changes: 2 additions & 2 deletions internal/notify/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var (
CertificateProviderInviteEmail: "a10341e3-3bbd-4452-b52f-ebb4f51a4d73",
CertificateProviderNameChangeEmail: "9f8be86f-864a-4cda-a58a-5768522bd325",
CertificateProviderPaperLpaDetailsChangedSMS: "d363a56f-e802-4f88-bd09-80b8c9e9d650",
CertificateProviderPaperMeetingPromptSMS: "6be11b4a-79f9-441e-8afe-adff96f7e7fc",
CertificateProviderPaperMeetingPromptSMS: "b5cd2c1b-e9b4-4f3e-8cf1-504aff93b16d",
CertificateProviderReturnEmail: "453917cd-d8bb-44af-90a1-d73ae0f3fd07",
ReplacementAttorneyInviteEmail: "1c4d5b24-fc7d-45ee-be40-f1ccda96f101",
ReplacementTrustCorporationInviteEmail: "1c4d5b24-fc7d-45ee-be40-f1ccda96f101",
Expand All @@ -60,7 +60,7 @@ var (
CertificateProviderInviteEmail: "dd864a1a-64b4-4b4e-b810-86267ebd6476",
CertificateProviderNameChangeEmail: "0f111ed1-5c58-47eb-a13f-931f2077523b",
CertificateProviderPaperLpaDetailsChangedSMS: "94477364-281a-4032-9a88-b215f969cd12",
CertificateProviderPaperMeetingPromptSMS: "0eba4e55-c07e-4427-b4ad-b03e08dad8ca",
CertificateProviderPaperMeetingPromptSMS: "ee39cd81-5802-44bb-b967-27da7e25e897",
CertificateProviderReturnEmail: "dd864a1a-64b4-4b4e-b810-86267ebd6476",
ReplacementAttorneyInviteEmail: "bf79859b-72b7-4701-bfd3-22ac6f0908c8",
ReplacementTrustCorporationInviteEmail: "bf79859b-72b7-4701-bfd3-22ac6f0908c8",
Expand Down
1 change: 1 addition & 0 deletions internal/page/app_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type AppData struct {
ActorType actor.Type
AttorneyID string
OneloginURL string
AppPublicURL string
}

func (d AppData) Redirect(w http.ResponseWriter, r *http.Request, url string) error {
Expand Down
1 change: 1 addition & 0 deletions internal/page/donor/check_your_lpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func (n *checkYourLpaNotifier) sendPaperNotification(ctx context.Context, appDat
} else {
sms.TemplateID = n.notifyClient.TemplateID(notify.CertificateProviderPaperMeetingPromptSMS)
sms.Personalisation["lpaType"] = appData.Localizer.T(donor.Type.LegalTermTransKey())
sms.Personalisation["CPLandingPageLink"] = appData.AppPublicURL + appData.Lang.URL(page.Paths.CertificateProviderStart.Format())
}

_, err := n.notifyClient.Sms(ctx, sms)
Expand Down
7 changes: 4 additions & 3 deletions internal/page/donor/check_your_lpa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,10 @@ func TestPostCheckYourLpaPaperCertificateProviderOnFirstCheck(t *testing.T) {
PhoneNumber: "07700900000",
TemplateID: "template-id",
Personalisation: map[string]string{
"donorFullName": "Teneil Throssell",
"lpaType": "property and affairs",
"donorFirstNames": "Teneil",
"donorFullName": "Teneil Throssell",
"lpaType": "property and affairs",
"donorFirstNames": "Teneil",
"CPLandingPageLink": "http://example.org/certificate-provider-start",
},
}).
Return("", nil)
Expand Down
9 changes: 5 additions & 4 deletions internal/page/donor/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ var (
}
expectedError = errors.New("err")
testAppData = page.AppData{
SessionID: "session-id",
LpaID: "lpa-id",
Lang: localize.En,
Paths: page.Paths,
SessionID: "session-id",
LpaID: "lpa-id",
Lang: localize.En,
Paths: page.Paths,
AppPublicURL: "http://example.org",
}
testNow = time.Date(2023, time.July, 3, 4, 5, 6, 1, time.UTC)
testNowFn = func() time.Time { return testNow }
Expand Down
17 changes: 8 additions & 9 deletions internal/page/donor/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ func Register(
attorneyStore AttorneyStore,
notifyClient NotifyClient,
evidenceReceivedStore EvidenceReceivedStore,
evidenceS3Client S3Client,
documentStore DocumentStore,
eventClient EventClient,
) {
Expand All @@ -184,11 +183,10 @@ func Register(
sessionStore: sessionStore,
donorStore: donorStore,
payClient: payClient,
appPublicURL: appPublicURL,
randomString: random.String,
}

handleRoot := makeHandle(rootMux, sessionStore, None, errorHandler)
handleRoot := makeHandle(rootMux, sessionStore, None, errorHandler, appPublicURL)

handleRoot(page.Paths.Login, None,
page.Login(logger, oneLoginClient, sessionStore, random.String, page.Paths.LoginCallback))
Expand All @@ -198,8 +196,8 @@ func Register(
lpaMux := http.NewServeMux()
rootMux.Handle("/lpa/", page.RouteToPrefix("/lpa/", lpaMux, notFoundHandler))

handleDonor := makeHandle(lpaMux, sessionStore, RequireSession, errorHandler)
handleWithDonor := makeLpaHandle(lpaMux, sessionStore, RequireSession, errorHandler, donorStore)
handleDonor := makeHandle(lpaMux, sessionStore, RequireSession, errorHandler, appPublicURL)
handleWithDonor := makeLpaHandle(lpaMux, sessionStore, RequireSession, errorHandler, donorStore, appPublicURL)

handleDonor(page.Paths.Root, None, notFoundHandler)

Expand Down Expand Up @@ -391,7 +389,7 @@ const (
CanGoBack
)

func makeHandle(mux *http.ServeMux, store sesh.Store, defaultOptions handleOpt, errorHandler page.ErrorHandler) func(page.Path, handleOpt, page.Handler) {
func makeHandle(mux *http.ServeMux, store sesh.Store, defaultOptions handleOpt, errorHandler page.ErrorHandler, appPublicURL string) func(page.Path, handleOpt, page.Handler) {
return func(path page.Path, opt handleOpt, h page.Handler) {
opt = opt | defaultOptions

Expand All @@ -402,6 +400,7 @@ func makeHandle(mux *http.ServeMux, store sesh.Store, defaultOptions handleOpt,
appData.Page = path.Format()
appData.CanGoBack = opt&CanGoBack != 0
appData.ActorType = actor.TypeDonor
appData.AppPublicURL = appPublicURL

if opt&RequireSession != 0 {
donorSession, err := sesh.Login(store, r)
Expand Down Expand Up @@ -431,7 +430,7 @@ func makeHandle(mux *http.ServeMux, store sesh.Store, defaultOptions handleOpt,
}
}

func makeLpaHandle(mux *http.ServeMux, store sesh.Store, defaultOptions handleOpt, errorHandler page.ErrorHandler, donorStore DonorStore) func(page.LpaPath, handleOpt, Handler) {
func makeLpaHandle(mux *http.ServeMux, store sesh.Store, defaultOptions handleOpt, errorHandler page.ErrorHandler, donorStore DonorStore, appPublicURL string) func(page.LpaPath, handleOpt, Handler) {
return func(path page.LpaPath, opt handleOpt, h Handler) {

opt = opt | defaultOptions
Expand All @@ -442,6 +441,7 @@ func makeLpaHandle(mux *http.ServeMux, store sesh.Store, defaultOptions handleOp
appData := page.AppDataFromContext(ctx)
appData.CanGoBack = opt&CanGoBack != 0
appData.ActorType = actor.TypeDonor
appData.AppPublicURL = appPublicURL

donorSession, err := sesh.Login(store, r)
if err != nil {
Expand Down Expand Up @@ -482,7 +482,6 @@ type payHelper struct {
sessionStore sessions.Store
donorStore DonorStore
payClient PayClient
appPublicURL string
randomString func(int) string
}

Expand All @@ -504,7 +503,7 @@ func (p *payHelper) Pay(appData page.AppData, w http.ResponseWriter, r *http.Req
Amount: donor.FeeAmount(),
Reference: p.randomString(12),
Description: "Property and Finance LPA",
ReturnUrl: p.appPublicURL + appData.Lang.URL(page.Paths.PaymentConfirmation.Format(donor.LpaID)),
ReturnUrl: appData.AppPublicURL + appData.Lang.URL(page.Paths.PaymentConfirmation.Format(donor.LpaID)),
Email: donor.Donor.Email,
Language: appData.Lang.String(),
}
Expand Down
67 changes: 33 additions & 34 deletions internal/page/donor/register_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (

func TestRegister(t *testing.T) {
mux := http.NewServeMux()
Register(mux, &log.Logger{}, template.Templates{}, nil, nil, &onelogin.Client{}, &place.Client{}, "http://public.url", &pay.Client{}, nil, &mockWitnessCodeSender{}, nil, nil, nil, nil, &notify.Client{}, nil, nil, nil, nil)
Register(mux, &log.Logger{}, template.Templates{}, nil, nil, &onelogin.Client{}, &place.Client{}, "http://example.org", &pay.Client{}, nil, &mockWitnessCodeSender{}, nil, nil, nil, nil, &notify.Client{}, nil, nil, nil)

assert.Implements(t, (*http.Handler)(nil), mux)
}
Expand All @@ -38,13 +38,14 @@ func TestMakeHandle(t *testing.T) {
Return(&sessions.Session{Values: map[any]any{"session": &sesh.LoginSession{Sub: "random"}}}, nil)

mux := http.NewServeMux()
handle := makeHandle(mux, sessionStore, None, nil)
handle := makeHandle(mux, sessionStore, None, nil, "http://example.org")
handle("/path", RequireSession|CanGoBack, func(appData page.AppData, hw http.ResponseWriter, hr *http.Request) error {
assert.Equal(t, page.AppData{
Page: "/path",
CanGoBack: true,
SessionID: "cmFuZG9t",
ActorType: actor.TypeDonor,
Page: "/path",
CanGoBack: true,
SessionID: "cmFuZG9t",
ActorType: actor.TypeDonor,
AppPublicURL: "http://example.org",
}, appData)
assert.Equal(t, w, hw)

Expand Down Expand Up @@ -72,14 +73,15 @@ func TestMakeHandleExistingSessionData(t *testing.T) {
Return(&sessions.Session{Values: map[any]any{"session": &sesh.LoginSession{Sub: "random"}}}, nil)

mux := http.NewServeMux()
handle := makeHandle(mux, sessionStore, None, nil)
handle := makeHandle(mux, sessionStore, None, nil, "http://example.org")
handle("/path", RequireSession|CanGoBack, func(appData page.AppData, hw http.ResponseWriter, hr *http.Request) error {
assert.Equal(t, page.AppData{
Page: "/path",
SessionID: "cmFuZG9t",
CanGoBack: true,
LpaID: "123",
ActorType: actor.TypeDonor,
Page: "/path",
SessionID: "cmFuZG9t",
CanGoBack: true,
LpaID: "123",
ActorType: actor.TypeDonor,
AppPublicURL: "http://example.org",
}, appData)
assert.Equal(t, w, hw)

Expand Down Expand Up @@ -110,7 +112,7 @@ func TestMakeHandleErrors(t *testing.T) {
Return(&sessions.Session{Values: map[any]any{"session": &sesh.LoginSession{Sub: "random"}}}, nil)

mux := http.NewServeMux()
handle := makeHandle(mux, sessionStore, None, errorHandler.Execute)
handle := makeHandle(mux, sessionStore, None, errorHandler.Execute, "")
handle("/path", RequireSession, func(appData page.AppData, hw http.ResponseWriter, hr *http.Request) error {
return expectedError
})
Expand All @@ -128,7 +130,7 @@ func TestMakeHandleSessionError(t *testing.T) {
Return(&sessions.Session{}, expectedError)

mux := http.NewServeMux()
handle := makeHandle(mux, sessionStore, None, nil)
handle := makeHandle(mux, sessionStore, None, nil, "")
handle("/path", RequireSession, func(appData page.AppData, hw http.ResponseWriter, hr *http.Request) error { return nil })

mux.ServeHTTP(w, r)
Expand All @@ -148,7 +150,7 @@ func TestMakeHandleSessionMissing(t *testing.T) {
Return(&sessions.Session{Values: map[any]any{}}, nil)

mux := http.NewServeMux()
handle := makeHandle(mux, sessionStore, None, nil)
handle := makeHandle(mux, sessionStore, None, nil, "")
handle("/path", RequireSession, func(appData page.AppData, hw http.ResponseWriter, hr *http.Request) error { return nil })

mux.ServeHTTP(w, r)
Expand All @@ -163,7 +165,7 @@ func TestMakeHandleNoSessionRequired(t *testing.T) {
r, _ := http.NewRequest(http.MethodGet, "/path", nil)

mux := http.NewServeMux()
handle := makeHandle(mux, nil, None, nil)
handle := makeHandle(mux, nil, None, nil, "")
handle("/path", None, func(appData page.AppData, hw http.ResponseWriter, hr *http.Request) error {
assert.Equal(t, page.AppData{
Page: "/path",
Expand Down Expand Up @@ -206,12 +208,13 @@ func TestMakeLpaHandleWhenDetailsProvidedAndUIDExists(t *testing.T) {
LpaUID: "a-uid",
}, nil)

handle := makeLpaHandle(mux, sessionStore, RequireSession, nil, donorStore)
handle := makeLpaHandle(mux, sessionStore, RequireSession, nil, donorStore, "http://example.org")
handle("/path", None, func(appData page.AppData, hw http.ResponseWriter, hr *http.Request, _ *actor.DonorProvidedDetails) error {
assert.Equal(t, page.AppData{
Page: "/lpa//path",
ActorType: actor.TypeDonor,
SessionID: "cmFuZG9t",
Page: "/lpa//path",
ActorType: actor.TypeDonor,
SessionID: "cmFuZG9t",
AppPublicURL: "http://example.org",
}, appData)

assert.Equal(t, w, hw)
Expand Down Expand Up @@ -240,7 +243,7 @@ func TestMakeLpaHandleWhenSessionStoreError(t *testing.T) {
On("Get", r, "session").
Return(&sessions.Session{}, expectedError)

handle := makeLpaHandle(mux, sessionStore, RequireSession, nil, nil)
handle := makeLpaHandle(mux, sessionStore, RequireSession, nil, nil, "")
handle("/path", None, func(_ page.AppData, _ http.ResponseWriter, _ *http.Request, _ *actor.DonorProvidedDetails) error {
return expectedError
})
Expand Down Expand Up @@ -272,7 +275,7 @@ func TestMakeLpaHandleWhenLpaStoreError(t *testing.T) {
errorHandler.
On("Execute", w, r, expectedError)

handle := makeLpaHandle(mux, sessionStore, RequireSession, errorHandler.Execute, donorStore)
handle := makeLpaHandle(mux, sessionStore, RequireSession, errorHandler.Execute, donorStore, "")
handle("/path", None, func(_ page.AppData, _ http.ResponseWriter, _ *http.Request, _ *actor.DonorProvidedDetails) error {
return expectedError
})
Expand All @@ -299,14 +302,15 @@ func TestMakeLpaHandleSessionExistingSessionData(t *testing.T) {
Return(&actor.DonorProvidedDetails{}, nil)

mux := http.NewServeMux()
handle := makeLpaHandle(mux, sessionStore, None, nil, donorStore)
handle := makeLpaHandle(mux, sessionStore, None, nil, donorStore, "http://example.org")
handle("/path", RequireSession|CanGoBack, func(appData page.AppData, hw http.ResponseWriter, hr *http.Request, _ *actor.DonorProvidedDetails) error {
assert.Equal(t, page.AppData{
Page: "/lpa/123/path",
SessionID: "cmFuZG9t",
CanGoBack: true,
LpaID: "123",
ActorType: actor.TypeDonor,
Page: "/lpa/123/path",
SessionID: "cmFuZG9t",
CanGoBack: true,
LpaID: "123",
ActorType: actor.TypeDonor,
AppPublicURL: "http://example.org",
}, appData)
assert.Equal(t, w, hw)

Expand Down Expand Up @@ -342,7 +346,7 @@ func TestMakeLpaHandleErrors(t *testing.T) {
Return(&actor.DonorProvidedDetails{}, nil)

mux := http.NewServeMux()
handle := makeLpaHandle(mux, sessionStore, None, errorHandler.Execute, donorStore)
handle := makeLpaHandle(mux, sessionStore, None, errorHandler.Execute, donorStore, "")
handle("/path", RequireSession, func(_ page.AppData, _ http.ResponseWriter, _ *http.Request, _ *actor.DonorProvidedDetails) error {
return expectedError
})
Expand Down Expand Up @@ -408,7 +412,6 @@ func TestPayHelperPay(t *testing.T) {
err := (&payHelper{
sessionStore: sessionStore,
payClient: payClient,
appPublicURL: "http://example.org",
randomString: func(int) string { return "123456789012" },
}).Pay(testAppData, w, r, &actor.DonorProvidedDetails{LpaID: "lpa-id", Donor: actor.Donor{Email: "[email protected]"}, FeeType: pay.FullFee})
resp := w.Result()
Expand Down Expand Up @@ -602,7 +605,6 @@ func TestPayHelperPayWhenFeeDenied(t *testing.T) {
sessionStore: sessionStore,
donorStore: donorStore,
payClient: payClient,
appPublicURL: "http://example.org",
randomString: func(int) string { return "123456789012" },
}).Pay(testAppData, w, r, &actor.DonorProvidedDetails{
LpaID: "lpa-id",
Expand Down Expand Up @@ -672,7 +674,6 @@ func TestPayHelperPayWhenFeeDeniedAndPutStoreError(t *testing.T) {
sessionStore: sessionStore,
donorStore: donorStore,
payClient: payClient,
appPublicURL: "http://example.org",
randomString: func(int) string { return "123456789012" },
}).Pay(testAppData, w, r, &actor.DonorProvidedDetails{
LpaID: "lpa-id",
Expand Down Expand Up @@ -703,7 +704,6 @@ func TestPayHelperPayWhenCreatePaymentErrors(t *testing.T) {
err := (&payHelper{
logger: logger,
payClient: payClient,
appPublicURL: "http://example.org",
randomString: func(int) string { return "123456789012" },
}).Pay(testAppData, w, r, &actor.DonorProvidedDetails{})

Expand Down Expand Up @@ -734,7 +734,6 @@ func TestPayHelperPayWhenSessionErrors(t *testing.T) {
err := (&payHelper{
sessionStore: sessionStore,
payClient: payClient,
appPublicURL: "http://example.org",
randomString: func(int) string { return "123456789012" },
}).Pay(testAppData, w, r, &actor.DonorProvidedDetails{})

Expand Down

0 comments on commit 09dd7dd

Please sign in to comment.