Submit Check your answers page #811

Demwunz · 2024-03-12T14:30:23Z

https://eaflood.atlassian.net/browse/WATER-4264

This page allows the user to submit their answers.

The "check your answers" page will submit the data to the check your answers service which in turn will check the licence.

If the licence is valid, the user will be directed to the approved page.

If the licence is invalid (expired, lapsed or revoked), the service will throw an error.

Cruikshanks

I've suggested merging FetchLicenceService and part of SubmitCheckYourAnswers into a new CheckLicenceEnded service that is also going to affect tests.

Happy to pair on this or jump on to get this over the line or if it's not clear what I'm suggesting 😜

Cruikshanks · 2024-03-14T10:18:20Z

app/services/return-requirements/check-your-answers.service.js

+
+  return {
+    activeNavBar: 'search',
+    licence_id: session?.data?.licence?.licenceRef,


There might be a valid reason. But could this not also just be licenceRef?

Suggested change

licence_id: session?.data?.licence?.licenceRef,

licenceRef: session?.data?.licence?.licenceRef,

Cruikshanks · 2024-03-14T10:21:24Z

app/services/return-requirements/check-your-answers.service.js

I'd avoid using the Optional Chaining (sometimes known as the safe navigation operator) unless we have a situation where we know a property may not be populated.

In the case of this service, the session should be correctly populated. If it isn't it's because someone has purposely tried to access the page with a valid session ID but an invalid session. So, it's own them!

@Cruikshanks ok, that makes sense. I'll remove it and replace with a direct reference.

Cruikshanks · 2024-03-14T10:28:44Z

app/services/return-requirements/fetch-licence.service.js

Looking at SubmitCheckYourAnswers the only thing we need from this service is $ends. In fact, we know SubmitCheckYourAnswers is going to get busier in the near future as it will be doing the work of persisting the return requirements.

So, it might be prudent to combine this service and _checkLicenceEnded() from SubmitCheckYourAnswers into a CheckLicenceEndedService that just returns a true/false. SubmitCheckYourAnswers can then use this result to decide whether to throw the error.

What do you think?

@Cruikshanks It does make sense to use the $ends property. I didn't want to jump the gun given the previous structure of JIRA tickets. I can make the above change you've suggested to save time later.

https://eaflood.atlassian.net/browse/WATER-4264 This page allows the user to submit their answers. Ther page will run submit the data to the check your answers service which in turn will check the licence. If the licence is valid, the user will be directed to the approved page. If the licence is invalid, the service will throw an error.

The fetch-licence.service.test.js was failing because it was getting a stubbed result back when what we wanted was the actual service to be called. The reason it was still stubbed out was because submit-check-your-answers.test.js was stubbing the service outside of a `beforeEach()`. This is the equivalent of stubbing the service as soon as the test suite is loaded (which it always is even if you then just run one test). It's outside the scope of the tests and therefore a `Sinon.restore()` in an `afterEach()` is not going to apply. Putting it inside of a `beforeEach()` means the stubbing now only happens in the scope of the `submit-check-your-answers.service.test.js`.

When dealing with async code unless we need to await the result (as we do in `go()` in order to be able to pass the licence to `_data()`) we should be returning immediately so the calling code then has the option to await or not. The other cleanup is that `_data()` was not doing anything blocking so didn't need to be `async`.

For the existing test that was there we can just let `LicenceHelper.add()` do its thing and don't need to provide any other details. With the other changes in place the test should now pass (works on my machine :-) !)

Cruikshanks

I've not gone through the tests or view yet because I think some things need tidying up in the services first.

Let's deal with those as they'll impact the tests then ping me and I'll check the rest.

app/services/return-requirements/check-licence-ended.service.js

app/services/return-requirements/check-your-answers.service.js

app/services/return-requirements/submit-check-your-answers.service.js

Co-authored-by: Alan Cruikshanks <[email protected]>

…vice.js Co-authored-by: Alan Cruikshanks <[email protected]>

Co-authored-by: Alan Cruikshanks <[email protected]>

…k-your-answers-page-iteration-2-3-of-3

Cruikshanks

Demwunz force-pushed the WATER-4264-returns-not-required-journey-check-your-answers-page-iteration-2-3-of-3 branch 4 times, most recently from ec5c3ed to e88c992 Compare March 12, 2024 15:36

Demwunz marked this pull request as ready for review March 13, 2024 13:29

Demwunz requested review from Cruikshanks and robertparkinson March 13, 2024 13:29

Cruikshanks requested changes Mar 14, 2024

View reviewed changes

Cruikshanks assigned Demwunz Mar 16, 2024

Demwunz requested a review from Cruikshanks March 19, 2024 10:20

Demwunz added the enhancement New feature or request label Mar 19, 2024

Demwunz and others added 12 commits March 19, 2024 16:05

feat(app): check your answers service

62d0835

fix(app): fixed broken tests

4f50823

fix:(app) - updated service test

2acfd29

fix:(app) - added fetch licence test

9bd3300

fix:(app) - removed console log

fc05b7a

Clean up FetchLicenceService test

ba1112a

For the existing test that was there we can just let `LicenceHelper.add()` do its thing and don't need to provide any other details. With the other changes in place the test should now pass (works on my machine :-) !)

fix:(app) - removed redundant tests

9af8930

feat(app): moved check licence expired to new service

6c2e52b

fix(app): simplified check licence

33ff4c0

Demwunz force-pushed the WATER-4264-returns-not-required-journey-check-your-answers-page-iteration-2-3-of-3 branch from 4b94b74 to 33ff4c0 Compare March 19, 2024 16:07

Cruikshanks requested changes Mar 19, 2024

View reviewed changes

Demwunz and others added 5 commits March 20, 2024 09:26

Update app/services/return-requirements/check-licence-ended.service.js

4fbdec2

Co-authored-by: Alan Cruikshanks <[email protected]>

Update app/services/return-requirements/check-licence-ended.service.js

78d7aba

Co-authored-by: Alan Cruikshanks <[email protected]>

Update app/services/return-requirements/check-licence-ended.service.js

c0906cd

Co-authored-by: Alan Cruikshanks <[email protected]>

Update app/services/return-requirements/check-licence-ended.service.js

c43048f

Co-authored-by: Alan Cruikshanks <[email protected]>

Update app/services/return-requirements/check-licence-ended.service.js

53db271

Co-authored-by: Alan Cruikshanks <[email protected]>

Demwunz and others added 6 commits March 20, 2024 09:29

Update app/services/return-requirements/check-licence-ended.service.js

9ae6d57

Co-authored-by: Alan Cruikshanks <[email protected]>

Update app/services/return-requirements/submit-check-your-answers.ser…

46e4b8c

…vice.js Co-authored-by: Alan Cruikshanks <[email protected]>

Update app/services/return-requirements/check-your-answers.service.js

c73673a

Co-authored-by: Alan Cruikshanks <[email protected]>

Merge branch 'main' into WATER-4264-returns-not-required-journey-chec…

85b45b9

…k-your-answers-page-iteration-2-3-of-3

fix(app): cleanup logic after changes

bf77e18

Merge branch 'main' into WATER-4264-returns-not-required-journey-chec…

195a614

…k-your-answers-page-iteration-2-3-of-3

Demwunz force-pushed the WATER-4264-returns-not-required-journey-check-your-answers-page-iteration-2-3-of-3 branch from 4ecf30b to 26b5c7a Compare March 20, 2024 11:03

fix(app): restored licenceId for redirect

98b6e2a

Demwunz force-pushed the WATER-4264-returns-not-required-journey-check-your-answers-page-iteration-2-3-of-3 branch from 26b5c7a to 98b6e2a Compare March 20, 2024 11:12

Cruikshanks approved these changes Mar 20, 2024

View reviewed changes

Demwunz merged commit 2a70d9e into main Mar 20, 2024
6 checks passed

Demwunz deleted the WATER-4264-returns-not-required-journey-check-your-answers-page-iteration-2-3-of-3 branch March 20, 2024 11:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Submit Check your answers page #811

Submit Check your answers page #811

Demwunz commented Mar 12, 2024 •

edited

Loading

Cruikshanks left a comment

Cruikshanks Mar 14, 2024

Cruikshanks Mar 14, 2024

Demwunz Mar 14, 2024

Cruikshanks Mar 14, 2024

Demwunz Mar 14, 2024 •

edited

Loading

Cruikshanks left a comment

Cruikshanks left a comment

	licence_id: session?.data?.licence?.licenceRef,
	licenceRef: session?.data?.licence?.licenceRef,

Submit Check your answers page #811

Submit Check your answers page #811

Conversation

Demwunz commented Mar 12, 2024 • edited Loading

Cruikshanks left a comment

Choose a reason for hiding this comment

Cruikshanks Mar 14, 2024

Choose a reason for hiding this comment

Cruikshanks Mar 14, 2024

Choose a reason for hiding this comment

Demwunz Mar 14, 2024

Choose a reason for hiding this comment

Cruikshanks Mar 14, 2024

Choose a reason for hiding this comment

Demwunz Mar 14, 2024 • edited Loading

Choose a reason for hiding this comment

Cruikshanks left a comment

Choose a reason for hiding this comment

Cruikshanks left a comment

Choose a reason for hiding this comment

Demwunz commented Mar 12, 2024 •

edited

Loading

Demwunz Mar 14, 2024 •

edited

Loading