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

87799 ARF migrate 21a into ARP #30835

Closed
wants to merge 7 commits into from

Conversation

williamphelps13
Copy link
Contributor

@williamphelps13 williamphelps13 commented Jul 15, 2024

Are you removing, renaming or moving a folder in this PR?

  • No, I'm not changing any folders (skip to Summary and delete the rest of this section)
  • Yes, I'm removing, renaming or moving a folder

Note: There is a PR to delete the 21a application entry in content-build registry.json that must be merged before this one

Summary

I aimed to keep the changes very minimal in this PR after I realized that we were running into some potential complexity.

So first commit just drags and drops the accreditation/21a folder into accredited-representative-portal.

Second commit aimed to make form 21a the visible content of the accredited-representative-portal in alignment with the V3 designs.

As such it reverts our app to react-router v3 since that is what form apps use by default (but of course feel free to revert it back to compat).

The url was going to be /attorney-claims-agent-form-21a but the third commit reverts that to not clash with identity code that had it as /representative.

The url is still /attorney-claims-agent-form-21a in the content build and DevOps PRs that I also pushed up as part of this migration:

Summarize the changes that have been made to the platform: All changes are in files owned by ARF

  • Migrate into 21a unchanged - 44 files
  • Migrate and update imports - 2 files
    • src/applications/accredited-representative-portal/accreditation/21a/config/form.js
    • src/applications/accredited-representative-portal/reducers/form21aSaveInProgress.js
  • Delete - 5 files
    • Removing key application files since 21a will now use the ARP versions
      • src/applications/accreditation/21a/app-entry.jsx
      • src/applications/accreditation/21a/manifest.json
      • src/applications/accreditation/21a/routes.jsx
      • src/applications/accreditation/21a/sass/21a.scss
    • Fixing accidental duplication in previous PR
      • src/applications/accreditation/21a/components/YourCharacterReferencesDescription.jsx - correctly placed file is src/applications/accredited-representative-portal/accreditation/21a/components/07-character-references-chapter/YourCharacterReferencesDescription.jsx
  • Update ARP files to use react-router V3 - this was done because the form save in progress platform helper and the files it uses all use react-router V3 src/platform/forms/save-in-progress/helpers.js - 4 files
    • src/applications/accredited-representative-portal/app-entry.jsx
    • src/applications/accredited-representative-portal/components/common/Header/MobileHeader/MobileLogoRow.jsx
    • src/applications/accredited-representative-portal/components/common/Header/WiderThanMobileHeader/WiderThanMobileLogoRow.jsx
    • src/applications/accredited-representative-portal/components/common/Header/WiderThanMobileHeader/WiderThanMobileMenu.jsx
  • Update ARP files to integrate 21a - 3 files
    • src/applications/accredited-representative-portal/manifest.json
    • src/applications/accredited-representative-portal/reducers/index.js
    • src/applications/accredited-representative-portal/routes.jsx
  • Add Header and Footer to 21a App
    • src/applications/accredited-representative-portal/accreditation/21a/containers/App.jsx
  • Which team do you work for, does your team own the maintenance of this component?
    • Accredited Representative Facing Team

Related issue(s)

Testing done

  • Describe what the old behavior was prior to the change
    • Before this change the 21a used the VA.gov header and footer and therefore the VA.gov user
    • The form was functional
  • Describe the steps required to verify your changes are working as expected

Screenshots

Mobile

Before

https://staging.va.gov/representative
Screenshot 2024-07-15 at 9 06 16 AM
https://staging.va.gov/representative/poa-requests
Screenshot 2024-07-15 at 9 06 32 AM

After

Still in progress since need to fix this error:
Screenshot 2024-07-15 at 9 08 49 AM
If I update src/applications/accredited-representative-portal/accreditation/21a/containers/App.jsx to:

<Header />
<h1>Form 21a</h1>
{/* <RoutedSavableApp formConfig={formConfig} currentLocation={location}>
  {children}
</RoutedSavableApp> */}
<Footer />
Screenshot 2024-07-15 at 9 07 51 AM

Desktop

Before

Screenshot 2024-07-15 at 9 10 29 AM Screenshot 2024-07-15 at 9 10 47 AM

After

Same Error
If I update src/applications/accredited-representative-portal/accreditation/21a/containers/App.jsx to:
Screenshot 2024-07-15 at 9 11 27 AM

Acceptance criteria

Quality Assurance & Testing

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (link to documentation *if necessary)
  • Screenshot of the developed feature is added
  • Accessibility testing has been performed

Authentication

  • Did you login to a local build and verify all authenticated routes work as expected with a test user

Requested Feedback

  • Considered creating our own save in progress helper functions so that we don't have to move our app back to react-router V3. There could also be a better way to handle this dependency clash.
  • Unknown what is causing error:
Screenshot 2024-07-15 at 9 14 01 AM - Still need to update (21a already has its own e2e tests so this may not be necessary), comment out, or skip our poa-requests e2e tests

