IIR - Integrate permission check into contact info lookup

[stevelong00]

User Story - Business Need

Companion Ticket from VA-IIR Team

Currently we have two celery tasks, one for looking up contact info and one for looking up communication permissions. We want to move the permissions check into the contact info lookup so that both can use a single call to Profile API.

User Story(ies)

As a software developer working on the notifications-api repository I want to integrate permission check into contact info lookup So that I can both can use a single call to Profile API

Additional Info and Resources

• Parent spike with details in the comments

• Profile API Notes

Currently we have two celery tasks, contact_information_tasks.py and lookup_recipient_communication_permissions_task.py.

We want to combine these into a single task, which makes a single call to Profile API. The simplest way to do this is to integrate the permission check into the contact info lookup, so that contact info is returned only when it both exists and has permissions enabled.

Acceptance Criteria

• [ ] In the VAProfileClient class, create a new method get_contact_info_with_permission. This makes a single call to the Profile API v3 to fetch email, phone, and permissions. If permissions are not granted for the requested email or phone number, a new exception should be raised indicating that permissions were denied.

• [ ] In the lookup_contact_info celery task

  • [ ] When the COMBINE_CONTACT_INFO_AND_PERMISSIONS_LOOKUP flag is enabled, replace calls to get_email or get_telephone with calls to get_contact_info_with_permission
  • [ ] add a check for the new permissions denied exception and handle it appropriately.

• [ ] In process_notifications.py, when the COMBINE_CONTACT_INFO_AND_PERMISSIONS_LOOKUP flag is enabled, bypass both places where the lookup_recipient_communication_permissions task is inserted

QA Considerations

All business logic should be the same as before. The only difference with these changes should be that the permission check now happens within the same task as the phone or email lookup.

Potential Dependencies

Out of Scope

[cris-oddball]

PR is approved and merged. There is follow-up work. The Prod feature flag in this PR is set to False, meaning it is off and using the default code in Prod. Will hold this ticket open for QA testing.

[cris-oddball]

PR approved and merged after the revert earlier this week. Currently deployed to Perf. Will begin manual testing for edge cases next week.

[cris-oddball]

Edge cases so far:

• callbacks are working with the feature flag enabled
• a vet with no contact info is still working as expected

Test case failures:

• with feature flag off in staging (had to cut a branch for that), notifications that exercised the default send scenarios were stuck in created. Kyle and Evan working on a fix.
• discrepancies when sending to a deceased veteran (test user 1411910), in perf with FF on the response is delivered, in staging with FF off, the response is a permanent failure "Failed to deliver email due to hard bounce","
• still looking into edge cases

[cris-oddball]

I went back to the original tickets for the explicit send feature and don't really see any other edge cases.

Kyle and Evan have been working on a fix for the 'stuck in created' issue, expect to see a PR 'soon'. They are also looking at that deceased veteran hard bounce vs delivered issue.

[cris-oddball]

A fix is up. That fix has been deployed to Perf and Staging PERF: Feature flag is enabled | True STAGING: Feature flag is disabled | False

Now have a known good deceased PID for perf/staging, will test with that one.

will retest things Tuesday morning.

[cris-oddball]

QA PASSED

PERF - ENABLED Has feature flag set to True (enabled, will use V3) Callback: /internal/qa-staging-automation-callback Both sms and email tested

• [x] Regression passes (exercises recipient identifier lookups and the 6 default send scenarios)
• [x] status delivered has no status_reason
• [x] status preferences-declined where recipient has opted-out of a notification has status_reason 'Contact preferences set to false'
• [x] status preferences-declined where recipient has not set a preference for notifications has status_reason 'No recipient opt-in found for explicit preference'
• [x] status 'permanent-failure' where recipient is deceased (MPI lookup) has status_reason 'Beneficiary has deceased status in MPI'
• [x] status 'permanent-failure' where VAPROFILEID has no contact info has status_reason 'No contact info found from VA Profile'
• [x] Callbacks work as expected

STAGING - DISABLED Has feature flag set to False (disabled, does not use V3) Callback: /internal/qa-staging-automation-callback Both sms and email tested

• [x] Regression passes (exercises recipient identifier lookups and the 6 default send scenarios)
• [x] status delivered has no status_reason
• [x] status preferences-declined where recipient has opted-out of a notification has status_reason 'Contact preferences set to false'
• [x] status preferences-declined where recipient has not set a preference for notifications has status_reason 'No recipient opt-in found for explicit preference'
• [x] status 'permanent-failure' where recipient is deceased (MPI lookup) has status_reason 'Beneficiary has deceased status in MPI'
• [x] status 'permanent-failure' where VAPROFILEID has no contact info has status_reason 'No contact info found from VA Profile'
• [x] Callbacks work as expected

[AdamKing0126]

Thanks for the update, Cris.

Can you help me understand how to reproduce these errors, and maybe write tests against the functionality? Are there any tests in the repo that I can look at, which we have not covered in the v3 functionality?

[AdamKing0126]

Oh! The checkboxes are filled in now! Maybe I spoke too soon?

[cris-oddball]

@AdamKing0126 you can refer to the PR hotfix that Evan and Kyle worked on and discuss with Evan if you have any questions.

[k-macmillan]

PR has been merged.

[cris-oddball]

This is going straight up to Prod. Closing ticket.