Skip to content

Commit

Permalink
Merge branch 'main' into 2137-send-va-profile-sms-delivery-status
Browse files Browse the repository at this point in the history
  • Loading branch information
MackHalliday authored Dec 4, 2024
2 parents 9b4488e + b6bc1e9 commit 5d6903a
Show file tree
Hide file tree
Showing 21 changed files with 65 additions and 35 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/cd-pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ on:

jobs:
prepare-deployment:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Checkout code
uses: actions/checkout@v4
Expand All @@ -34,7 +34,7 @@ jobs:
needs: prepare-deployment
environment:
name: perf-deploy
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Pause for manual approval
run: |
Expand All @@ -53,7 +53,7 @@ jobs:

build-push-artifacts:
needs: create-and-post-tag
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Checkout Repo
uses: actions/checkout@v4
Expand All @@ -80,7 +80,7 @@ jobs:
needs: deploy-to-perf
environment:
name: staging-deploy
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Pause for manual approval
run: echo "Deployment paused for manual approval to staging and production."
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/create-and-post-tag.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ on:

jobs:
create-and-post-tag:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
outputs:
previousVersion: ${{ steps.create_tag.outputs.previousVersion }}
newVersion: ${{ steps.create_tag.outputs.newVersion }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/create-release-notes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ on:

jobs:
create-release-notes:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
outputs:
draftReleaseReference: ${{ steps.create_notes.outputs.draftReleaseReference }}
permissions:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/db-downgrade.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ on:
default: container_tag
jobs:
run-downgrade:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- uses: actions/checkout@v4

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependency-ticket-creation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:

jobs:
create_issue:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Checkout code
uses: actions/checkout@v3
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/deploy-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ env:
jobs:
setup-environment:
name: 'setup-env'
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Checkout Repo
uses: actions/checkout@v4
Expand All @@ -70,7 +70,7 @@ jobs:

upload-env-file:
needs: [setup-environment]
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Checkout Repo
uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/deploy_lambda_dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ on:

jobs:
setup_job:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Checkout Repo
uses: actions/checkout@v4
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/deployment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ on:

jobs:
run-migrations:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- uses: actions/checkout@v4
with:
Expand Down Expand Up @@ -58,7 +58,7 @@ jobs:
bash ./scripts/run_ci_migrations.sh -c ${{ inputs.environment }}-notification-cluster -e ${{ inputs.environment }} -t ${{ steps.register.outputs.arn }}
deploy-api:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
needs: [run-migrations]
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -102,7 +102,7 @@ jobs:
wait-for-service-stability: true

deploy-celery:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
needs: [run-migrations]
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -146,7 +146,7 @@ jobs:
wait-for-service-stability: true

deploy-celery-beat:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
needs: [run-migrations]
steps:
- uses: actions/checkout@v4
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/dev-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ jobs:
make test
code-scan:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- uses: actions/checkout@v4
with:
Expand All @@ -75,7 +75,7 @@ jobs:
- run: make check-vulnerabilities

dependency-scan:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- uses: actions/checkout@v4
with:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/dev_deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ on:
jobs:
setup-environment:
name: "setup-env-${{ inputs.environment }}"
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
outputs:
git-hash: ${{ steps.set-hash.outputs.commit-hash }}
steps:
Expand All @@ -61,7 +61,7 @@ jobs:
build-push-artifacts:
needs: [setup-environment]
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- name: Checkout Repo
uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lambda-functions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ env:
jobs:
# All of the lambda functions use layers for dependencies, if any. Bundling dependencies should not be necessary.
build-and-deploy-lambda-functions:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
defaults:
run:
working-directory: "./lambda_functions"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/load-test-email-delivery-time.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ on:

jobs:
trigger:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- uses: actions/checkout@v4

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/load-test-sms-response-time.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ on:

jobs:
trigger:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- uses: actions/checkout@v4

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pr-label-semver.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:

jobs:
check-pr-sem:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
permissions:
contents: write
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pre-tag-summary.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ on:

jobs:
pre-tag-summary:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
permissions:
contents: write
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish-release-notes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
type: string
jobs:
publish-release-notes:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
permissions:
contents: write
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/run-regression.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ on:
jobs:
trigger-regression-tests:
name: "Run regression for ${{ inputs.environment }}"
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
environment: ${{ inputs.environment }}
steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/twistlock.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:

jobs:
twistlock-scan:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
steps:
- uses: actions/checkout@v4
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/update-datadog-image.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ on:

jobs:
update-image:
runs-on: ubuntu-latest
runs-on: ${{ vars.RUNS_ON }}
env:
IMAGE_REPOSITORY: "datadog/agent"
IMAGE_TAG: ${{ github.event.inputs.version_tag }} # Use the provided version tag for the image
Expand Down
8 changes: 7 additions & 1 deletion app/clients/sms/twilio.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class TwilioSMSClient(SmsClient):
'21610': TwilioStatus(21610, NOTIFICATION_PERMANENT_FAILURE, OPT_OUT_MESSAGE),
'21612': TwilioStatus(21612, NOTIFICATION_PERMANENT_FAILURE, 'Invalid to/from combo'),
'21614': TwilioStatus(21614, NOTIFICATION_PERMANENT_FAILURE, 'Non-mobile number'),
'21617': TwilioStatus(21617, NOTIFICATION_PERMANENT_FAILURE, 'Message too long'),
'21635': TwilioStatus(21635, NOTIFICATION_PERMANENT_FAILURE, 'Non-mobile number'),
'30001': TwilioStatus(30001, NOTIFICATION_TEMPORARY_FAILURE, 'Queue overflow'),
'30002': TwilioStatus(30002, NOTIFICATION_PERMANENT_FAILURE, 'Account suspended'),
Expand Down Expand Up @@ -175,7 +176,6 @@ def send_sms(
Return: a string containing the Twilio message.sid
"""

start_time = monotonic()
from app.dao.service_sms_sender_dao import (
dao_get_service_sms_sender_by_service_id_and_number,
Expand Down Expand Up @@ -238,6 +238,12 @@ def send_sms(
if e.status == 400 and 'phone number' in e.msg:
self.logger.exception('Twilio send SMS request for %s failed', reference)
raise InvalidProviderException from e
elif e.status == 400 and e.code == 21617: # Twilio error code for max length exceeded
self.logger.exception(
'Twilio send SMS request for %s failed, message content max length exceeded.', reference
)
self.logger.debug('Twilio error details for %s - %s: %s', reference, e.code, e.msg)
raise NonRetryableException('Twilio request failed') from e
else:
raise
except:
Expand Down
36 changes: 30 additions & 6 deletions tests/app/clients/test_twilio.py
Original file line number Diff line number Diff line change
Expand Up @@ -599,19 +599,43 @@ def test_send_sms_twilio_callback(
assert response_dict['sid'] == twilio_sid


def test_send_sms_raises_invalid_provider_error_with_invalid_twilio_number(
@pytest.mark.parametrize(
(
'response_dict',
'exception',
),
(
(
{
'code': 21606,
'message': "The 'From' phone number provided (+61412345678) is not a valid message-capable Twilio phone number for this destination",
},
InvalidProviderException,
),
(
{
'code': 21617,
'message': 'Unable to create record: The concatenated message body exceeds the 1600 character limit',
},
NonRetryableException,
),
),
ids=(
'invalid-from-number',
'message-too-long',
),
)
def test_send_sms_raises_non_retryable_exception_with_invalid_request(
notify_api,
mocker,
response_dict,
exception,
):
to = '+61412345678'
content = 'my message'
reference = 'my reference'
response_dict = {
'code': 21606,
'message': "The 'From' phone number provided (+61412345678) is not a valid message-capable Twilio phone number for this destination",
}

with pytest.raises(InvalidProviderException):
with pytest.raises(exception):
with requests_mock.Mocker() as r_mock:
r_mock.post(
f'https://api.twilio.com/2010-04-01/Accounts/{twilio_sms_client._account_sid}/Messages.json',
Expand Down

0 comments on commit 5d6903a

Please sign in to comment.