@va-vfs-bot va-vfs-bot temporarily deployed to master/87799-arf-migrate-21a-into-arp/87799-arf-migrate-21a-into-arp July 16, 2024 16:32 Inactive
nihil2501 and others added 3 commits July 19, 2024 12:30
Fixing this error unlocks the next error which I believe exposes a more meaningful incompatibility that SIP experiences when the the app's state has not been set up by the platform user component. The key idea here is that in progress forms are fundamentally connected to a user. to integrate in progress forms into our app we'll need to introduce and expose user state that in progress forms understands
…t-of-veterans-affairs/vets-website into 87799-arf-migrate-21a-into-arp
@va-vfs-bot va-vfs-bot temporarily deployed to master/87799-arf-migrate-21a-into-arp/87799-arf-migrate-21a-into-arp July 19, 2024 19:30 Inactive
@nihil2501
Copy link
Contributor

This is really hairy. With this version I do get the SIP functionality (pointing it at this backend PR), but I don't quite have a sense of how far off we are from the integration really being correct. It seems like there are a lot of really careful cases for course correcting out of many really ephemeral logged in states that are interspersed throughout the various interactions and pages of SIP. I have a couple of other information dumps that I hope can orient anyone trying to make the next earnest effort on this.

Dump 1

In Progress Forms Protocol

For an app to conform to the in progress forms protocol, it must implement a particular set of interactions. These interactions break down into four possible categories which are the combinatorial product of the interactions' possible Redux media (state or action) and directions (inbound to or outbound from the form component tree).

inbound, state

Redux state that is read inside the form component tree and updated by reducers that are not registered by the in progress form system.

- navigation
  - showLoginModal
- scheduledDowntime
- user
  - login
    - currentlyLoggedIn
  - profile
    - accountUuid
    - loading
    - prefillsAvailable
    - savedForms
    - verified

outbound, state

Redux state that is read outside the form component tree and updated by reducers that are registered by the in progress form system.

- form
  - additionalRoutes
  - autoSavedStatus

^ These are used to calculate a shouldConfirmLeavingForm which would communicate: "If you sign in now, you’ll lose any information you’ve filled in.". If we don't have unauthed submission and our form takes a long time, we're probably making them log in before filling out the form. Even so, this path might pop up in some edge case again with expiring sesions.

inbound, action

Redux actions that are dispatched outside the form component tree and reacted to by reducers that are registered by the in progress form system.

Nothing here.

outbound, action

Redux actions that are dispatched inside the form component tree and reacted to by reducers that are not registered by the in progress form system.

- TOGGLE_LOGIN_MODAL
- REMOVING_SAVED_FORM_SUCCESS
- UPDATE_PROFILE_FIELDS
- LOG_OUT

^ These need some more scrutiny. It looks like UPDATE_PROFILE_FIELDS only happens from the ApplicationStatus SIP component that we don't use here. LOG_OUT is probably still a gap and maybe because we're not doing anything for session expiration. Same for TOGGLE_LOGIN_MODAL. I think we're handling REMOVING_SAVED_FORM_SUCCESS fine.

Dump 2

  • Bad flow outside ARP on 400: http://localhost:3001/auth/login/callback/?auth=fail&code=400&request_id=2bced83c-6950-446f-bc02-5699509f7e3a
  • Comment dead ARP components somewhere?
  • Does verified matter for SIP 21a?
  • FYI, we load almost 1k feature togglers
  • Maintenance windows. But why not backend statuses? Is that meant to be handled in user nav?
  • Review GenericError has "Go Back to VA.gov" and href "/" hardcoded.
  • Need to check prefillsAvailable - pretty sure it's not relevant because it's just from account details which we maybe want to forego here?
  • Will we even leverage the downtime integration in our SIP integration at all?
  • Token refresh doesn't seem to work: /refreshnull due to null type argument in oauth utilities / constants code. I didn't really investigate this one
  • Prevent unauthed start with hideUnauthedStartLink? It's a long form...
  • Failed and then successful submission maintains error status in form metadata in client state? Just happened to see this, but who knows how many sequences of interactions would yield something weird for us.

@@ -77,6 +77,8 @@ export const VA_FORM_IDS_IN_PROGRESS_FORMS_API = Object.freeze({
// 526 save-in-progress endpoint that adds an `updatedRatedDisabilities` array
// to the saved form data from /v0/disability_compensation_in_progress_forms/
[VA_FORM_IDS.FORM_21_526EZ]: '/v0/disability_compensation_in_progress_forms/',
[VA_FORM_IDS.FORM_21A]:
'/accredited_representative_portal/v0/in_progress_forms/',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we may need to override inProgressApi if our module is hosted separately (i.e. api.representative.va.gov).

@nihil2501 nihil2501 closed this Aug 15, 2024
@nihil2501
Copy link
Contributor

Supplanted by #31352

@gabezurita gabezurita deleted the 87799-arf-migrate-21a-into-arp branch September 24, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benefits-accredited-rep-facing Label for the OCTO Slack team #benefits-representative-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants