[Injected (TeamSites) Header/Footer] Complete implementation of authentication code

randimays commented 9 months ago

Status

[2024-05-23] Moved to Stretch/Next sprint. This ticket should be picked up after the Design work (va-icon & V1/V3 components) is completed. Randi will access and there may be more tickets created due to the need to refresh the code, and if anything else is found during assessment.

Description

~In support of #16815, the Design System team will be contributing the code for the TeamSites header authentication functionality. We have added the static Sign in button, but the sign in modal and authenticated header will be put together by DST.~

In order to complete our effort to completely separate the VA.gov header and footer code from the TeamSites header and footer code, we need this authentication piece added to the code in PR 27590 in vets-website.

Matt Dingee has shared that he believes Micah did some discovery work around sticking with React for the auth portion. If he and Randi sync up on that (which they may have already done) then we should be all set.

The Design Team is heads down with other work and this isn't on their radar, so the PW team will be completing the changes.

WORK NEEDED

https://github.com/department-of-veterans-affairs/vets-design-system-documentation/issues/2477 -- epic tracking react injection (Can't add as a dependency without being a member of their repo.)

Estimate

The 5 points indicated on this ticket represent the work for the Public Websites team (see acceptance criteria). The Design System team will be adding the authentication code. Optimistically, this is mostly a tracking ticket for the DST's work until it's time to test for the production release.

User story

AS A Veteran I WANT to see a fully functional, not regressed, copy of the VA.gov header and footer on TeamSites pages SO THAT I have a consistent visual and functional experience on all VA websites

Engineering notes / background

The work-in-progress branch in vets-website is 16815-dup-header-for-teamsites. This branch currently (as of 2/20/2024) contains all of the code for the TeamSites header and footer in the src/applications/proxy-rewrite folder with the exception of the authentication functionality (sign in modal, authenticated header, sign out button).

Noting here: we have some heavy limitations with testing TeamSites. We can test locally, but then we can't test again until we get to production. It is important that we have as much confidence as possible in our local instance before deploying to production.

Quality / testing notes

Manual testing and accessibility testing will need to be conducted, as well as adding / adjusting any existing unit tests and end-to-end tests. Note: end-to-end tests will be very limited as we are not able to easily render the TeamSites instances in pipelines.

Acceptance criteria

[x] Accessibility review (by Laura)
[x] Manual testing on all supported browsers/platforms (ngrok may be useful for mobile testing)
- [x] Chrome
- [x] Safari
- [x] Firefox
- [x] Android Chrome
- [x] Android Firefox
- [ ] iOS Safari
- [ ] iOS Chrome
[x] All unit/e2e tests passing and none skipped
[x] Documentation completely up to date
[x] Plan in place for rollback of production deploy if needed

randimays commented 9 months ago

@jilladams @FranECross Ticket here is to track the DST work for authentication and getting the fonts to work. Not sure what Epic this should be in.

Closing #16815.

FranECross commented 9 months ago

I'm going to make a separate epic just for the Injected Header work and move tickets (and this one) to that epic so we can close it after the Design team does their work. The other new header epic can remain in blocked. Thank you! cc @jilladams

randimays commented 9 months ago

Removed the font portion of the ticket description as Jami was able to find the fix for us! I had the font definitions in the wrong part of the code; they just needed to be moved 😆

randimays commented 7 months ago

https://dsva.slack.com/archives/CBU0KDSB1/p1712240169266659

Per the above Slack thread, we have hardcoded the Contact us and My VA (authenticated menu) to always direct to https://va.gov/contact-us and https://va.gov/my-va accordingly, regardless of environment.

Once the injected header/footer code is split off from va.gov, we can localize these links so when we are in lower environments, the pages will load in those local environments for testing.

i.e., point to /contact-us as the href so it'll go to https://staging.va.gov/contact-us instead of https://va.gov/contact-us (from staging).

FranECross commented 5 months ago

@randimays I scooted this ticket over to Stretch/Next Sprint and will ensure it's ordered there correctly. I also added a Status of " Moved to Stretch/Next sprint. This ticket should be picked up after the Design work (va-icon & V1/V3 components) is completed. Randi will access and there may be more tickets created due to the need to refresh the code, and if anything else is found during assessment." Thanks! cc @jilladams

