Skip to content

Commit

Permalink
[#190] add logic in accept_recipient_identifiers_enabled() to check i…
Browse files Browse the repository at this point in the history
…f the env variable is equal to the string True, since env var is always a string, not a boolean

move the feature flag out of config, directly referencing environment variables from accept_recipient_idenifiers_enabled()
  • Loading branch information
marisahoenig committed Oct 30, 2020
1 parent 4e0e8cd commit 7ac537e
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 17 deletions.
1 change: 0 additions & 1 deletion app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,6 @@ class Config(object):
API_RATE_LIMIT_ENABLED = False
API_MESSAGE_LIMIT_ENABLED = False
SWITCH_SLOW_SMS_PROVIDER_ENABLED = False
ACCEPT_RECIPIENT_IDENTIFIERS_ENABLED = os.getenv('ACCEPT_RECIPIENT_IDENTIFIERS_ENABLED', False)


######################
Expand Down
9 changes: 7 additions & 2 deletions app/feature_flags.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import os

PROVIDER_FEATURE_FLAGS = {
'govdelivery': 'GOVDELIVERY_EMAIL_CLIENT_ENABLED'
}
Expand All @@ -10,5 +12,8 @@ def is_provider_enabled(current_app, provider_identifier):
return True


def accept_recipient_identifiers_enabled(current_app):
return current_app.config.get('ACCEPT_RECIPIENT_IDENTIFIERS_ENABLED', False)
def accept_recipient_identifiers_enabled():
if os.getenv('ACCEPT_RECIPIENT_IDENTIFIERS_ENABLED', 'False') == 'True':
return True
else:
return False
2 changes: 1 addition & 1 deletion app/notifications/process_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def persist_notification(
reply_to_text=reply_to_text,
billable_units=billable_units
)
if accept_recipient_identifiers_enabled(current_app) and recipient_identifier:
if accept_recipient_identifiers_enabled() and recipient_identifier:
_recipient_identifier = RecipientIdentifier(
notification_id=notification_id,
id_type=recipient_identifier['id_type'],
Expand Down
2 changes: 1 addition & 1 deletion app/v2/notifications/post_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def post_notification(notification_type):
reply_to_text=reply_to
)
else:
if accept_recipient_identifiers_enabled(current_app):
if accept_recipient_identifiers_enabled():
notification = process_notification_with_recipient_identifier(
form=form,
notification_type=notification_type,
Expand Down
23 changes: 11 additions & 12 deletions tests/app/test_feature_flags.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import os

import pytest

from app.feature_flags import is_provider_enabled, accept_recipient_identifiers_enabled


Expand All @@ -13,15 +17,10 @@ def test_is_provider_without_a_flag_enabled(mocker):
assert is_provider_enabled(current_app, 'some-provider-without-a-flag')


def test_accept_recipient_identifiers_enabled(mocker):
current_app = mocker.Mock(config={
'ACCEPT_RECIPIENT_IDENTIFIERS_ENABLED': True
})
assert accept_recipient_identifiers_enabled(current_app)


def test_accept_recipient_identifiers_not_enabled(mocker):
current_app = mocker.Mock(config={
'ACCEPT_RECIPIENT_IDENTIFIERS_ENABLED': False
})
assert not accept_recipient_identifiers_enabled(current_app)
@pytest.mark.parametrize('enabled_string, enabled_boolean', [
('True', True),
('False', False)
])
def test_accept_recipient_identifiers_flag(mocker, enabled_string, enabled_boolean):
mocker.patch.dict(os.environ, {'ACCEPT_RECIPIENT_IDENTIFIERS_ENABLED': enabled_string})
assert accept_recipient_identifiers_enabled() == enabled_boolean

0 comments on commit 7ac537e

Please sign in to comment.