department-of-veterans-affairs / notification-api

Notification API
MIT License
15 stars 9 forks source link

TODO comment Get sender name and sender email separately #880

Open trevor2718 opened 1 year ago

trevor2718 commented 1 year ago

User Story - Business Need

This TODO comment asks that we take the sender name and email separately rather than together.

It is also in notification-api/app/clients/email/govdelivery_client.py and is recommended that we should move this code up to before a provider is called so that any provider that is used gets the same information (since all providers will likely have this code to strip out email from name).

Suggestion: upon receiving the payload, run a "Sanitize" method that does any necessary formatting of the payload before sending it to a provider.

TODO: Sometimes the source is Foo <foo@bar.com> vs just foo@bar.com. Current code gets the source like :

if "<" in source:
    source = source.split("<")[1].split(">")[0]

Take sender name and email separately.

User Story(ies)

As a VANotify I want to reduce tech debt and handle TODO comments So that development is more straightforward

Additional Info and Resources

Engineering Checklist

Acceptance Criteria

mjones-oddball commented 1 year ago

Hey team! Please add your planning poker estimate with Zenhub @EvanParish @k-macmillan @kalbfled @trevor2718 @jakehova @cris-oddball @ianperera

jakehova commented 1 year ago

We should move this code up to before a provider is called so that any provider that is used gets the same information (since all providers will likely have this code to strip out email from name).

Suggestion: upon receiving the payload, run a "Sanitize" method that does any necessary formatting of the payload before sending it to a provider.

mjones-oddball commented 1 year ago

Please add your planning poker estimate with Zenhub @justaskdavidb2

cris-oddball commented 10 months ago

@mjones-oddball this code is in a file called govdelivery_client.py Do we use govdelivery? Should the ticket be modified to remove this file instead?

Nevermind, I just saw Jacob's comment above - will refactor this ticket.

mjones-oddball commented 10 months ago

@cris-oddball Unless we were facing issues with govdelivery being a supported provider I wouldn't want to remove the ability. God forbid something happened to Amazon we could easily revert to sending email thru govdelivery which used to be the email provider before Amazon. (before our time)