Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DDLS-387 Pull the NDR report through to the reports page #1734

Merged
merged 45 commits into from
Dec 9, 2024
Merged

Conversation

iqpalm
Copy link
Contributor

@iqpalm iqpalm commented Nov 13, 2024

Purpose

As a multi-client deputy with a current NDR, I want to be able to access the NDR from the reports page, so that I can complete or review NDRs

When we implemented the multi-client pathway, we removed the NDR page for these users. Some users in future may need to be able to access and complete NDRs.

Fixes DDLS-387

Approach

The approach taken has been to implement the change for all Lay users, single and multi clients. This means the "/ndr" endpoint has been deprecated.

  1. I deleted the if statement that checks for isNdrEnabled as it is no longer required in getCorrectRouteIfDifferent in Redirector. The lay_home will be accessed whether isNdrEnabled is true or false. The report_create in getLayDeputyHomepage is only required if the NDR has not been enabled else it will show the NDR on the report page. The RedirectorTest file has been amended to delete the tests for ndr_index as the Redirector no longer checks for these scenarios.

  2. The functionality for ndr_index has been moved into lay_home page. A check is made against all users attached to a client to find out if isNdrEnabled is set to true (i.e the checkbox has been ticked in admin app). Depending on this a flag is set (ndrEnabled) that determines if a NDR or a report will be shown.

    The twig template includes a reference to the NDR index twig file if ndrEnabled is set to true.

    The ndr fixture createLayNdrNotStarted in FixtureHelper required a change where isPrimary is now set to true. This was
    required as it was logging out due to the isPrimary check in lay_home endpoint.

  3. During the NDR submission journey the app was failing after the post user research step as the client id was required from the NDR by the lay_home endpoint. This was solved by including the JMS groups ['ndr-client', 'client-id'] to ensure the app returns to the correct client.

  4. Previously, the pattern was to bring back the client and then access the NDR. However, this has been amended to use the ndrId from the route to access the NDR and then bring back the client. This is cleaner code especially for multi client deputies.

  5. Deprecate ndr_index endpoint and redirect to hompage. Amend ndr_index references to lay_home and also include clientId.

  6. Create behat tests to test NDR enabled checkbox results in correct report on reports page. I tried to do in 1 test but failed in pipeline so decided to do in 2 seperate tests.

  7. Included parameters to breadcrumb for ndr review which was showing as deputyFirstname deputyLastname

  8. Added in functionality for multi client breadcrumbs for NDR journey (similar to report journey)

Learning

Any tips and tricks, blog posts or tools which helped you. Plus anything notable you've discovered about DigiDeps

Checklist

  • I have performed a self-review of my own code
  • I have updated documentation (Confluence/ADR/tech debt doc) where relevant
  • I have added tests to prove my work
  • The product team have approved these changes
  • I have checked my work for potential security issues and refered to the OWASP top 10

Frontend

  • I have run an in-browser accessibility test (e.g. WAVE, Lighthouse)
  • There are no deprecated CSS classes noted in the profiler
  • Translations are used and the profiler doesn't identify any missing
  • Any links or buttons added are screen reader friendly and contextually complete
  • If adding GA events, I have updated or checked the existing category or label values

@iqpalm iqpalm marked this pull request as ready for review November 26, 2024 09:24
@iqpalm iqpalm requested a review from a team as a code owner November 26, 2024 09:24
@iqpalm iqpalm requested a review from Raffers December 2, 2024 16:11
@iqpalm iqpalm merged commit 6a83326 into main Dec 9, 2024
37 checks passed
@iqpalm iqpalm deleted the DDLS-387 branch December 9, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants