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

Test VA Notify email in staging and promote to production #92238

Closed
9 tasks done
opticbob opened this issue Sep 5, 2024 · 28 comments
Closed
9 tasks done

Test VA Notify email in staging and promote to production #92238

opticbob opened this issue Sep 5, 2024 · 28 comments
Assignees
Labels
accredited-representation-management-team Accredited Representation Management team backend mvp Initial version of thing

Comments

@opticbob
Copy link

opticbob commented Sep 5, 2024

Background

This is a continuation of Implement New API Endpoint to Trigger VA Notify Email on PDF Download Page#91460. The code part of that ticket is complete so we've split the testing into this ticket so it can be completed separately and the original ticket can be closed. We will have to discuss with VA Notify the best way to test this in staging.

Waiting until we can trigger this download through the UI. The download page and the rep selection piece.

Tasks

  • Test in Staging: Ensure the email template works as expected in the staging environment. Verify that the email is sent correctly and that the correct data is being populated in the template.
  • Make sure organizations display as "Veterans Service Organization"
  • Make sure representatives display as "VSO representative"
  • Make sure claims agents display as "claims agent"
  • Make sure attorneys display as "attorney"
  • Partner with Lindsay to get CAIA's confirmation that everything looks as expected.
  • Copy Template to Production: Once confirmed working in staging and we have CAIA's thumbs up, replicate the email template in the production environment. This may involve manually copying the template into the production tab on notifications.va.gov if there is no automated interface available.

Acceptance Criteria

  • The email is successfully sent via VA Notify using the correct template, with accurate data populated in all fields.
  • The email template is available and functional in the production environment.
@oddball-lindsay
Copy link
Contributor

Hopefully department-of-veterans-affairs/vets-website#32262 will be merged soon, which will open up Appoint in Staging

@opticbob
Copy link
Author

opticbob commented Oct 9, 2024

When going through the appoint experience there were some validation errors in the PDF generation step. Those have been discussed with @cosu419 and will require additional backend work.

@oddball-lindsay
Copy link
Contributor

@opticbob can you share more about what the additional backend work needed is? It sounds like we might need another ticket.

@opticbob
Copy link
Author

@oddball-lindsay Yep! The country picker for the forms library gives us 3 character country codes but our form only supports two so that was failing validations and the claimant zip code suffix validation was missing a detail that was causing it to behave inconsistently from other suffix validations. You can find all the details here: #94683.

@opticbob
Copy link
Author

I'm moving this to Blocked status until #94683 is done and merged.

@opticbob
Copy link
Author

#94683 Has been merged so hopefully it'll be deployed by Monday morning and this ticket will be able to proceed.

@oddball-lindsay
Copy link
Contributor

#94683 is done and merged, but there may be some new things that were uncovered and need to be handled to unblock this ticket, since Colin and Josh saw some validation errors.

Will check with @opticbob tomorrow when he's back, but likely this will be addressed in the draft PR department-of-veterans-affairs/vets-api#18877

@opticbob
Copy link
Author

@oddball-lindsay That's correct. department-of-veterans-affairs/vets-api#18877 should unblock this.

@opticbob
Copy link
Author

The ticket that was blocking this merged today: department-of-veterans-affairs/vets-api#18877. I'll check in the morning if this ticket can proceed.

@opticbob
Copy link
Author

There were still some validation errors when I tried this today. I'll speak to @cosu419 about it tomorrow to come up with a plan.

@opticbob
Copy link
Author

Currently there are a few difference between the data request body being sent by the frontend and the one the backend is expecting. The frontend currently sends the following data like this:

{
  "recordConsent": "Yes, they can access all of these types of records",
  "consentAddressChange": "Yes, they can change my address if it's incorrect or outdated",
  "consentLimits": [],
  "representative": {
    "organizationName": "23563963-e31c-4797-a259-e26b727072e1"
  }
}

The backend is expecting it like this:

{
  "recordConsent": true,
  "consentAddressChange": false,
  "consentLimits": ["DRUG_ABUSE", "HIV", "SICKLE_CELL"],
  "representative": {
    "organizationId": "23563963-e31c-4797-a259-e26b727072e1",
    "id": "b7f9c8d2-3b4a-4d6a-9b6e-2f3e4a2d1c8f"
  }
}

record_consent and consent_address_change are booleans, consent_limits come from a capitalized list, and the representative just uses the organization_id and id.

@oddball-lindsay oddball-lindsay added the mvp Initial version of thing label Oct 23, 2024
@opticbob
Copy link
Author

