Refactor redirects (#853)

* Refactor lpa redirects
* Remove lpa argument from other redirects
* Refactor redirects for other paths
* Move BuildUrl to Lang

internal/localize/lang.go

@@ -10,6 +10,14 @@ func (l Lang) String() string {
 	return englishAbbreviation
 }
 
+func (l Lang) URL(path string) string {
+	if l == Cy {
+		return "/" + Cy.String() + path
+	}
+
+	return path
+}
+
 const (
 	En Lang = iota
 	Cy

internal/localize/lang_test.go

@@ -26,3 +26,22 @@ func TestLangAbbreviation(t *testing.T) {
 		})
 	}
 }
+
+func TestLangURL(t *testing.T) {
+	testCases := map[string]struct {
+		lang Lang
+		url  string
+		want string
+	}{
+		"english":        {lang: En, url: "/example.org", want: "/example.org"},
+		"welsh":          {lang: Cy, url: "/example.org", want: "/cy/example.org"},
+		"other language": {lang: Lang(3), url: "/example.org", want: "/example.org"},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			builtUrl := tc.lang.URL(tc.url)
+			assert.Equal(t, tc.want, builtUrl)
+		})
+	}
+}

internal/page/app_data.go

@@ -28,33 +28,11 @@ type AppData struct {
 	OneloginURL      string
 }
 
