diff --git a/app/main/views/providers.py b/app/main/views/providers.py index d440daebd..0484f4463 100644 --- a/app/main/views/providers.py +++ b/app/main/views/providers.py @@ -10,7 +10,7 @@ from app.main import main from app.main.forms import ProviderForm, ProviderRatioForm from app.utils import user_is_platform_admin -PROVIDER_PRIORITY_MEANING_SWITCHOVER = datetime(2019, 11, 29, 11, 0) +PROVIDER_PRIORITY_MEANING_SWITCHOVER = datetime(2019, 11, 29, 11, 0).isoformat() @main.route("/providers") @@ -69,65 +69,39 @@ def edit_sms_provider_ratio(): form = ProviderRatioForm(ratio=providers[0]['priority']) - if form.validate_on_submit(): - provider_client.update_provider(providers[0]['id'], form.percentage_left) - provider_client.update_provider(providers[1]['id'], form.percentage_right) - if len(providers) < 2: abort(400) + primary_provider, secondary_provider = providers[0:2] + + if form.validate_on_submit(): + provider_client.update_provider(primary_provider['id'], form.percentage_left) + provider_client.update_provider(secondary_provider['id'], form.percentage_right) + return redirect(url_for('.edit_sms_provider_ratio')) + return render_template( 'views/providers/edit-sms-provider-ratio.html', - versions=_chunk_versions_by_day(_get_versions(providers[0], providers[1])), + versions=_chunk_versions_by_day(_get_versions_since_switchover(primary_provider['id'])), form=form, + primary_provider=providers[0]['display_name'], + secondary_provider=providers[1]['display_name'], ) -def _get_versions(provider0, provider1): +def _get_versions_since_switchover(provider_id): - versions = sorted(( - provider_client.get_provider_versions(provider0['id'])['data'] + - provider_client.get_provider_versions(provider1['id'])['data'] - ), key=lambda version: version['updated_at'] or '', reverse=True) + for version in sorted( + provider_client.get_provider_versions(provider_id)['data'], + key=lambda version: version['updated_at'] or '' + ): - for index, version in enumerate(versions): - - previous_version = get_previous_version( - versions, - index, - version['identifier'], - ) - - if ( - (version['updated_at'] or '0') < str(PROVIDER_PRIORITY_MEANING_SWITCHOVER) - ): - version['priority'], previous_version['priority'] = ( - int(version['priority'] > previous_version['priority']), - int(version['priority'] <= previous_version['priority']) - ) - - if version['identifier'] == provider0['identifier']: - fresh_version = version.copy() - fresh_version['other_provider'] = previous_version.copy() - yield fresh_version - elif previous_version['identifier'] in {provider0['identifier'], None}: - fresh_version = previous_version.copy() - fresh_version['other_provider'] = version.copy() - yield fresh_version - - -def get_previous_version(versions, start_index, current_provider_identifier): - for index, version in enumerate(versions): - if index < start_index: + if not version['updated_at']: continue - if current_provider_identifier == version['identifier']: + + if version['updated_at'] < PROVIDER_PRIORITY_MEANING_SWITCHOVER: continue - return version - return { - 'identifier': None, - 'priority': 999, - 'updated_at': None, - } + + yield version def _chunk_versions_by_day(versions): @@ -136,7 +110,7 @@ def _chunk_versions_by_day(versions): for version in sorted(versions, key=lambda version: version['updated_at'] or '', reverse=True): days[ - format_date_numeric(version['updated_at']) if version['updated_at'] else '' + format_date_numeric(version['updated_at']) ].append(version) return sorted(days.items(), reverse=True) diff --git a/app/templates/views/providers/edit-sms-provider-ratio.html b/app/templates/views/providers/edit-sms-provider-ratio.html index 1f1435f39..dc7ca15ae 100644 --- a/app/templates/views/providers/edit-sms-provider-ratio.html +++ b/app/templates/views/providers/edit-sms-provider-ratio.html @@ -24,10 +24,10 @@
- MMG + {{ primary_provider }}
- Firetext + {{ secondary_provider }}
{% call form_wrapper() %}
@@ -71,17 +71,16 @@ {% endif %}
- {% set percentage = (version.priority / (version.priority + version.other_provider.priority) * 100) %}
- {{ version.display_name }}

- {{ percentage|format_thousands }}% + {{ primary_provider }}

+ {{ version.priority|format_thousands }}%
- {{ version.other_provider.display_name }}

- {{ (100 - percentage)|format_thousands }}% + {{ secondary_provider }}

