From e2bcbdcb6c12e6ab135b33fea3e02782c36c4987 Mon Sep 17 00:00:00 2001 From: Nick Lupton <19624419+mr-nick17@users.noreply.github.com> Date: Tue, 11 Apr 2023 09:37:52 +0100 Subject: [PATCH 01/19] 1599 refactor handlers and remove redundant code --- handlers/feedback.go | 39 ++++++++++++++++----------------------- handlers/feedback_test.go | 15 +++++---------- handlers/handlers.go | 22 ++++++++++++++++++++++ routes/routes.go | 10 ++++++---- 4 files changed, 49 insertions(+), 37 deletions(-) diff --git a/handlers/feedback.go b/handlers/feedback.go index 4243da9..4b91b63 100644 --- a/handlers/feedback.go +++ b/handlers/feedback.go @@ -18,8 +18,8 @@ import ( "github.com/gorilla/schema" ) -// Feedback represents a user's feedback -type Feedback struct { +// FeedbackForm represents a user's feedback +type FeedbackForm struct { Type string `schema:"type"` URI string `schema:":uri"` URL string `schema:"url"` @@ -30,9 +30,9 @@ type Feedback struct { } // FeedbackThanks loads the Feedback Thank you page -func FeedbackThanks(rend interfaces.Renderer, cacheService *cacheHelper.Helper) http.HandlerFunc { +func (f *Feedback) FeedbackThanks() http.HandlerFunc { return dphandlers.ControllerHandler(func(w http.ResponseWriter, req *http.Request, lang, collectionID, accessToken string) { - feedbackThanks(w, req, req.Referer(), "", rend, cacheService, lang) + feedbackThanks(w, req, req.Referer(), "", f.Render, f.CacheService, lang) }) } @@ -75,9 +75,9 @@ func feedbackThanks(w http.ResponseWriter, req *http.Request, url, errorType str } // GetFeedback handles the loading of a feedback page -func GetFeedback(rend interfaces.Renderer, cacheService *cacheHelper.Helper) http.HandlerFunc { +func (f *Feedback) GetFeedback() http.HandlerFunc { return dphandlers.ControllerHandler(func(w http.ResponseWriter, req *http.Request, lang, collectionID, accessToken string) { - getFeedback(w, req, req.Referer(), "", "", "", "", lang, rend, cacheService) + getFeedback(w, req, req.Referer(), "", "", "", "", lang, f.Render, f.CacheService) }) } @@ -123,13 +123,13 @@ func getFeedback(w http.ResponseWriter, req *http.Request, url, errorType, descr } // AddFeedback handles a users feedback request and sends a message to slack -func AddFeedback(to, from string, isPositive bool, rend interfaces.Renderer, emailSender email.Sender, cacheService *cacheHelper.Helper) http.HandlerFunc { +func (f *Feedback) AddFeedback() http.HandlerFunc { return dphandlers.ControllerHandler(func(w http.ResponseWriter, req *http.Request, lang, collectionID, accessToken string) { - addFeedback(w, req, isPositive, rend, emailSender, from, to, lang, cacheService) + addFeedback(w, req, f.Render, f.EmailSender, f.Config.FeedbackFrom, f.Config.FeedbackTo, lang, f.CacheService) }) } -func addFeedback(w http.ResponseWriter, req *http.Request, isPositive bool, rend interfaces.Renderer, emailSender email.Sender, from, to, lang string, cacheService *cacheHelper.Helper) { +func addFeedback(w http.ResponseWriter, req *http.Request, rend interfaces.Renderer, emailSender email.Sender, from, to, lang string, cacheService *cacheHelper.Helper) { ctx := req.Context() if err := req.ParseForm(); err != nil { log.Error(ctx, "unable to parse request form", err) @@ -140,19 +140,19 @@ func addFeedback(w http.ResponseWriter, req *http.Request, isPositive bool, rend decoder := schema.NewDecoder() decoder.IgnoreUnknownKeys(true) - var f Feedback + var f FeedbackForm if err := decoder.Decode(&f, req.Form); err != nil { log.Error(ctx, "unable to decode request form", err) w.WriteHeader(http.StatusInternalServerError) return } - if f.Description == "" && !isPositive { + if f.Description == "" { getFeedback(w, req, f.URL, "description", f.Description, f.Name, f.Email, lang, rend, cacheService) return } - if len(f.Email) > 0 && !isPositive { + if len(f.Email) > 0 { if ok, err := regexp.MatchString(`^[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,6}$`, f.Email); !ok || err != nil { getFeedback(w, req, f.URL, "email", f.Description, f.Name, f.Email, lang, rend, cacheService) return @@ -166,7 +166,7 @@ func addFeedback(w http.ResponseWriter, req *http.Request, isPositive bool, rend if err := emailSender.Send( from, []string{to}, - generateFeedbackMessage(f, from, to, isPositive), + generateFeedbackMessage(f, from, to), ); err != nil { log.Error(ctx, "failed to send message", err) w.WriteHeader(http.StatusInternalServerError) @@ -183,26 +183,19 @@ func addFeedback(w http.ResponseWriter, req *http.Request, isPositive bool, rend http.Redirect(w, req, redirectURL, http.StatusMovedPermanently) } -func generateFeedbackMessage(f Feedback, from, to string, isPositive bool) []byte { - var description string - if isPositive { - description = "Positive feedback received" - } else { - description = f.Description - } - +func generateFeedbackMessage(f FeedbackForm, from, to string) []byte { var b bytes.Buffer b.WriteString(fmt.Sprintf("From: %s\n", from)) b.WriteString(fmt.Sprintf("To: %s\n", to)) - b.WriteString(fmt.Sprintf("Subject: Feedback received\n\n")) + b.WriteString("Subject: Feedback received\n\n") if len(f.Type) > 0 { b.WriteString(fmt.Sprintf("Feedback Type: %s\n", f.Type)) } b.WriteString(fmt.Sprintf("Page URL: %s\n", f.URL)) - b.WriteString(fmt.Sprintf("Description: %s\n", description)) + b.WriteString(fmt.Sprintf("Description: %s\n", f.Description)) if len(f.Name) > 0 { b.WriteString(fmt.Sprintf("Name: %s\n", f.Name)) diff --git a/handlers/feedback_test.go b/handlers/feedback_test.go index 5e0f3b8..42a5e74 100644 --- a/handlers/feedback_test.go +++ b/handlers/feedback_test.go @@ -113,7 +113,6 @@ func Test_addFeedback(t *testing.T) { Convey("Given a valid request", t, func() { req := httptest.NewRequest("GET", "http://localhost?description=whatever", nil) w := httptest.NewRecorder() - isPositive := false from := "" to := "" lang := "en" @@ -144,7 +143,7 @@ func Test_addFeedback(t *testing.T) { }, }} Convey("When addFeedback is called", func() { - addFeedback(w, req, isPositive, mockRenderer, mockSender, from, to, lang, mockNagivationCache) + addFeedback(w, req, mockRenderer, mockSender, from, to, lang, mockNagivationCache) Convey("Then the renderer is not called", func() { So(len(mockRenderer.BuildPageCalls()), ShouldEqual, 0) }) @@ -160,7 +159,6 @@ func Test_addFeedback(t *testing.T) { Convey("Given an error returned from the sender", t, func() { req := httptest.NewRequest("GET", "http://localhost?description=whatever", nil) w := httptest.NewRecorder() - isPositive := false from := "" to := "" lang := "en" @@ -191,7 +189,7 @@ func Test_addFeedback(t *testing.T) { }, }} Convey("When addFeedback is called", func() { - addFeedback(w, req, isPositive, mockRenderer, mockSender, from, to, lang, mockNagivationCache) + addFeedback(w, req, mockRenderer, mockSender, from, to, lang, mockNagivationCache) Convey("Then the renderer is not called", func() { So(len(mockRenderer.BuildPageCalls()), ShouldEqual, 0) }) @@ -209,7 +207,6 @@ func Test_addFeedback(t *testing.T) { Convey("Given a request with invalid form data", t, func() { req := httptest.NewRequest("POST", "http://localhost?!@£$@$£%£$%^^&^&*", nil) w := httptest.NewRecorder() - isPositive := false from := "" to := "" lang := "en" @@ -240,7 +237,7 @@ func Test_addFeedback(t *testing.T) { }, }} Convey("When addFeedback is called", func() { - addFeedback(w, req, isPositive, mockRenderer, mockSender, from, to, lang, mockNagivationCache) + addFeedback(w, req, mockRenderer, mockSender, from, to, lang, mockNagivationCache) Convey("Then the renderer is not called", func() { So(len(mockRenderer.BuildPageCalls()), ShouldEqual, 0) }) @@ -259,7 +256,6 @@ func Test_addFeedback(t *testing.T) { req := httptest.NewRequest("POST", "http://localhost?service=dev", nil) req.Header.Set("Content-Type", "application/x-www-form-urlencoded") w := httptest.NewRecorder() - isPositive := false from := "" to := "" lang := "en" @@ -290,7 +286,7 @@ func Test_addFeedback(t *testing.T) { }, }} Convey("When addFeedback is called", func() { - addFeedback(w, req, isPositive, mockRenderer, mockSender, from, to, lang, mockNagivationCache) + addFeedback(w, req, mockRenderer, mockSender, from, to, lang, mockNagivationCache) Convey("Then the renderer is called to render the feedback page", func() { So(len(mockRenderer.BuildPageCalls()), ShouldEqual, 1) }) @@ -308,7 +304,6 @@ func Test_addFeedback(t *testing.T) { req.Header.Set("Content-Type", "application/x-www-form-urlencoded") w := httptest.NewRecorder() - isPositive := false from := "" to := "" lang := "en" @@ -339,7 +334,7 @@ func Test_addFeedback(t *testing.T) { }, }} Convey("When addFeedback is called", func() { - addFeedback(w, req, isPositive, mockRenderer, mockSender, from, to, lang, mockNagivationCache) + addFeedback(w, req, mockRenderer, mockSender, from, to, lang, mockNagivationCache) Convey("Then the renderer is called to render the feedback page", func() { So(len(mockRenderer.BuildPageCalls()), ShouldEqual, 1) }) diff --git a/handlers/handlers.go b/handlers/handlers.go index f8c144c..6b891f9 100644 --- a/handlers/handlers.go +++ b/handlers/handlers.go @@ -3,9 +3,31 @@ package handlers import ( "net/http" + cacheHelper "github.com/ONSdigital/dp-frontend-cache-helper/pkg/navigation/helper" + "github.com/ONSdigital/dp-frontend-feedback-controller/config" + "github.com/ONSdigital/dp-frontend-feedback-controller/email" + "github.com/ONSdigital/dp-frontend-feedback-controller/interfaces" "github.com/ONSdigital/log.go/v2/log" ) +// Feedback represents the handlers required to provide feedback +type Feedback struct { + Render interfaces.Renderer + CacheService *cacheHelper.Helper + Config *config.Config + EmailSender email.Sender +} + +// NewFeedback creates a new instance of Feedback +func NewFeedback(rc interfaces.Renderer, c *cacheHelper.Helper, cfg *config.Config, es email.Sender) *Feedback { + return &Feedback{ + Render: rc, + CacheService: c, + Config: cfg, + EmailSender: es, + } +} + // ClientError is an interface that can be used to retrieve the status code if a client has errored type ClientError interface { Error() string diff --git a/routes/routes.go b/routes/routes.go index d4f78ea..2033455 100644 --- a/routes/routes.go +++ b/routes/routes.go @@ -36,12 +36,14 @@ func Setup(ctx context.Context, r *mux.Router, cfg *config.Config, rend *render. Auth: auth, } + f := handlers.NewFeedback(rend, cacheService, cfg, emailSender) + log.Info(ctx, "adding routes") r.StrictSlash(true).Path("/health").HandlerFunc(hc.Handler) - r.StrictSlash(true).Path("/feedback").Methods("GET").HandlerFunc(handlers.GetFeedback(rend, cacheService)) - r.StrictSlash(true).Path("/feedback").Methods("POST").HandlerFunc(handlers.AddFeedback(cfg.FeedbackTo, cfg.FeedbackFrom, false, rend, emailSender, cacheService)) - r.StrictSlash(true).Path("/feedback/thanks").Methods("GET").HandlerFunc(handlers.FeedbackThanks(rend, cacheService)) - r.StrictSlash(true).Path("/feedback/thanks").Methods("POST").HandlerFunc(handlers.AddFeedback(cfg.FeedbackTo, cfg.FeedbackFrom, false, rend, emailSender, cacheService)) + r.StrictSlash(true).Path("/feedback").Methods("GET").HandlerFunc(f.GetFeedback()) + r.StrictSlash(true).Path("/feedback").Methods("POST").HandlerFunc(f.AddFeedback()) + r.StrictSlash(true).Path("/feedback/thanks").Methods("GET").HandlerFunc(f.FeedbackThanks()) + r.StrictSlash(true).Path("/feedback/thanks").Methods("POST").HandlerFunc(f.AddFeedback()) } type unencryptedAuth struct { From 654bd1c5b7363a36a54a4fe16f4f94f56c108e77 Mon Sep 17 00:00:00 2001 From: Nick Lupton <19624419+mr-nick17@users.noreply.github.com> Date: Wed, 12 Apr 2023 15:43:19 +0100 Subject: [PATCH 02/19] 1599 update html to design system and refactoring --- assets/locales/service.cy.toml | 12 +- assets/locales/service.en.toml | 12 +- assets/templates/feedback-thanks.tmpl | 51 ++++--- assets/templates/feedback.tmpl | 192 ++++++++++++-------------- go.mod | 3 +- go.sum | 4 - handlers/feedback.go | 22 ++- handlers/feedback_test.go | 10 +- mocks/mocks.go | 16 +++ model/model.go | 1 + 10 files changed, 173 insertions(+), 150 deletions(-) create mode 100644 mocks/mocks.go diff --git a/assets/locales/service.cy.toml b/assets/locales/service.cy.toml index e8975d8..77bf858 100644 --- a/assets/locales/service.cy.toml +++ b/assets/locales/service.cy.toml @@ -14,8 +14,8 @@ #------------------------------------------------------------------------------------------------------------------------------------------------------------------- [FeedbackTitle] -description = "{{.Metadata.Title}}" -one = "{{.arg0}}" +description = "Feedback" +one = "Feedback" [FeedbackDesc] descriptions = "You can use the form below to ask a question, report a problem or suggest an improvement we can make to ONS.GOV.UK" @@ -38,8 +38,8 @@ description = "{{.ServiceDescription}}" one = "({{.arg0}})" [FeedbackWhatOptNewService] -description = "This new service" -one = "This new service" +description = "This {{.ServiceName}} service" +one = "The {{.arg0}} service" [FeedbackWhatOptGeneral] description = "The whole ONS website or general feedback" @@ -86,8 +86,8 @@ description = "Send feedback" one = "Send feedback" [FeedbackFinished] -description = "Your feedback will help us to improve the website. We are unable to respond to all enquiries. If your matter is urgent, please contact us.