-func (d AppData) Redirect(w http.ResponseWriter, r *http.Request, lpa *Lpa, url string) error {
-	if fromURL := r.FormValue("from"); fromURL != "" {
-		url = fromURL
-	}
-
-	if lpa != nil && d.LpaID == "" {
-		d.LpaID = lpa.ID
-	}
-
-	// as a shortcut for when you don't have an Lpa but know the transition is fine we allow passing nil
-	if lpa == nil || lpa.CanGoTo(url) {
-		http.Redirect(w, r, d.BuildUrl(url), http.StatusFound)
-	} else {
-		http.Redirect(w, r, d.BuildUrl(Paths.TaskList.Format(d.LpaID)), http.StatusFound)
-	}
-
+func (d AppData) Redirect(w http.ResponseWriter, r *http.Request, url string) error {
+	http.Redirect(w, r, d.Lang.URL(url), http.StatusFound)
 	return nil
 }
 
-func (d AppData) BuildUrl(url string) string {
-	if d.Lang == localize.Cy {
-		return "/" + localize.Cy.String() + url
-	}
-
-	return url
-}
-
 func ContextWithAppData(ctx context.Context, appData AppData) context.Context {
 	return context.WithValue(ctx, contextKey("appData"), appData)
 }

internal/page/app_data_test.go

@@ -7,7 +7,6 @@ import (
 	"testing"
 
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
-	"github.com/ministryofjustice/opg-modernising-lpa/internal/form"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/localize"
 	"github.com/stretchr/testify/assert"
 )
@@ -23,7 +22,7 @@ func TestAppDataRedirect(t *testing.T) {
 			r, _ := http.NewRequest(http.MethodGet, "/", nil)
 			w := httptest.NewRecorder()
 
-			AppData{Lang: lang, LpaID: "lpa-id"}.Redirect(w, r, nil, "/dashboard")
+			AppData{Lang: lang, LpaID: "lpa-id"}.Redirect(w, r, "/dashboard")
 			resp := w.Result()
 
 			assert.Equal(t, http.StatusFound, resp.StatusCode)
@@ -32,93 +31,6 @@ func TestAppDataRedirect(t *testing.T) {
 	}
 }
 
-func TestAppDataRedirectWhenCanGoTo(t *testing.T) {
-	testCases := map[string]struct {
-		url      string
-		lpa      *Lpa
-		expected string
-	}{
-		"nil": {
-			url:      "/",
-			lpa:      nil,
-			expected: Paths.HowToConfirmYourIdentityAndSign.Format("lpa-id"),
-		},
-		"nil and from": {
-			url:      "/?from=" + Paths.Restrictions.Format("lpa-id"),
-			lpa:      nil,
-			expected: Paths.Restrictions.Format("lpa-id"),
-		},
-		"allowed": {
-			url: "/",
-			lpa: &Lpa{
-				Donor: actor.Donor{
-					CanSign: form.Yes,
-				},
-				Type: LpaTypeHealthWelfare,
-				Tasks: Tasks{
-					YourDetails:                actor.TaskCompleted,
-					ChooseAttorneys:            actor.TaskCompleted,
-					ChooseReplacementAttorneys: actor.TaskCompleted,
-					LifeSustainingTreatment:    actor.TaskCompleted,
-					Restrictions:               actor.TaskCompleted,
-					CertificateProvider:        actor.TaskCompleted,
-					PeopleToNotify:             actor.TaskCompleted,
-					CheckYourLpa:               actor.TaskCompleted,
-					PayForLpa:                  actor.PaymentTaskCompleted,
-				},
-			},
-			expected: Paths.HowToConfirmYourIdentityAndSign.Format("lpa-id"),
-		},
-		"allowed from": {
-			url:      "/?from=" + Paths.Restrictions.Format("lpa-id"),
-			lpa:      &Lpa{Tasks: Tasks{YourDetails: actor.TaskCompleted, ChooseAttorneys: actor.TaskCompleted}},
-			expected: Paths.Restrictions.Format("lpa-id"),
-		},
-		"not allowed": {
-			url:      "/",
-			lpa:      &Lpa{},
-			expected: Paths.TaskList.Format("lpa-id"),
-		},
-		"not allowed from": {
-			url:      "/?from=" + Paths.Restrictions.Format("lpa-id"),
-			lpa:      &Lpa{},
-			expected: Paths.TaskList.Format("lpa-id"),
-		},
-	}
-
-	for name, tc := range testCases {
-		t.Run(name, func(t *testing.T) {
-			r, _ := http.NewRequest(http.MethodGet, tc.url, nil)
-			w := httptest.NewRecorder()
-
-			AppData{Lang: localize.En, LpaID: "lpa-id"}.Redirect(w, r, tc.lpa, Paths.HowToConfirmYourIdentityAndSign.Format("lpa-id"))
-			resp := w.Result()
-
-			assert.Equal(t, http.StatusFound, resp.StatusCode)
-			assert.Equal(t, tc.expected, resp.Header.Get("Location"))
-		})
-	}
-}
-
-func TestAppDataBuildUrl(t *testing.T) {
-	testCases := map[string]struct {
-		lang localize.Lang
-		url  string
-		want string
-	}{
-		"english":        {lang: localize.En, url: "/example.org", want: "/example.org"},
-		"welsh":          {lang: localize.Cy, url: "/example.org", want: "/cy/example.org"},
-		"other language": {lang: localize.Lang(3), url: "/example.org", want: "/example.org"},
-	}
-
-	for name, tc := range testCases {
-		t.Run(name, func(t *testing.T) {
-			builtUrl := AppData{Lang: tc.lang}.BuildUrl(tc.url)
-			assert.Equal(t, tc.want, builtUrl)
-		})
-	}
-}
-
 func TestAppDataContext(t *testing.T) {
 	appData := AppData{LpaID: "me"}
 	ctx := context.Background()

internal/page/attorney/confirm_your_details.go

@@ -27,7 +27,7 @@ func ConfirmYourDetails(tmpl template.Template, attorneyStore AttorneyStore, don
 				return err
 			}
 
-			return appData.Redirect(w, r, nil, page.Paths.Attorney.ReadTheLpa.Format(attorneyProvidedDetails.LpaID))
+			return page.Paths.Attorney.ReadTheLpa.Redirect(w, r, appData, attorneyProvidedDetails.LpaID)
 		}
 
 		lpa, err := donorStore.GetAny(r.Context())

internal/page/attorney/enter_reference_number.go

@@ -64,7 +64,7 @@ func EnterReferenceNumber(tmpl template.Template, shareCodeStore ShareCodeStore,
 				}
 
 				appData.LpaID = shareCode.LpaID
-				return appData.Redirect(w, r, nil, page.Paths.Attorney.CodeOfConduct.Format(shareCode.LpaID))
+				return page.Paths.Attorney.CodeOfConduct.Redirect(w, r, appData, shareCode.LpaID)
 			}
 		}
 

