From c3a1d5c5066364ac9bf3579c4ffb5a801530f957 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 24 Dec 2020 15:09:41 +0000 Subject: [PATCH 1/2] Pass language through to lambda MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we’re sending non-GSM characters, we need to mark the language in the XML as Welsh (`cy-GB` in CAP, `Welsh` in IBAG). Currently, the CBC proxy checks the content we’re sending, and then uses an approximation based on ASCII to determine whether we’re sending any non-GSM characters, and if so, sets the language appropriately. Instead, we should can functionality from the notifications-utils repo to determine the language. If any non-GSM characters are used, then the we can set the language to Welsh. We’ll need to update the proxy to look at this new language flag. --- app/clients/cbc_proxy.py | 14 ++++++++++++++ tests/app/clients/test_cbc_proxy.py | 26 ++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index a52eac80d..eb329b270 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -2,6 +2,7 @@ import json import boto3 from flask import current_app +from notifications_utils.template import non_gsm_characters from app.config import BroadcastProvider from app.utils import DATETIME_FORMAT, format_sequential_number @@ -51,6 +52,9 @@ class CBCProxyClient: class CBCProxyClientBase: lambda_name = None + LANGUAGE_ENGLISH = 'en-GB' + LANGUAGE_WELSH = 'cy-GB' + def __init__(self, lambda_client): self._lambda_client = lambda_client @@ -111,6 +115,11 @@ class CBCProxyClientBase: return result + def infer_language_from(self, content): + if non_gsm_characters(content): + return self.LANGUAGE_WELSH + return self.LANGUAGE_ENGLISH + class CBCProxyCanary(CBCProxyClientBase): """ @@ -158,6 +167,7 @@ class CBCProxyEE(CBCProxyClientBase): 'areas': areas, 'sent': sent, 'expires': expires, + 'language': self.infer_language_from(description), } self._invoke_lambda(payload=payload) @@ -184,6 +194,9 @@ class CBCProxyEE(CBCProxyClientBase): class CBCProxyVodafone(CBCProxyClientBase): lambda_name = 'vodafone-1-proxy' + LANGUAGE_ENGLISH = 'English' + LANGUAGE_WELSH = 'Welsh' + def send_link_test( self, identifier, @@ -215,6 +228,7 @@ class CBCProxyVodafone(CBCProxyClientBase): 'areas': areas, 'sent': sent, 'expires': expires, + 'language': self.infer_language_from(description), } self._invoke_lambda(payload=payload) diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py index 742e97736..c8db4cc45 100644 --- a/tests/app/clients/test_cbc_proxy.py +++ b/tests/app/clients/test_cbc_proxy.py @@ -59,10 +59,18 @@ def test_cbc_proxy_lambda_client_has_correct_keys(cbc_proxy_ee): assert secret == 'cbc-proxy-aws-secret-access-key' -def test_cbc_proxy_ee_create_and_send_invokes_function(mocker, cbc_proxy_ee): +@pytest.mark.parametrize('description, expected_language', ( + ('my-description', 'en-GB'), + ('mŷ-description', 'cy-GB'), +)) +def test_cbc_proxy_ee_create_and_send_invokes_function( + mocker, + cbc_proxy_ee, + description, + expected_language, +): identifier = 'my-identifier' headline = 'my-headline' - description = 'my-description' sent = 'a-passed-through-sent-value' expires = 'a-passed-through-expires-value' @@ -117,6 +125,7 @@ def test_cbc_proxy_ee_create_and_send_invokes_function(mocker, cbc_proxy_ee): assert payload['areas'] == areas assert payload['sent'] == sent assert payload['expires'] == expires + assert payload['language'] == expected_language def test_cbc_proxy_ee_cancel_invokes_function(mocker, cbc_proxy_ee): @@ -175,10 +184,18 @@ def test_cbc_proxy_ee_cancel_invokes_function(mocker, cbc_proxy_ee): assert payload['sent'] == sent -def test_cbc_proxy_vodafone_create_and_send_invokes_function(mocker, cbc_proxy_vodafone): +@pytest.mark.parametrize('description, expected_language', ( + ('my-description', 'English'), + ('mŷ-description', 'Welsh'), +)) +def test_cbc_proxy_vodafone_create_and_send_invokes_function( + mocker, + cbc_proxy_vodafone, + description, + expected_language, +): identifier = 'my-identifier' headline = 'my-headline' - description = 'my-description' sent = 'a-passed-through-sent-value' expires = 'a-passed-through-expires-value' @@ -233,6 +250,7 @@ def test_cbc_proxy_vodafone_create_and_send_invokes_function(mocker, cbc_proxy_v assert payload['areas'] == areas assert payload['sent'] == sent assert payload['expires'] == expires + assert payload['language'] == expected_language def test_cbc_proxy_vodafone_cancel_invokes_function(mocker, cbc_proxy_vodafone): From 982546961346ef89f310aa9e634ce369d934fc16 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 24 Dec 2020 15:19:46 +0000 Subject: [PATCH 2/2] Make language attributes abstract properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will make it impossible to create a new client without at least having to define these properties. Which should get someone thinking about language support… --- app/clients/cbc_proxy.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index eb329b270..8500e57c4 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -1,4 +1,5 @@ import json +from abc import ABC, abstractmethod import boto3 from flask import current_app @@ -49,11 +50,18 @@ class CBCProxyClient: return proxy_classes[provider](self._lambda_client) -class CBCProxyClientBase: +class CBCProxyClientBase(ABC): lambda_name = None - LANGUAGE_ENGLISH = 'en-GB' - LANGUAGE_WELSH = 'cy-GB' + @property + @abstractmethod + def LANGUAGE_ENGLISH(self): + pass + + @property + @abstractmethod + def LANGUAGE_WELSH(self): + pass def __init__(self, lambda_client): self._lambda_client = lambda_client @@ -128,6 +136,9 @@ class CBCProxyCanary(CBCProxyClientBase): """ lambda_name = 'canary' + LANGUAGE_ENGLISH = None + LANGUAGE_WELSH = None + def send_canary( self, identifier, @@ -138,6 +149,9 @@ class CBCProxyCanary(CBCProxyClientBase): class CBCProxyEE(CBCProxyClientBase): lambda_name = 'bt-ee-1-proxy' + LANGUAGE_ENGLISH = 'en-GB' + LANGUAGE_WELSH = 'cy-GB' + def send_link_test( self, identifier,