+ {{ (100 - version.priority)|format_thousands }}%
-
+
diff --git a/tests/app/main/views/test_providers.py b/tests/app/main/views/test_providers.py index 9bf47f2ac..372ef2d6d 100644 --- a/tests/app/main/views/test_providers.py +++ b/tests/app/main/views/test_providers.py @@ -1,12 +1,15 @@ import copy import re from datetime import datetime +from unittest.mock import call +import pytest from bs4 import BeautifulSoup from flask import url_for import app from app.main.views.providers import add_monthly_traffic +from tests.conftest import normalize_spaces stub_providers = { 'provider_details': [ @@ -79,12 +82,13 @@ stub_providers = { 'id': '67c770f5-918e-4afa-a5ff-880b9beb161d', 'active': False, 'priority': 10, - 'display_name': 'International SMS Provider (no flag)', + 'display_name': 'Second International SMS Provider', 'identifier': 'second_sms_international', 'notification_type': 'sms', 'updated_at': None, 'version': 1, 'created_by': None, + 'supports_international': True, 'current_month_billable_sms': 0, } ] @@ -406,3 +410,143 @@ def test_should_show_provider_version_history( assert second_row[2].text.strip() == "None" assert second_row[3].text.strip() == "10" assert second_row[4].text.strip() == "True" + + +def test_should_show_version_history_for_first_two_sms_providers( + client_request, + platform_admin_user, + mocker +): + mocker.patch( + 'app.provider_client.get_all_providers', + return_value=copy.deepcopy(stub_providers) + ) + # second_sms_international will be the primary provider because it’s + # the first in the list when reverse sorting the SMS providers + second_sms_international = stub_providers['provider_details'][5] + mocker.patch( + 'app.provider_client.get_provider_versions', + return_value={'data': [ + { + 'id': id, + 'priority': priority, + 'display_name': second_sms_international['display_name'], + 'identifier': second_sms_international['identifier'], + 'updated_at': updated_at, + 'created_by': { + 'email_address': 'test@foo.bar', + 'name': 'Test User', + 'id': '7cc1dddb-bcbc-4739-8fc1-61bedde3332a' + }, + 'supports_international': False, + } + for updated_at, priority in [ + (datetime(2022, 2, 22, 14).isoformat(), 100), + (datetime(2020, 1, 1, 5).isoformat(), 80), + (datetime(2020, 1, 1, 3).isoformat(), 10), + # Anything older than 11am on 29 November 2019 + # should be ignored because the priority numbers + # didn’t mean the same thing before then + (datetime(2019, 11, 29, 10, 59).isoformat(), 123), + (datetime(2000, 1, 1, 0).isoformat(), 1999), + (None, 30), + ] + ]} + ) + + client_request.login(platform_admin_user) + page = client_request.get('main.edit_sms_provider_ratio') + + assert [ + radio['value'] + for radio in page.select('input[name=ratio]') + ] == [ + '100', '90', '80', '70', '60', '50', '40', '30', '20', '10', '0', + ] + + assert [ + radio['value'] + for radio in page.select('input[checked]') + ] == [ + str(second_sms_international['priority']) + ] + + assert [ + normalize_spaces(heading.text) + for heading in page.select('main h2') + ] == [ + 'Now', + '21 February 2022', + '31 December', + ] + + assert [ + normalize_spaces(version.text) + for version in page.select('li.history-list-item') + ] == [ + ( + 'Test User 2:00pm ' + 'Second International SMS Provider 100% ' + 'Second Domestic SMS Provider 0%' + ), + ( + 'Test User 5:00am ' + 'Second International SMS Provider 80% ' + 'Second Domestic SMS Provider 20%' + ), + ( + 'Test User 3:00am ' + 'Second International SMS Provider 10% ' + 'Second Domestic SMS Provider 90%' + ), + ] + + +@pytest.mark.parametrize('posted_number, expected_calls', [ + ( + '10', + [ + call(stub_providers['provider_details'][5]['id'], 10), + call(stub_providers['provider_details'][1]['id'], 90), + ], + ), + ( + '80', + [ + call(stub_providers['provider_details'][5]['id'], 80), + call(stub_providers['provider_details'][1]['id'], 20), + ], + ), +]) +def test_should_update_priority_of_first_two_sms_providers( + client_request, + platform_admin_user, + mocker, + posted_number, + expected_calls, +): + mocker.patch( + 'app.provider_client.get_all_providers', + return_value=copy.deepcopy(stub_providers) + ) + mocker.patch( + 'app.provider_client.get_provider_versions', + return_value={'data': []} + ) + mock_update_provider = mocker.patch( + 'app.provider_client.update_provider' + ) + + client_request.login(platform_admin_user) + client_request.post( + '.edit_sms_provider_ratio', + _data={ + 'ratio': posted_number, + }, + _expected_redirect=url_for( + '.edit_sms_provider_ratio', + _external=True, + ), + ) + + assert mock_update_provider.call_args_list == expected_calls