internal/page/attorney/mobile_number.go

@@ -43,7 +43,7 @@ func MobileNumber(tmpl template.Template, attorneyStore AttorneyStore) Handler {
 					return err
 				}
 
-				return appData.Redirect(w, r, nil, page.Paths.Attorney.ConfirmYourDetails.Format(attorneyProvidedDetails.LpaID))
+				return page.Paths.Attorney.ConfirmYourDetails.Redirect(w, r, appData, attorneyProvidedDetails.LpaID)
 			}
 		}
 

internal/page/attorney/read_the_lpa.go

@@ -24,7 +24,7 @@ func ReadTheLpa(tmpl template.Template, donorStore DonorStore, attorneyStore Att
 				return err
 			}
 
-			return appData.Redirect(w, r, nil, page.Paths.Attorney.RightsAndResponsibilities.Format(attorneyProvidedDetails.LpaID))
+			return page.Paths.Attorney.RightsAndResponsibilities.Redirect(w, r, appData, attorneyProvidedDetails.LpaID)
 		}
 
 		lpa, err := donorStore.GetAny(r.Context())

internal/page/attorney/sign.go

@@ -53,7 +53,7 @@ func Sign(tmpl template.Template, donorStore DonorStore, certificateProviderStor
 		}
 
 		if ok, _ := canSign(r.Context(), certificateProviderStore, lpa); !ok {
-			return appData.Redirect(w, r, lpa, page.Paths.Attorney.TaskList.Format(attorneyProvidedDetails.LpaID))
+			return page.Paths.Attorney.TaskList.Redirect(w, r, appData, attorneyProvidedDetails.LpaID)
 		}
 
 		data := &signData{
@@ -90,7 +90,7 @@ func Sign(tmpl template.Template, donorStore DonorStore, certificateProviderStor
 
 			attorney, ok := attorneys.Get(appData.AttorneyID)
 			if !ok {
-				return appData.Redirect(w, r, lpa, page.Paths.Attorney.Start.Format())
+				return page.Paths.Attorney.Start.Redirect(w, r, appData)
 			}
 
 			data.Attorney = attorney
@@ -125,9 +125,9 @@ func Sign(tmpl template.Template, donorStore DonorStore, certificateProviderStor
 				}
 
 				if appData.IsTrustCorporation() && signatoryIndex == 0 {
-					return appData.Redirect(w, r, lpa, page.Paths.Attorney.WouldLikeSecondSignatory.Format(attorneyProvidedDetails.LpaID))
+					return page.Paths.Attorney.WouldLikeSecondSignatory.Redirect(w, r, appData, attorneyProvidedDetails.LpaID)
 				} else {
-					return appData.Redirect(w, r, lpa, page.Paths.Attorney.WhatHappensNext.Format(attorneyProvidedDetails.LpaID))
+					return page.Paths.Attorney.WhatHappensNext.Redirect(w, r, appData, attorneyProvidedDetails.LpaID)
 				}
 			}
 		}

internal/page/attorney/would_like_second_signatory.go

@@ -2,6 +2,7 @@ package attorney
 
 import (
 	"net/http"
+	"net/url"
 
 	"github.com/ministryofjustice/opg-go-common/template"
 	"github.com/ministryofjustice/opg-modernising-lpa/internal/actor"
@@ -37,9 +38,9 @@ func WouldLikeSecondSignatory(tmpl template.Template, attorneyStore AttorneyStor
 				}
 
 				if form.YesNo.IsYes() {
-					return appData.Redirect(w, r, nil, page.Paths.Attorney.Sign.Format(attorneyProvidedDetails.LpaID)+"?second")
+					return page.Paths.Attorney.Sign.RedirectQuery(w, r, appData, attorneyProvidedDetails.LpaID, url.Values{"second": {""}})
 				} else {
-					return appData.Redirect(w, r, nil, page.Paths.Attorney.WhatHappensNext.Format(attorneyProvidedDetails.LpaID))
+					return page.Paths.Attorney.WhatHappensNext.Redirect(w, r, appData, attorneyProvidedDetails.LpaID)
 				}
 			}
 		}

