Email the "contact us" form data instead of sending to Freshdesk for sensitive services

[smcmurtry]

Summary | Résumé

This PR does the following:

• adds a check when form data is submitted to our "contact us" form (this could be a go live request, branding request, help request, etc.). If the user submitting the form is logged in and belongs to at least one sensitive service, then we do not send their form data to Freshdesk, we email it to an inbox
• adds a feature flag (FF_PT_SERVICE_SKIP_FRESHDESK) to switch this feature on and off
• adds a db migration to create a new template for this email, to make it more obvious why the data was sent to email

Related Issues | Cartes liées

• https://app.zenhub.com/workspaces/notify-planning-614b3ad91bc2030015ed22f5/issues/gh/cds-snc/notification-planning/1738

Test instructions | Instructions pour tester la modification

1. Check this out locally and either rebuild your container or run poetry run flask db upgrade
2. edit your local .env file to include the following:

FF_SENSITIVE_SERVICE_SKIP_FRESHDESK=true
[email protected]
FRESH_DESK_PRODUCT_ID=42

4. run the api locally
5. Run the admin locally
6. create a new organisation "Ontario" with the type "Provincial/Territorial government"
7. create a new service
8. on the service settings page, set the organisation to Ontario
9. submit the form from the contact us page.
10. Observe that you get an email with the form data in it

Then switch the service to use a different organisation and try this flow again - the data should not be emailed and it should be sent to Freshdesk (if you have the credentials for that set locally)

Release Instructions | Instructions pour le déploiement

We will need to add FF_PT_SERVICE_SKIP_FRESHDESK to notification-manifests.

Reviewer checklist | Liste de vérification du réviseur

• This PR does not break existing functionality.
• This PR does not violate GCNotify's privacy policies.
• This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
• This PR does not significantly alter performance.
• Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

[jzbahrai]

so it looks good to me while testing! The email gets sent, but the format of the email is a little off:

“product_id”: 42,
“subject”: “Other”,
“description”: “lol<br><br>---<br><be>

We should not be sending the

Why is there a product id as well? Can we just change the template to say
user_Id:
service_id:
description:

[jzbahrai]

this is a short list for now, and this is a temporary feature - hence the below is a question not a request to change.

Would it make sense to take the email_address of the person -> get the service_ids the user is associated with? Instead of the larger list?

[smcmurtry]

Good point actually! We already have the organisation_type data for a user's services without doing an extra query. I've changed the code and removed the dao_fetch_service_ids_of_sensitive_services fn.

[smcmurtry]

We don't have the service_id since support requests are just tied to a user, not a specific service.

The user_id and description are in there but the formatting is bad.

It would be a tonne of work to improve the formatting, because I'm re-using this existing function that just spits out html: https://github.com/cds-snc/notification-api/blob/fix/send-sensitive-to-inbox/app/clients/freshdesk.py#L26-L97

That fn has a bunch of logic to include different data depending on the type of request. So I would have to duplicate all of that if I were going to include the same data as plain text in the template.

[jimleroyer]

😍