randimays commented 5 months ago

Pulling in as stretch.

randimays commented 5 months ago

DST did some research on how we would put together the authentication piece for the separated injected header. Here's the ticket: https://github.com/department-of-veterans-affairs/vets-design-system-documentation/issues/2477. I don't have access to the Confluence discovery doc they linked in the ticket, but I looked at the branch with the proof-of-concept that Ian provided. It uses the sign in modal directly from the platform/user/authentication folder of VW: https://github.com/department-of-veterans-affairs/vets-website/blob/16815-with-sign-in-modal/src/applications/proxy-rewrite/partials/signInModal.jsx.

This is a "simple" React implementation similar to what we did for the search functionality. But if DST (or any other team) wants to eventually convert the authentication / login modal pieces to web components, this implementation would block that effort.

I have a Slack thread with Ian Harrison on the DST to ask about future web component plans: https://dsva.slack.com/archives/C01DBGX4P45/p1717776395974439

CC @laflannery

laflannery commented 5 months ago

@randimays This is the Confluence Document DST-Discovery_ Viability of React sign-in modal inside injected static header-070624-161905.pdf

randimays commented 5 months ago

This ticket will carry over to the next sprint. I have added in the login modal but it has some focus and styling issues that need to be addressed. I also have other things that broke when I updated the code with the latest from main, and I need to re-test everything across breakpoints.

randimays commented 5 months ago

Back in September 2023, the login modal was updated to use a va-telephone web component for the help desk phone numbers. When that change was made, the injected header's login modal was not tested, so I think it has been broken since then.

In prod right now at va.gov/health:

Since I'm pulling in the same login modal as VA.gov for the separate injected header code, I have to fix this issue first. Working on getting that merged today and then I can resume injected header work.

PR: https://github.com/department-of-veterans-affairs/vets-website/pull/30329

randimays commented 5 months ago

Update: most of the coding / testing is complete. Jill created a test plan during our 16th min discussion in scrum today: https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18358. Jerry is working on identifying test cases for us to get rolling on review. My branch is not yet ready for review. I need to review the code in the PR to be sure nothing unintended crept in, and make sure the CI is passing. There are some URL sanitization warnings on the PR that I also need to figure out how to address. I'm hoping the branch will be stable and ready for review early next week.

randimays commented 4 months ago

This work is ready for review. The PR is here: https://github.com/department-of-veterans-affairs/vets-website/pull/27590

The QA / test plan ticket is here: https://github.com/department-of-veterans-affairs/va.gov-cms/issues/18358

This will carry over to Sprint 7 as it will take some time to review and get merged / deployed.

randimays commented 4 months ago

End of sprint update: the first round of testing / feedback was complete, but a discovery was made for a significant regression that also required significant rework. I am still working on the code changes, and this will certainly carry over. I estimate there are about 2 points of engineering/manual testing work remaining for me, and then I can pass this back to the testing crew for another look.

randimays commented 4 months ago

Adding an extra 1-2 points of engineering work; I also need to update the unit tests for everything that was refactored.

randimays commented 3 months ago

Update: Chris and Laura have approved the latest round of changes. I'm waiting for @thejordanwood to give us the greenlight. She is out on wellness currently so this will carry over to the next sprint.

randimays commented 3 months ago

Code deployed successfully this afternoon. Validated all links and functionality on:

We have identified one issue with fonts not working correctly on https://cem.va.gov. It is another CORS problem (similar to the one from back in May). I have a Platform ticket open to get some help troubleshooting. Fonts are working correctly on the www domain: https://www.cem.va.gov.

Slack thread of CORS/font discussion: https://dsva.slack.com/archives/C52CL1PKQ/p1721927778370739?thread_ts=1721927672.794809&cid=C52CL1PKQ

@jilladams @FranECross Should we keep this ticket open until we figure out the font issue? I'm not sure there's more we can do from a front end perspective to resolve this. Perhaps a ticket to track the known bug?

jilladams commented 3 months ago

Yep, agree. Closing, will cut a stub for the fonts.

department-of-veterans-affairs / va.gov-cms