internal/page/attorney/would_like_second_signatory_test.go

@@ -55,7 +55,7 @@ func TestGetWouldLikeSecondSignatoryWhenTemplateErrors(t *testing.T) {
 
 func TestPostWouldLikeSecondSignatory(t *testing.T) {
 	testcases := map[form.YesNo]string{
-		form.Yes: page.Paths.Attorney.Sign.Format("lpa-id") + "?second",
+		form.Yes: page.Paths.Attorney.Sign.Format("lpa-id") + "?second=",
 		form.No:  page.Paths.Attorney.WhatHappensNext.Format("lpa-id"),
 	}
 

internal/page/auth_redirect.go

@@ -27,6 +27,6 @@ func AuthRedirect(logger Logger, store sesh.Store) http.HandlerFunc {
 
 		appData := AppData{Lang: lang, LpaID: oneLoginSession.LpaID}
 
-		appData.Redirect(w, r, nil, oneLoginSession.Redirect+"?"+r.URL.RawQuery)
+		appData.Redirect(w, r, oneLoginSession.Redirect+"?"+r.URL.RawQuery)
 	}
 }

internal/page/certificateprovider/confirm_your_details.go

@@ -29,9 +29,9 @@ func ConfirmYourDetails(tmpl template.Template, donorStore DonorStore, certifica
 		}
 
 		if r.Method == http.MethodPost {
-			redirect := page.Paths.CertificateProvider.YourRole.Format(certificateProvider.LpaID)
+			redirect := page.Paths.CertificateProvider.YourRole
 			if certificateProvider.Tasks.ConfirmYourDetails.Completed() || !lpa.SignedAt.IsZero() {
-				redirect = page.Paths.CertificateProvider.TaskList.Format(certificateProvider.LpaID)
+				redirect = page.Paths.CertificateProvider.TaskList
 			}
 
 			certificateProvider.Tasks.ConfirmYourDetails = actor.TaskCompleted
@@ -40,7 +40,7 @@ func ConfirmYourDetails(tmpl template.Template, donorStore DonorStore, certifica
 				return err
 			}
 
-			return appData.Redirect(w, r, nil, redirect)
+			return redirect.Redirect(w, r, appData, certificateProvider.LpaID)
 		}
 
 		data := &confirmYourDetailsData{

internal/page/certificateprovider/enter_date_of_birth.go

@@ -62,7 +62,7 @@ func EnterDateOfBirth(tmpl template.Template, donorStore DonorStore, certificate
 					return err
 				}
 
-				return appData.Redirect(w, r, lpa, page.Paths.CertificateProvider.ConfirmYourDetails.Format(certificateProvider.LpaID))
+				return page.Paths.CertificateProvider.ConfirmYourDetails.Redirect(w, r, appData, certificateProvider.LpaID)
 			}
 		}
 

internal/page/certificateprovider/enter_reference_number.go

@@ -64,7 +64,7 @@ func EnterReferenceNumber(tmpl template.Template, shareCodeStore ShareCodeStore,
 				}
 
 				appData.LpaID = shareCode.LpaID
-				return appData.Redirect(w, r, nil, page.Paths.CertificateProvider.WhoIsEligible.Format(shareCode.LpaID))
+				return page.Paths.CertificateProvider.WhoIsEligible.Redirect(w, r, appData, shareCode.LpaID)
 			}
 		}
 

internal/page/certificateprovider/identity_with_one_login_callback.go