Return to {{.Metadata.Description}}" -one = "Your feedback will help us to improve the website. We are unable to respond to all enquiries. If your matter is urgent, please contact us.

Return to {{.arg0}}" +description = "Your feedback will help us to improve the website. We are unable to respond to all enquiries. If your matter is urgent, please contact us." +one = "Your feedback will help us to improve the website. We are unable to respond to all enquiries. If your matter is urgent, please contact us." [FeedbackWholeWebsite] description = "The whole ONS website or general feedback" diff --git a/assets/locales/service.en.toml b/assets/locales/service.en.toml index f9f5d5f..75ba56c 100644 --- a/assets/locales/service.en.toml +++ b/assets/locales/service.en.toml @@ -14,8 +14,8 @@ #------------------------------------------------------------------------------------------------------------------------------------------------------------------- [FeedbackTitle] -description = "{{.Metadata.Title}}" -one = "{{.arg0}}" +description = "Feedback" +one = "Feedback" [FeedbackDesc] descriptions = "You can use the form below to ask a question, report a problem or suggest an improvement we can make to ONS.GOV.UK" @@ -38,8 +38,8 @@ description = "{{.ServiceDescription}}" one = "({{.arg0}})" [FeedbackWhatOptNewService] -description = "This new service" -one = "This new service" +description = "This {{.ServiceName}} service" +one = "The {{.arg0}} service" [FeedbackWhatOptGeneral] description = "The whole ONS website or general feedback" @@ -86,8 +86,8 @@ description = "Send feedback" one = "Send feedback" [FeedbackFinished] -description = "Your feedback will help us to improve the website. We are unable to respond to all enquiries. If your matter is urgent, please contact us.

