department-of-veterans-affairs / notification-api

Notification API
MIT License
16 stars 8 forks source link

TODO: Behaviour change when default communication item permissions #899

Open trevor2718 opened 1 year ago

trevor2718 commented 1 year ago

User Story - Business Need

The codebase is littered with TODO comments. This TODO note that the behavior in the test test_should_raise_exception_if_recipient_has_no_permissions() in app/va/vaprofile/va_profile_client.py will change "once we starting using default communication item permissions"

Determine whether to just remove the TODO comment or to produce some work that addresses the changed behavior.

User Story(ies)

As a VANotify I want to resolve issues noted in TODOs So that so that the code base is straightforward to develop against

Additional Info and Resources

TODO: Note that this behaviour will change once we starting using default communication item permissions

def test_should_raise_exception_if_recipient_has_no_permissions(
            self, test_va_profile_client, rmock
    ):
        # TODO: Note that this behavior will change once we starting using default communication item permissions
        response = {
            "txAuditId": "37df9590-e791-4392-ae77-eaffc782276c",
            "status": "COMPLETED_SUCCESS"
        }
        rmock.get(ANY, json=response, status_code=200)

        recipient_identifier = RecipientIdentifier(id_type='VAPROFILEID', id_value='1')

        with pytest.raises(CommunicationItemNotFoundException):
            test_va_profile_client.get_is_communication_allowed(
                recipient_identifier, 'some-random-id', 'some-notification-id', SMS_TYPE
            )

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

mjones-oddball commented 1 year ago

Please add your planning poker estimate with Zenhub @justaskdavidb2

cris-oddball commented 1 year ago

@mjones-oddball "this behaviour will change once we starting using default communication item permissions" -> are we using default communication item permissions currently or is this some future work?

mjones-oddball commented 1 year ago

@cris-oddball I'm not following this ticket. We've been using default send for communication items the entire time. Only recently was explicit consent logic added, so we now support both.