We thought the previous validation fix PR would fix this but @jvcAdHoc discovered some remaining fixes that are required. Those can be found at #95975 and that work is mostly done.

@opticbob
Copy link
Author

The PR for #95975 has been submitted for another round of review. Once that merges and is deployed I'll be back on this ticket.

@opticbob
Copy link
Author

opticbob commented Nov 1, 2024

The PR for #95975 merged but testing this didn't work. The request is getting a response of 403 Forbidden. It looks like the Cookie and X-CSRF-Token fields were missing. @holdenhinkle wrote up a ticket and quick fix PR that has already been reviewed by @jvcAdHoc so once Platform reviews it and it gets merged I'll try this again.

@opticbob
Copy link
Author

opticbob commented Nov 1, 2024

The 403 Forbidden is fixed and now we can see the actual endpoint errors. Right now the frontend is submitting this:

{
  "firstName": "Josh",
  "representativeType": "VSO Representative",
  "representativeAddress": "",
  "formNumber": "21-22",
  "formName": "Appointment of Veterans Service Organization as Claimant's Representative"
}

and the response we're getting back looks like this:

{
    "errors": [
        "Email address can't be blank",
        "Representative type is not included in the list",
        "Representative name can't be blank",
        "Representative address can't be blank"
    ]
}

Backend changes that need to be made:

  1. We should accept entity_id and entity_type which would be individual or organization. We can use those to look up the name, address, and type.
  2. Along with the first point we should stop accepting representative Type, Address, and Name.

Frontend changes that need to be made:

  1. The email_address needs to be sent in the payload. The email address should be the veteran's unless there is a claimant, then use the claimant's.
  2. entity_id and entity_type need to be sent as well. entity_type should be individual or organization.

@holdenhinkle
Copy link
Collaborator

The frontend already has those all those values. Why not just pass them to the api and avoid a lookup?

@opticbob
Copy link
Author

opticbob commented Nov 1, 2024

Mainly for consistency's sake. We lookup the values in the PDF generation to make sure we aren't filling in stale data for forms that have been filled out over a long period of time.

@holdenhinkle
Copy link
Collaborator

Pros of passing the id and type:

  1. Consistency
  2. The very rare edge case where a rep updates their address information while a user is going through appoint, making the data the frontend has stale - the correct contact information would be emailed

Cons

  1. An additional db query

Going with Josh's recommendation, which involves changes to the backend and frontend

@opticbob
Copy link
Author

opticbob commented Nov 1, 2024

Blocked by backend ticket #96351 and frontend ticket #96353.

@opticbob
Copy link
Author

opticbob commented Nov 5, 2024

The frontend ticket is done and we're waiting for review on the backend ticket.

@opticbob
Copy link
Author

opticbob commented Nov 6, 2024

We are still waiting for Platform to review the backend ticket. They explained that their policy has now been clarified to say that a review can be requested 3 business days after a teammates approval.

@opticbob
Copy link
Author

opticbob commented Nov 8, 2024

Platform has commented on the ticket but has not responded to follow up questions.

@opticbob
Copy link
Author

The backend ticket #96351 was approved and merged this morning. I can test it as soon as it is deployed.

@opticbob
Copy link
Author

We have emails:

21-22:

Image

21-22a:

Image

@opticbob
Copy link
Author

I'm going to put this ticket in review while @oddball-lindsay and CAIA look at it.

@oddball-lindsay
Copy link
Contributor

Looking really good! The only tweak to get this to match the intended content from CAIA is to clean up the rep type terminology.

  1. organization = Veterans Service Organization (VSO)
  2. representative = VSO representative
  3. claims_agent = claims agent
  4. attorney = attorney

Some tweaks to 1 and 2, but 3 and 4 would really just be removing the "titleizing" ie capitalization.

After this is in place, we can test the emails one more time then promote to Production!

@opticbob
Copy link
Author

The PR with the test fixes has been merged and the results look good:

organization:

Image

VSO representative:

Image

attorney:

Image

claims agent:

Image

@oddball-lindsay oddball-lindsay self-assigned this Nov 12, 2024
@oddball-lindsay
Copy link
Contributor

Did some testing on my end, and I also confirmed that everything is looking good for each rep type. I promoted the email to production in the VA Notify system! Closing this out 🥳

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accredited-representation-management-team Accredited Representation Management team backend mvp Initial version of thing
Projects
None yet
Development

No branches or pull requests

3 participants