Return to {{.Metadata.Description}}" -one = "Your feedback will help us to improve the website. We are unable to respond to all enquiries. If your matter is urgent, please contact us.

Return to {{.arg0}}" +description = "Your feedback will help us to improve the website. We are unable to respond to all enquiries. If your matter is urgent, please contact us." +one = "Your feedback will help us to improve the website. We are unable to respond to all enquiries. If your matter is urgent, please contact us." [FeedbackWholeWebsite] description = "The whole ONS website or general feedback" diff --git a/assets/templates/feedback-thanks.tmpl b/assets/templates/feedback-thanks.tmpl index 99cc706..1e576cb 100644 --- a/assets/templates/feedback-thanks.tmpl +++ b/assets/templates/feedback-thanks.tmpl @@ -1,21 +1,36 @@ -{{$Language := .Language}} -
-
-
-
- {{ template "partials/breadcrumb" . }} -

- {{ localise "FeedbackThanks" $Language 1 $.Metadata.Title }} -

-
-
-
-
-
-
-
-
- {{ localise "FeedbackFinished" $Language 1 .Metadata.Description .Metadata.Description | safeHTML }} +
+
+
+
+ + + + + Done + + +
diff --git a/assets/templates/feedback.tmpl b/assets/templates/feedback.tmpl index fc84cda..c99c495 100644 --- a/assets/templates/feedback.tmpl +++ b/assets/templates/feedback.tmpl @@ -1,120 +1,104 @@ -{{$Language := .Language}} - -
-
-
- {{if eq .Metadata.Title "Feedback"}} -
-

