Feature/1599 update feedback controller #48
Conversation
|
Have reviewed code, design and accessibility and all seems fine - will just review the tests and their coverage after meetings this morning. |
There was a problem hiding this comment.
I don't have any major concerns about this and it reflects a really positive upshift in the quality of this application. I've highlighted a few bits on the PR which if you want to tackle / discuss that's great, but if you feel like it continues to add weight to this then happy for this PR to proceed as is.
config/config.go
Outdated
| IsPublishingMode bool `envconfig:"IS_PUBLISHING_MODE"` | ||
| IsPublishing bool `envconfig:"IS_PUBLISHING"` |
There was a problem hiding this comment.
Another card for the secrets update or want to cover it under this?
There was a problem hiding this comment.
Secrets have already been done 😄 https://github.com/ONSdigital/dp-configs/pull/1285
handlers/feedback_test.go
Outdated
| @@ -199,12 +199,12 @@ func Test_addFeedback(t *testing.T) { | |||
| So(len(mockRenderer.BuildPageCalls()), ShouldEqual, 0) | |||
| }) | |||
|
|
|||
| Convey("Then the email sender is called", func() { | |||
| Convey("Then the email sender is not called", func() { | |||
| So(len(mockSender.SendCalls()), ShouldEqual, 0) | |||
| }) | |||
|
|
|||
| Convey("Then a 500 response is returned", func() { | |||
There was a problem hiding this comment.
Minor update for Convey description now it's a 400.
What
Rewriting the app
dp-renderer(template changes within app mean fields and inputs can be imported 🎉 )mapper.go+ covered logic with unit testsmapper.goandfeedback.go)validateForm(with unit tests)isPositivelogicgotov1.20.4How to review
Sense check
Tests pass
Image review
Form using the ONS design system
Example of validation error
Who can review
!me