department-of-veterans-affairs / notification-api

Notification API
MIT License
16 stars 9 forks source link

TODO: Update predetermined list of simulated email addresses #891

Open trevor2718 opened 2 years ago

trevor2718 commented 2 years ago

User Story - Business Need

The code base is littered with TODO comments. This TODO asks to pass simulated as a parameter to process_sms_or_email_notification or to mock the undesired side-effects in test code, rather than using a predetermined list of e-mail addresses defined or simulation

User Story(ies)

As a VANotify I want to remove tech debt So that to have a maintainable codebase

Additional Info and Resources

TODO: This value is computed using a predetermined list of e-mail addresses defined to be for simulation. A better approach might be to pass "simulated" as a parameter to process_sms_or_email_notification or to mock the undesired side-effects in test code.

def process_sms_or_email_notification(*, form, notification_type, api_key, template, service, reply_to_text=None):
    form_send_to = form["email_address" if (notification_type == EMAIL_TYPE) else "phone_number"]

    send_to = validate_and_format_recipient(
        send_to=form_send_to,
        key_type=api_key.key_type,
        service=service,
        notification_type=notification_type
    )

    # Do not persist or send notification to the queue if it is a simulated recipient.
    #
    # TODO (tech debt) - This value is computed using a predetermined list of e-mail addresses defined
    # to be for simulation.  A better approach might be to pass "simulated" as a parameter to
    # process_sms_or_email_notification or to mock the undesired side-effects in test code.
    simulated: bool = simulated_recipient(send_to, notification_type)

See @EvanParish for more details.

Engineering Checklist

Acceptance Criteria

mjones-oddball commented 2 years ago

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

mjones-oddball commented 1 year ago

Please add your planning poker estimate with Zenhub @justaskdavidb2