{{ localise "FeedbackDesc" $Language 1}}

-
-
-
+
+
+ {{ template "partials/breadcrumb" . }} +

{{- localise "FeedbackTitle" .Language 1 $.Metadata.Title -}}

+
+
+

{{- localise "FeedbackDesc" .Language 1 -}}

+ -
-

{{ localise "FeedbackTitleWhat" $Language 1 }}

 -
- - {{if .ServiceDescription}} -
- diff --git a/go.mod b/go.mod index a219a93..e3cd665 100644 --- a/go.mod +++ b/go.mod @@ -4,13 +4,13 @@ go 1.19 require ( github.com/ONSdigital/dp-frontend-cache-helper v0.2.0 - github.com/ONSdigital/dp-frontend-models v1.10.1 github.com/ONSdigital/dp-healthcheck v1.5.0 github.com/ONSdigital/dp-net/v2 v2.6.0 github.com/ONSdigital/dp-renderer v1.62.0 github.com/ONSdigital/dp-topic-api v0.16.0 github.com/ONSdigital/go-ns v0.0.0-20210916104633-ac1c1c52327e github.com/ONSdigital/log.go/v2 v2.3.0 + github.com/elazarl/go-bindata-assetfs v1.0.1 github.com/gorilla/mux v1.8.0 github.com/gorilla/schema v1.2.0 github.com/kelseyhightower/envconfig v1.4.0 @@ -25,7 +25,6 @@ require ( github.com/ONSdigital/dp-net v1.5.0 // indirect github.com/aws/aws-sdk-go v1.44.76 // indirect github.com/c2h5oh/datasize v0.0.0-20200825124411-48ed595a09d2 // indirect - github.com/elazarl/go-bindata-assetfs v1.0.1 // indirect github.com/fatih/color v1.13.0 // indirect github.com/fsnotify/fsnotify v1.4.9 // indirect github.com/go-bindata/go-bindata v3.1.2+incompatible // indirect diff --git a/go.sum b/go.sum index 0d9f058..547f638 100644 --- a/go.sum +++ b/go.sum @@ -13,8 +13,6 @@ github.com/ONSdigital/dp-cache v0.3.0/go.mod h1:cyduXeUO8n9TLZIorCWDzEoLkk1jK8q4 github.com/ONSdigital/dp-frontend-cache-helper v0.2.0 h1:IL1wUKY1gbI4rkLCmzNDDke0gm6D7YQgHufekFq94p8= github.com/ONSdigital/dp-frontend-cache-helper v0.2.0/go.mod h1:3tOGFnElMHDTfAiFbWD2W64zF+IWJSlPS/f/wbfvEpg= github.com/ONSdigital/dp-frontend-models v1.1.0/go.mod h1:TT96P7Mi69N3Tc/jFNdbjiwG4GAaMjP26HLotFQ6BPw= -github.com/ONSdigital/dp-frontend-models v1.10.1 h1:fHXZB/MwcS7pR2MySFvAhkzZpMSaKyhYb6v80cRKVhI= -github.com/ONSdigital/dp-frontend-models v1.10.1/go.mod h1:K4n0EwATkzbuWzSajBHja+uc9zvqnKiq6WtUwLav4Kg= github.com/ONSdigital/dp-healthcheck v0.0.0-20200131122546-9db6d3f0494e/go.mod h1:zighxZ/0m5u7zo0eAr8XFlA+Dz2ic7A1vna6YXvhCjQ= github.com/ONSdigital/dp-healthcheck v1.0.5/go.mod h1:2wbVAUHMl9+4tWhUlxYUuA1dnf2+NrwzC+So5f5BMLk= github.com/ONSdigital/dp-healthcheck v1.1.0/go.mod h1:vZwyjMJiCHjp/sJ2R1ZEqzZT0rJ0+uHVGwxqdP4J5vg= @@ -35,8 +33,6 @@ github.com/ONSdigital/dp-net/v2 v2.6.0 h1:orxdb0SVDrJgz/zma0QXgQCAGJdLDhp6g2XqH6 github.com/ONSdigital/dp-net/v2 v2.6.0/go.mod h1:4/2ZyId//hFa7AtbbRNQctY729C1Vbw4UEdOliRvpZI= github.com/ONSdigital/dp-rchttp v0.0.0-20190919143000-bb5699e6fd59/go.mod h1:KkW68U3FPuivW4ogi9L8CPKNj9ZxGko4qcUY7KoAAkQ= github.com/ONSdigital/dp-rchttp v0.0.0-20200114090501-463a529590e8/go.mod h1:821jZtK0oBsV8hjIkNr8vhAWuv0FxJBPJuAHa2B70Gk= -github.com/ONSdigital/dp-renderer v1.58.0 h1:GBGKiFFtP91Q6XJ4VwYlKUh8LPjpNRNVAaDk1Mkmc0Q= -github.com/ONSdigital/dp-renderer v1.58.0/go.mod h1:aEgUZoxxw4vy5QG/1WYid3ApbLkaakbyn3/CqIGR/yE= github.com/ONSdigital/dp-renderer v1.62.0 h1:RMIYXulgOICDKz0SivAgcpUsECIVYyNOP3rH2rgss0U= github.com/ONSdigital/dp-renderer v1.62.0/go.mod h1:aEgUZoxxw4vy5QG/1WYid3ApbLkaakbyn3/CqIGR/yE= github.com/ONSdigital/dp-topic-api v0.16.0 h1:SScnzJ9uaVxBoXECAbVOsaqLmGqjss7UPdyVLu9L9C8= diff --git a/handlers/feedback.go b/handlers/feedback.go index 4b91b63..e195a56 100644 --- a/handlers/feedback.go +++ b/handlers/feedback.go @@ -14,6 +14,8 @@ import ( "github.com/ONSdigital/dp-frontend-feedback-controller/interfaces" "github.com/ONSdigital/dp-frontend-feedback-controller/model" dphandlers "github.com/ONSdigital/dp-net/v2/handlers" + "github.com/ONSdigital/dp-renderer/helper" + core "github.com/ONSdigital/dp-renderer/model" "github.com/ONSdigital/log.go/v2/log" "github.com/gorilla/schema" ) @@ -57,11 +59,11 @@ func feedbackThanks(w http.ResponseWriter, req *http.Request, url, errorType str } } p.Type = "feedback" - p.Metadata.Title = "Thank you" + p.Metadata.Title = helper.Localise("FeedbackThanks", lang, 1) p.ErrorType = errorType p.PreviousURL = url - // returnTo is redered on page so needs XSS protection + // returnTo is rendered on page so needs XSS protection returnTo := html.EscapeString(req.URL.Query().Get("returnTo")) if returnTo == "Whole site" { returnTo = wholeSite @@ -69,7 +71,7 @@ func feedbackThanks(w http.ResponseWriter, req *http.Request, url, errorType str returnTo = url } - p.Metadata.Description = returnTo + p.ReturnTo = returnTo rend.BuildPage(w, p, "feedback-thanks") } @@ -86,16 +88,22 @@ func getFeedback(w http.ResponseWriter, req *http.Request, url, errorType, descr p := model.Feedback{ Page: basePage, } + p.Breadcrumb = []core.TaxonomyNode{ + { + Title: "Home", + URI: "/", + }, + } var services = make(map[string]string) - services["cmd"] = "Customising data by applying filters" - services["dev"] = "ONS developer website" + services["cmd"] = "customising data by applying filters" + services["dev"] = "ONS developer" p.ServiceDescription = services[req.URL.Query().Get("service")] p.Language = lang p.Type = "feedback" - p.Metadata.Title = "Feedback" + p.Metadata.Title = helper.Localise("FeedbackTitle", lang, 1) p.Metadata.Description = url ctx := context.Background() cfg, err := config.Get() @@ -179,7 +187,7 @@ func addFeedback(w http.ResponseWriter, req *http.Request, rend interfaces.Rende returnTo = "https://www.ons.gov.uk" } - redirectURL := "/feedback/thanks?returnTo=" + returnTo + redirectURL := fmt.Sprintf("/feedback/thanks?returnTo=%s", returnTo) http.Redirect(w, req, redirectURL, http.StatusMovedPermanently) } diff --git a/handlers/feedback_test.go b/handlers/feedback_test.go index 42a5e74..f343c1c 100644 --- a/handlers/feedback_test.go +++ b/handlers/feedback_test.go @@ -14,8 +14,9 @@ import ( cacheHelper "github.com/ONSdigital/dp-frontend-cache-helper/pkg/navigation/helper" "github.com/ONSdigital/dp-frontend-feedback-controller/email/emailtest" "github.com/ONSdigital/dp-frontend-feedback-controller/interfaces/interfacestest" + "github.com/ONSdigital/dp-frontend-feedback-controller/mocks" "github.com/ONSdigital/dp-frontend-feedback-controller/model" - "github.com/ONSdigital/dp-frontend-models/model/feedback" + "github.com/ONSdigital/dp-renderer/helper" coreModel "github.com/ONSdigital/dp-renderer/model" topicModel "github.com/ONSdigital/dp-topic-api/models" @@ -23,6 +24,7 @@ import ( ) func Test_getFeedback(t *testing.T) { + helper.InitialiseLocalisationsHelper(mocks.MockAssetFunction) Convey("Given a request without a query string", t, func() { req := httptest.NewRequest("GET", "http://localhost", nil) w := httptest.NewRecorder() @@ -92,7 +94,7 @@ func Test_getFeedback(t *testing.T) { Convey("When getFeedback is called", func() { getFeedback(w, req, url, errorType, description, name, email, lang, mockRenderer, mockNagivationCache) Convey("Then the page model is sent to the renderer", func() { - var expectedPage feedback.Page + var expectedPage model.Feedback expectedPage.Language = "en" expectedPage.Metadata.Title = "Feedback" expectedPage.PreviousURL = url @@ -110,6 +112,7 @@ func Test_getFeedback(t *testing.T) { } func Test_addFeedback(t *testing.T) { + helper.InitialiseLocalisationsHelper(mocks.MockAssetFunction) Convey("Given a valid request", t, func() { req := httptest.NewRequest("GET", "http://localhost?description=whatever", nil) w := httptest.NewRecorder() @@ -349,6 +352,7 @@ func Test_addFeedback(t *testing.T) { } func Test_feedbackThanks(t *testing.T) { + helper.InitialiseLocalisationsHelper(mocks.MockAssetFunction) lang := "en" Convey("Given a valid request", t, func() { req := httptest.NewRequest("GET", "http://localhost", nil) @@ -414,7 +418,7 @@ func Test_feedbackThanks(t *testing.T) { feedbackThanks(w, req, url, errorType, mockRenderer, mockNagivationCache, lang) Convey("Then the handler sanitises the request text", func() { dataSentToRender := mockRenderer.BuildPageCalls()[0].PageModel.(model.Feedback) - returnToUrl := dataSentToRender.Metadata.Description + returnToUrl := dataSentToRender.ReturnTo So(returnToUrl, ShouldEqual, "<script>alert(1)</script>") }) }) diff --git a/mocks/mocks.go b/mocks/mocks.go new file mode 100644 index 0000000..5b4d7aa --- /dev/null +++ b/mocks/mocks.go @@ -0,0 +1,16 @@ +package mocks + +import "strings" + +// English only as no Welsh translations exist +var enLocale = []string{ + "[FeedbackTitle]", + "one = \"Feedback\"", + "[FeedbackThanks]", + "one = \"Thank you\"", +} + +// MockAssetFunction returns mocked toml []bytes +func MockAssetFunction(name string) ([]byte, error) { + return []byte(strings.Join(enLocale, "\n")), nil +} diff --git a/model/model.go b/model/model.go index 9d11cb3..6c5c402 100644 --- a/model/model.go +++ b/model/model.go @@ -11,5 +11,6 @@ type Feedback struct { Email string `json:"email"` ErrorType string `json:"error_type"` PreviousURL string `json:"previous_url"` + ReturnTo string `json:"return_to"` ServiceDescription string `json:"service_description"` } From 1b48139b2567f36f6928d0f4ca66ba7e874a2716 Mon Sep 17 00:00:00 2001 From: Nick Lupton <19624419+mr-nick17@users.noreply.github.com> Date: Tue, 18 Apr 2023 15:13:56 +0100 Subject: [PATCH 03/19] 1599 add form validation --- assets/locales/service.cy.toml | 6 +- assets/locales/service.en.toml | 6 +- assets/templates/feedback.tmpl | 160 +++++++++++++++++++-------------- handlers/feedback.go | 121 +++++++++++++++++++------ handlers/feedback_test.go | 39 ++++---- model/model.go | 5 +- 6 files changed, 214 insertions(+), 123 deletions(-) diff --git a/assets/locales/service.cy.toml b/assets/locales/service.cy.toml index 77bf858..13370d1 100644 --- a/assets/locales/service.cy.toml +++ b/assets/locales/service.cy.toml @@ -33,9 +33,9 @@ one = "This specific page" description = "Enter URL or name of the page" one = "Enter URL or name of the page" -[FeedbackWhatOptHintSpecificPage] -description = "{{.ServiceDescription}}" -one = "({{.arg0}})" +[FeedbackChooseType] +description = "Choose a type" +one = "Choose a type" [FeedbackWhatOptNewService] description = "This {{.ServiceName}} service" diff --git a/assets/locales/service.en.toml b/assets/locales/service.en.toml index 75ba56c..4a43cf8 100644 --- a/assets/locales/service.en.toml +++ b/assets/locales/service.en.toml @@ -33,9 +33,9 @@ one = "This specific page" description = "Enter URL or name of the page" one = "Enter URL or name of the page" -[FeedbackWhatOptHintSpecificPage] -description = "{{.ServiceDescription}}" -one = "({{.arg0}})" +[FeedbackChooseType] +description = "Choose a type" +one = "Choose a type" [FeedbackWhatOptNewService] description = "This {{.ServiceName}} service" diff --git a/assets/templates/feedback.tmpl b/assets/templates/feedback.tmpl index c99c495..e3ed2cc 100644 --- a/assets/templates/feedback.tmpl +++ b/assets/templates/feedback.tmpl @@ -1,84 +1,94 @@
{{ template "partials/breadcrumb" . }} + {{ if .Page.Error.Title }} + {{ template "partials/error-summary" .Page.Error }} + {{ end }}

{{- localise "FeedbackTitle" .Language 1 $.Metadata.Title -}}

{{- localise "FeedbackDesc" .Language 1 -}}

-
- - {{- localise "FeedbackTitleWhat" .Language 1 -}} - -
-
-
- - -
-
- {{ if .ServiceDescription }} -
-
- - -
-
+ {{ if or (.IsURLErr) (.IsRadioErr) }} +
+ + {{ localise "Error" .Language 1 }}: + +
+

+ + {{ if .IsURLErr }} + {{- localise "FeedbackWhatEnterURL" .Language 1 -}} + {{ else }} + {{- localise "FeedbackChooseType" .Language 1 -}} + {{ end }} + +

{{ end }} - {{ if .PreviousURL }} -
-
- - - -
- - +
+ + {{- localise "FeedbackTitleWhat" .Language 1 -}} + +
+
+
+ +
-
- {{ else }} -
-
- - - -
- - + {{ if .ServiceDescription }} +
+
+ + +
+
+ {{ end }} +
+
+ + + +
+ + +
+
+ {{ if or (.IsURLErr) (.IsRadioErr) }} +
+
+ {{ end }} + {{ if .IsFeedbackErr }} +
+ + {{ localise "Error" .Language 1 }}: + +
+

+ {{- localise "FeedbackAlertEntry" .Language 1 -}} +

{{ end }} +
+ + {{- localise "FeedbackHintEntry" .Language 1 -}} + +
+ {{ if .IsFeedbackErr }} +
-
-
- - {{- localise "FeedbackHintEntry" .Language 1 -}} - -
+ {{ end }}
{{- localise "FeedbackTitleReply" .Language 1 -}}

{{- localise "FeedbackDescReply" .Language 1 -}}

@@ -87,10 +97,24 @@
-
- - -
+ {{ if .IsEmailErr }} +
+ + {{ localise "Error" .Language 1 }}: + +
+

+ {{- localise "FeedbackAlertEmail" .Language 1 -}} +

+ {{ end }} +
+ + +
+ {{ if .IsEmailErr }} +
+
+ {{ end }}