@@ -36,9 +36,9 @@ func IdentityWithOneLoginCallback(tmpl template.Template, oneLoginClient OneLogi
 
 		if r.Method == http.MethodPost {
 			if certificateProvider.CertificateProviderIdentityConfirmed(lpa.CertificateProvider.FirstNames, lpa.CertificateProvider.LastName) {
-				return appData.Redirect(w, r, nil, page.Paths.CertificateProvider.ReadTheLpa.Format(certificateProvider.LpaID))
+				return page.Paths.CertificateProvider.ReadTheLpa.Redirect(w, r, appData, certificateProvider.LpaID)
 			} else {
-				return appData.Redirect(w, r, nil, page.Paths.CertificateProvider.ProveYourIdentity.Format(certificateProvider.LpaID))
+				return page.Paths.CertificateProvider.ProveYourIdentity.Redirect(w, r, appData, certificateProvider.LpaID)
 			}
 		}
 

internal/page/certificateprovider/provide_certificate.go

@@ -31,7 +31,7 @@ func ProvideCertificate(tmpl template.Template, donorStore DonorStore, now func(
 		}
 
 		if lpa.SignedAt.IsZero() {
-			return appData.Redirect(w, r, lpa, page.Paths.CertificateProvider.TaskList.Format(lpa.ID))
+			return page.Paths.CertificateProvider.TaskList.Redirect(w, r, appData, lpa.ID)
 		}
 
 		data := &provideCertificateData{
@@ -55,7 +55,7 @@ func ProvideCertificate(tmpl template.Template, donorStore DonorStore, now func(
 					return err
 				}
 
-				return appData.Redirect(w, r, lpa, page.Paths.CertificateProvider.CertificateProvided.Format(certificateProvider.LpaID))
+				return page.Paths.CertificateProvider.CertificateProvided.Redirect(w, r, appData, certificateProvider.LpaID)
 			}
 		}
 

internal/page/certificateprovider/read_the_lpa.go

@@ -30,15 +30,15 @@ func ReadTheLpa(tmpl template.Template, donorStore DonorStore, certificateProvid
 
 		if r.Method == http.MethodPost {
 			if lpa.SignedAt.IsZero() || !lpa.Tasks.PayForLpa.IsCompleted() {
-				return appData.Redirect(w, r, nil, page.Paths.CertificateProvider.TaskList.Format(lpa.ID))
+				return page.Paths.CertificateProvider.TaskList.Redirect(w, r, appData, lpa.ID)
 			}
 
 			certificateProvider.Tasks.ReadTheLpa = actor.TaskCompleted
 			if err := certificateProviderStore.Put(r.Context(), certificateProvider); err != nil {
 				return err
 			}
 
-			return appData.Redirect(w, r, nil, page.Paths.CertificateProvider.WhatHappensNext.Format(lpa.ID))
+			return page.Paths.CertificateProvider.WhatHappensNext.Redirect(w, r, appData, lpa.ID)
 		}
 
 		data := &readTheLpaData{

internal/page/dashboard.go

@@ -37,7 +37,7 @@ func Dashboard(tmpl template.Template, donorStore DonorStore, dashboardStore Das
 				return err
 			}
 
-			return appData.Redirect(w, r, lpa, Paths.YourDetails.Format(lpa.ID))
+			return Paths.YourDetails.Redirect(w, r, appData, lpa)
 		}
 
 		donorLpas, attorneyLpas, certificateProviderLpas, err := dashboardStore.GetAll(r.Context())

internal/page/donor/are_you_applying_for_fee_discount_or_exemption.go

@@ -39,7 +39,7 @@ func AreYouApplyingForFeeDiscountOrExemption(tmpl template.Template, payer Payer
 				if data.Form.YesNo.IsNo() {
 					return payer.Pay(appData, w, r, lpa)
 				} else {
-					return appData.Redirect(w, r, lpa, page.Paths.WhichFeeTypeAreYouApplyingFor.Format(lpa.ID))
+					return page.Paths.WhichFeeTypeAreYouApplyingFor.Redirect(w, r, appData, lpa)
 				}
 			}
 		}