Skip to content

Commit

Permalink
Merge branch 'main' into 1979-enrollment-confirmation-sms-notification
Browse files Browse the repository at this point in the history
  • Loading branch information
MackHalliday authored Oct 22, 2024
2 parents 4ff293f + 8cd65f0 commit 5a966d1
Show file tree
Hide file tree
Showing 56 changed files with 1,071 additions and 5,080 deletions.
10 changes: 5 additions & 5 deletions .github/scripts/createAndPostTag.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ async function createAndPostTag(params) {
const previousVersion = await getReleaseVersionValue(github, owner, repo);

// Retrieve PR data to decide the new version tag
const { releaseBranchSha, newVersion } = await prData({
const { mainBranchSha, newVersion } = await prData({
github,
context,
core,
});

// Create and push the tag using the SHA from releaseBranchSha
await createTag(github, owner, repo, newVersion, releaseBranchSha);
// Create and push the tag using the SHA from mainBranchSha
await createTag(github, owner, repo, newVersion, mainBranchSha);

// Update the RELEASE_VERSION repo variable
await github.rest.actions.updateRepoVariable({
Expand All @@ -86,10 +86,10 @@ async function createAndPostTag(params) {
// Construct the summary content
const summaryContent = `
### Successful Tag Creation!
- After merge to the release branch, a tag was created.
- After merge to the main branch, a tag was created.
- Previous version was ${previousVersion}
- New version is ${newVersion}
- Tag created for version ${newVersion} using the new release branch SHA: ${releaseBranchSha}
- Tag created for version ${newVersion} using the new main branch SHA: ${mainBranchSha}
`;

// Append the summary to the GitHub step summary file or log it
Expand Down
14 changes: 7 additions & 7 deletions .github/scripts/prData.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,21 @@ async function fetchPullRequests(github, owner, repo, sha) {
}

/**
* Retrieves the SHA of the release branch's latest commit.
* Retrieves the SHA of the main branch's latest commit.
* @param {Object} github - The GitHub client instance.
* @param {string} owner - The owner of the GitHub repository.
* @param {string} repo - The repository name.
* @returns {Promise<string>} - A promise resolving to the SHA of the latest commit on the release branch.
* @returns {Promise<string>} - A promise resolving to the SHA of the latest commit on the main branch.
*/
async function fetchReleaseBranchSha(github, owner, repo) {
async function fetchMainBranchSha(github, owner, repo) {
const { data } = await github.rest.repos.getCommit({
owner,
repo,
ref: 'heads/release',
ref: 'heads/main',
});

if (data && data.sha) {
console.log('The release branch head SHA is: ' + data.sha);
console.log('The main branch head SHA is: ' + data.sha);
return data.sha;
} else {
throw new Error('No SHA found in the response');
Expand Down Expand Up @@ -94,7 +94,7 @@ async function prData(params) {
try {
const pullRequestData = await fetchPullRequests(github, owner, repo, sha);
const currentVersion = await getReleaseVersionValue(github, owner, repo);
const releaseBranchSha = await fetchReleaseBranchSha(github, owner, repo);
const mainBranchSha = await fetchMainBranchSha(github, owner, repo);

const labels = pullRequestData.data[0].labels;
const prNumber = pullRequestData.data[0].number;
Expand All @@ -106,7 +106,7 @@ async function prData(params) {
);

return {
releaseBranchSha,
mainBranchSha,
currentVersion,
newVersion,
label,
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ jobs:
role-skip-session-tagging: true
role-duration-seconds: 900

- name: Upload Env File to S3
run: |
aws s3 cp cd/application-deployment/${{ inputs.environment }}/${{ inputs.environment }}.env s3://vanotify-environment-variables-${{ inputs.environment }}/
- name: Login to VAEC ECR
id: login-ecr-vaec
uses: aws-actions/amazon-ecr-login@v2
Expand Down
32 changes: 2 additions & 30 deletions .github/workflows/cd-pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,35 +65,8 @@ jobs:
- name: Pause for manual approval
run: echo "Deployment paused for manual approval."

merge-to-release:
needs: approval-deploy-staging
# These permissions are needed to write to the release branch
permissions:
contents: write
runs-on: ubuntu-latest
steps:
- name: Checkout release branch
uses: actions/checkout@v4
with:
ref: release
fetch-depth: 0
# Fine-grained PAT with contents:write and workflows:write
# scopes
token: ${{ secrets.CD_PAT }}

- name: Setup git user
# The following is taken from https://github.com/actions/checkout/issues/13 as a common work-around
run: |
git config --global user.name "$(git --no-pager log --format=format:'%an' -n 1)"
git config --global user.email "$(git --no-pager log --format=format:'%ae' -n 1)"
- name: Merge commit SHA to release
run: |
git merge ${{ github.sha }} --no-squash -X theirs
git push
create-and-post-tag:
needs: merge-to-release
needs: approval-deploy-staging
uses: ./.github/workflows/create-and-post-tag.yml
secrets: inherit

Expand Down Expand Up @@ -136,5 +109,4 @@ jobs:
with:
environment: prod
ref: ${{ needs.create-and-post-tag.outputs.newVersion }}
lambdaDeploy: true

lambdaDeploy: true
11 changes: 10 additions & 1 deletion .talismanrc
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,16 @@ fileignoreconfig:
checksum: b42aefd1ae0e6ea76e75db4cf14d425facd0941943b17f7ba2e41f850ad1ec23
- filename: app/template/rest.py
checksum: 1e5bdac8bc694d50f8f656dec127dd036b7b1b5b6156e3282d3411956c71ba0b
- filename: cd/application-deployment/dev/dev.env
checksum: a6bed7de359c7cec67940c1f0113826365400684c5a3bd182e8237d48ad5c1f1
- filename: cd/application-deployment/dev/vaec-api-task-definition.json
checksum: f328ff821339b802eb1d82559e624d5b719857c813d427da5aaa39b240331ddd
- filename: cd/application-deployment/perf/perf.env
checksum: 1b3b7539dd80b0661594082956e61fd86451692946d845cfe676798aac75618d
- filename: cd/application-deployment/prod/prod.env
checksum: 55252b1cb0e16b02301ae8bffb1015f7da5286d4bce0b415a95842cdb368c275
- filename: cd/application-deployment/staging/staging.env
checksum: 9e5161e8a0a13974d9b67d8a7e61d1b3fed9657a7e2dfeb6d82fd8ace64e2715
- filename: ci/docker-compose-test.yml
checksum: e3efec2749e8c19e60f5bfc68eafabe24eba647530a482ceccfc4e0e62cff424
- filename: lambda_functions/pinpoint_callback/pinpoint_callback_lambda.py
Expand All @@ -43,4 +51,5 @@ fileignoreconfig:
checksum: 3181930a13e3679bb2f17eaa3f383512eb9caf4ed5d5e14496ca4193c6083965
- filename: tests/lambda_functions/va_profile/test_va_profile_integration.py
checksum: f9fdfcde669c8ece589cf405a70170c88d21992b08a6fb07b624138b759984b9
version: "1.0"
- filename: app/va/va_profile/va_profile_client.py
checksum: fe634f26f7dc3874f4afcfd1ba3f03bae380b53befe973a752c7347097a88701
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ To support running locally, the repository includes a default `app/version.py` f
Running `flask db migrate` on the container ci_app_1 errors because the files in the migrations folder are read-only. Follow this procedure to create a database migration using Flask:

1. Ensure all containers are stopped and that the notification_api image has been built
2. Run `docker compose -f ci/docker-compose-local-migrate.yml up`. This creates the container ci_app_migrate with your local notification-api directory mounted in read-write mode. The container runs `flask db migrate` and exits.
3. Press Ctrl-C to stop the containers, and identify the new file in `migrations/versions/`.
2. Run migrations at least once e.g. `docker compose -f ci/docker-compose-local.yml up` and stop any running containers.
3. Run `docker compose -f ci/docker-compose-local-migrate.yml up`. This creates the container ci_app_migrate with your local notification-api directory mounted in read-write mode. The container runs `flask db migrate` and exits.
4. Press Ctrl-C to stop the containers, and identify the new file in `migrations/versions/`.

### Unit testing
See the [tests README.md](tests/README.md) for information.
Expand Down
1 change: 1 addition & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@

DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%S.%fZ'
DATE_FORMAT = '%Y-%m-%d'
HTTP_TIMEOUT = (3.05, 1) if os.getenv('NOTIFY_ENVIRONMENT') in ('production', 'staging') else (30, 30)

load_dotenv()

Expand Down
4 changes: 2 additions & 2 deletions app/callback/webhook_callback_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from requests.api import request
from requests.exceptions import HTTPError, RequestException

from app import statsd_client
from app import statsd_client, HTTP_TIMEOUT
from app.callback.service_callback_strategy_interface import ServiceCallbackStrategyInterface
from app.celery.exceptions import NonRetryableException, RetryableException
from app.dao.api_key_dao import get_unsigned_secret
Expand All @@ -32,7 +32,7 @@ def send_callback(
'Content-Type': 'application/json',
'Authorization': 'Bearer {}'.format(callback.bearer_token),
},
timeout=(3.05, 1),
timeout=HTTP_TIMEOUT,
)
current_app.logger.info('Callback sent to %s, response %d, %s', callback.url, response.status_code, tags)
response.raise_for_status()
Expand Down
54 changes: 10 additions & 44 deletions app/celery/contact_information_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
update_notification_status_by_id,
)
from app.exceptions import NotificationTechnicalFailureException, NotificationPermanentFailureException
from app.feature_flags import FeatureFlag, is_feature_enabled
from app.models import (
NOTIFICATION_PERMANENT_FAILURE,
NOTIFICATION_PREFERENCES_DECLINED,
Expand Down Expand Up @@ -49,8 +48,7 @@ def lookup_contact_info(
):
"""
Celery task to look up contact information (email/phone number) for a given notification.
If the feature flag, VA_PROFILE_V3_COMBINE_CONTACT_INFO_AND_PERMISSIONS_LOOKUP, is enabled,
also check for related communication permissions.
Also check for related communication permissions.
Args:
self (Task): The Celery task instance.
Expand All @@ -71,50 +69,16 @@ def lookup_contact_info(
recipient_identifier = notification.recipient_identifiers[IdentifierType.VA_PROFILE_ID.value]

try:
if is_feature_enabled(FeatureFlag.VA_PROFILE_V3_COMBINE_CONTACT_INFO_AND_PERMISSIONS_LOOKUP):
result = get_profile_result(notification, recipient_identifier)
notification.to = result.recipient
if not result.communication_allowed:
handle_communication_not_allowed(notification, recipient_identifier, result.permission_message)
# Otherwise, this communication is allowed. We will update the notification below and continue the chain.
else:
notification.to = get_recipient(
notification.notification_type,
notification_id,
recipient_identifier,
)
result = get_profile_result(notification, recipient_identifier)
notification.to = result.recipient
if not result.communication_allowed:
handle_communication_not_allowed(notification, recipient_identifier, result.permission_message)
# Otherwise, this communication is allowed. We will update the notification below and continue the chain.
dao_update_notification(notification)
except Exception as e:
handle_lookup_contact_info_exception(self, notification, recipient_identifier, e)


def get_recipient(
notification_type: str,
notification_id: str,
recipient_identifier: RecipientIdentifier,
) -> str:
"""
Retrieve the recipient email or phone number.
Args:
notification_type (str): The type of recipient info requested.
notification_id (str): The notification ID associated with this request.
recipient_identifier (RecipientIdentifier): The VA profile ID to retrieve the profile for.
Returns:
str: The recipient email or phone number.
"""
if notification_type == EMAIL_TYPE:
return va_profile_client.get_email(recipient_identifier)
elif notification_type == SMS_TYPE:
return va_profile_client.get_telephone(recipient_identifier)
else:
raise NotImplementedError(
f'The task lookup_contact_info failed for notification {notification_id}. '
f'{notification_type} is not supported'
)


def get_profile_result(
notification: Notification,
recipient_identifier: RecipientIdentifier,
Expand All @@ -130,9 +94,9 @@ def get_profile_result(
VAProfileResult: The contact info result from VA Profile.
"""
if notification.notification_type == EMAIL_TYPE:
return va_profile_client.get_email_with_permission(recipient_identifier, notification)
return va_profile_client.get_email(recipient_identifier, notification)
elif notification.notification_type == SMS_TYPE:
return va_profile_client.get_telephone_with_permission(recipient_identifier, notification)
return va_profile_client.get_telephone(recipient_identifier, notification)
else:
raise NotImplementedError(
f'The task lookup_contact_info failed for notification {notification.id}. '
Expand Down Expand Up @@ -208,6 +172,8 @@ def handle_lookup_contact_info_exception(
else:
# Means the default_send is True and this does not require an explicit opt-in
return None
elif isinstance(e, NotificationPermanentFailureException):
raise e
else:
current_app.logger.exception(f'Unhandled exception for notification {notification.id}: {e}')
raise e
Expand Down
Loading

0 comments on commit 5a966d1

Please sign in to comment.