department-of-veterans-affairs / notification-api

Notification API
MIT License
16 stars 8 forks source link

Remove Todo Code Comment: Phone Number Formatting #879

Open trevor2718 opened 1 year ago

trevor2718 commented 1 year ago

User Story - Business Need

The code base is littered with TODOs that should either be removed or worked as needed. This particular TODO should be removed based on PM decision

TODO: In send_inbound_sms_to_service method, payload includes source_number.

payload = {
        "id": str(inbound_sms.id),
        # TODO: should we be validating and formatting the phone number here?
        "source_number": inbound_sms.user_number,
        "destination_number": inbound_sms.notify_number,
        "message": inbound_sms.content,
        "date_received": inbound_sms.provider_date.strftime(DATETIME_FORMAT),
        "sms_sender_id": str(sms_sender.id) if sms_sender else None
    }

User Story(ies)

As a VANotify I want to keep a clean and tidy repo So that development is straightforward and tech debt is addressed

Additional Info and Resources

Decision: We are not going to validate or format the phone number because we want to keep the data as an audit trail.

Helpful links: https://developers.omnisend.com/guides/e164-phone-number-formatting https://www.twilio.com/docs/glossary/what-e164

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

kalbfled commented 1 year ago

This might be useful: https://stackoverflow.com/questions/59686863/flask-sqlalchemy-phonenumber-type

mjones-oddball commented 1 year ago

We want to keep the database accurate as an audit trail so I don't want to not track a phone number that has sent an inbound message successfully (e.g. if somehow a short code texted us).

jakehova commented 1 year ago

We are not going to validate or format the phone number because we want to keep the data as an audit trail.

mjones-oddball commented 1 year ago

This is only open to remove the todo from the code