From 34d727748bfac7ca8ac40eecb77c349721d9d88a Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 26 Jul 2024 09:15:09 -0700 Subject: [PATCH 01/13] cleanup notify-compliance-46 --- app/notify_client/__init__.py | 20 +++++++++++-------- .../test_notify_admin_api_client.py | 19 ++++++++++++++++++ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py index 5f65e85c6..ea3bd8e61 100644 --- a/app/notify_client/__init__.py +++ b/app/notify_client/__init__.py @@ -56,6 +56,9 @@ class NotifyAdminAPIClient(BaseAPIClient): ): abort(403) + def is_calling_signin_url(self, arg): + return arg[0].startswith("/user") + def check_inactive_user(self, *args): still_signing_in = False @@ -64,14 +67,15 @@ class NotifyAdminAPIClient(BaseAPIClient): # and we only want to check the first arg for arg in args: arg = str(arg) - if ( - "get-login-gov-user" in arg - or "user/email" in arg - or "/activate" in arg - or "/email-code" in arg - or "/verify/code" in arg - or "/user" in arg - ): + if self.is_calling_signin_url(arg): + # if ( + # "get-login-gov-user" in arg + # or "user/email" in arg + # or "/activate" in arg + # or "/email-code" in arg + # or "/verify/code" in arg + # or "/user" in arg + # ): still_signing_in = True # This seems to be a weird edge case that happens intermittently with invites diff --git a/tests/app/notify_client/test_notify_admin_api_client.py b/tests/app/notify_client/test_notify_admin_api_client.py index e68d4669f..ec5de4522 100644 --- a/tests/app/notify_client/test_notify_admin_api_client.py +++ b/tests/app/notify_client/test_notify_admin_api_client.py @@ -41,6 +41,25 @@ def test_active_service_can_be_modified(notify_admin, method, user, service): assert ret == request.return_value +@pytest.mark.parametrize( + ("arg", "expected_result"), + [ + ( + ("/user/c5f8a5c9-56d5-4fa9-8c30-3449ae10c072/verify/code",), + True, + ), + ( + ("/service/blahblahblah",), + False, + ), + ], +) +def test_is_calling_signin_url(arg, expected_result): + api_client = NotifyAdminAPIClient() + result = api_client.is_calling_signin_url(arg) + assert result == expected_result + + @pytest.mark.parametrize("method", ["put", "post", "delete"]) def test_inactive_service_cannot_be_modified_by_normal_user( notify_admin, api_user_active, method From e884f31fa08e86f4d2485672646b51f280a67004 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 26 Jul 2024 09:49:12 -0700 Subject: [PATCH 02/13] write test --- app/notify_client/__init__.py | 2 +- tests/app/notify_client/test_notify_admin_api_client.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py index ea3bd8e61..3d7fd62a8 100644 --- a/app/notify_client/__init__.py +++ b/app/notify_client/__init__.py @@ -57,7 +57,7 @@ class NotifyAdminAPIClient(BaseAPIClient): abort(403) def is_calling_signin_url(self, arg): - return arg[0].startswith("/user") + return arg.startswith("('/user") def check_inactive_user(self, *args): still_signing_in = False diff --git a/tests/app/notify_client/test_notify_admin_api_client.py b/tests/app/notify_client/test_notify_admin_api_client.py index ec5de4522..d15eebfc3 100644 --- a/tests/app/notify_client/test_notify_admin_api_client.py +++ b/tests/app/notify_client/test_notify_admin_api_client.py @@ -45,11 +45,12 @@ def test_active_service_can_be_modified(notify_admin, method, user, service): ("arg", "expected_result"), [ ( - ("/user/c5f8a5c9-56d5-4fa9-8c30-3449ae10c072/verify/code",), + "('/user/c5f8a5c9-56d5-4fa9-8c30-3449ae10c072/verify/code',)", True, ), + ("('/user/get-login-gov-user',)", True), ( - ("/service/blahblahblah",), + "('/service/blahblahblah',)", False, ), ], From af59b13836ff069405d0cfff70e09b15c61765a4 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Fri, 2 Aug 2024 14:30:35 -0700 Subject: [PATCH 03/13] code review feedback --- app/notify_client/__init__.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py index 3d7fd62a8..8db3425f2 100644 --- a/app/notify_client/__init__.py +++ b/app/notify_client/__init__.py @@ -68,14 +68,6 @@ class NotifyAdminAPIClient(BaseAPIClient): for arg in args: arg = str(arg) if self.is_calling_signin_url(arg): - # if ( - # "get-login-gov-user" in arg - # or "user/email" in arg - # or "/activate" in arg - # or "/email-code" in arg - # or "/verify/code" in arg - # or "/user" in arg - # ): still_signing_in = True # This seems to be a weird edge case that happens intermittently with invites From cd1cedad693c036c2d8dd083cfa995e64e1017d5 Mon Sep 17 00:00:00 2001 From: Beverly Nguyen Date: Mon, 5 Aug 2024 12:09:38 -0700 Subject: [PATCH 04/13] add debug staging api to docs --- docs/debug-issues-with-staging-api.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 docs/debug-issues-with-staging-api.md diff --git a/docs/debug-issues-with-staging-api.md b/docs/debug-issues-with-staging-api.md new file mode 100644 index 000000000..e80a2ac32 --- /dev/null +++ b/docs/debug-issues-with-staging-api.md @@ -0,0 +1,26 @@ +### Setting Up Environment Variables for Local Development to the Staging API + +When you’re working locally, you can point your local admin repo to the staging API and use that to help debug +issues with the staging data set. To do this, you’ll need to modify your .env file for the admin project and +include the following new environment variables: + +- `ADMIN_CLIENT_SECRET` +- `ADMIN_CLIENT_USERNAME` +- `DANGEROUS_SALT` +- `SECRET_KEY` + +Additionally, update `API_HOST_NAME` and `NOTIFY_ENVIRONMENT`: + +1. Change `API_HOST_NAME` to `API_HOST_NAME=https://notify-api-staging.app.cloud.gov` +2. Change `NOTIFY_ENVIRONMENT` to `NOTIFY_ENVIRONMENT=staging` + +### Retrieving Environment Variables for Staging + +You can retrieve the values needed for these by using the cf CLI (Cloud Foundry CLI tool) and making sure +you’re targeting the notify-staging space. + +1. `cf login -a [api.fr.cloud.gov](http://api.fr.cloud.gov/) --sso` +2. select `notify-staging` +3. `cf env notify-admin-staging` + +By pointing your local environment to staging, it should mirror what's in staging. From 89eb24c795070ef3c38dc292e9aa3a659267ed9b Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 6 Aug 2024 08:37:43 -0700 Subject: [PATCH 05/13] add ds.baseline --- .ds.baseline | 4 ++-- .github/workflows/checks.yml | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.ds.baseline b/.ds.baseline index 6d2034588..e397d83f8 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -141,7 +141,7 @@ "filename": ".github/workflows/checks.yml", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 101, + "line_number": 102, "is_secret": false } ], @@ -692,5 +692,5 @@ } ] }, - "generated_at": "2024-07-24T14:13:02Z" + "generated_at": "2024-08-06T15:37:18Z" } diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 71ede96c5..a35ed404b 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -84,6 +84,7 @@ jobs: ports: # Maps tcp port 6379 on service container to the host - 6379:6379 + if: ${{ github.actor != 'dependabot[bot]' }} steps: - uses: actions/checkout@v4 - uses: ./.github/actions/setup-project From b77a142f6b76d132bdfc94626eaee1c94f75fdf6 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Wed, 7 Aug 2024 07:37:04 -0700 Subject: [PATCH 06/13] try moving if block --- .ds.baseline | 6 +++--- .github/workflows/checks.yml | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.ds.baseline b/.ds.baseline index e397d83f8..f7f116b20 100644 --- a/.ds.baseline +++ b/.ds.baseline @@ -133,7 +133,7 @@ "filename": ".github/workflows/checks.yml", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 67, + "line_number": 68, "is_secret": false }, { @@ -141,7 +141,7 @@ "filename": ".github/workflows/checks.yml", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 102, + "line_number": 103, "is_secret": false } ], @@ -692,5 +692,5 @@ } ] }, - "generated_at": "2024-08-06T15:37:18Z" + "generated_at": "2024-08-07T14:36:40Z" } diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index a35ed404b..efeefdbd6 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -54,6 +54,7 @@ jobs: run: poetry run coverage report --fail-under=90 end-to-end-tests: + if: ${{ github.actor != 'dependabot[bot]' }} permissions: checks: write pull-requests: write @@ -84,7 +85,7 @@ jobs: ports: # Maps tcp port 6379 on service container to the host - 6379:6379 - if: ${{ github.actor != 'dependabot[bot]' }} + steps: - uses: actions/checkout@v4 - uses: ./.github/actions/setup-project From 3bafa485779023dcffd948f6ce387bafd4dcd7ba Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 8 Aug 2024 16:39:07 +0000 Subject: [PATCH 07/13] Bump botocore from 1.34.150 to 1.34.156 Bumps [botocore](https://github.com/boto/botocore) from 1.34.150 to 1.34.156. - [Changelog](https://github.com/boto/botocore/blob/develop/CHANGELOG.rst) - [Commits](https://github.com/boto/botocore/compare/1.34.150...1.34.156) --- updated-dependencies: - dependency-name: botocore dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- poetry.lock | 10 +++++----- pyproject.toml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/poetry.lock b/poetry.lock index 865e6da97..6e4ba5a7f 100644 --- a/poetry.lock +++ b/poetry.lock @@ -201,13 +201,13 @@ crt = ["botocore[crt] (>=1.21.0,<2.0a0)"] [[package]] name = "botocore" -version = "1.34.150" +version = "1.34.156" description = "Low-level, data-driven core of boto 3." optional = false python-versions = ">=3.8" files = [ - {file = "botocore-1.34.150-py3-none-any.whl", hash = "sha256:b988d47f4d502df85befce11a48002421e4e6ea4289997b5e0261bac5fa76ce6"}, - {file = "botocore-1.34.150.tar.gz", hash = "sha256:4d23387e0f076d87b637a2a35c0ff2b8daca16eace36b63ce27f65630c6b375a"}, + {file = "botocore-1.34.156-py3-none-any.whl", hash = "sha256:c48f8c8996216dfdeeb0aa6d3c0f2c7ae25234766434a2ea3e57bdc08494bdda"}, + {file = "botocore-1.34.156.tar.gz", hash = "sha256:5d1478c41ab9681e660b3322432fe09c4055759c317984b7b8d3af9557ff769a"}, ] [package.dependencies] @@ -216,7 +216,7 @@ python-dateutil = ">=2.1,<3.0.0" urllib3 = {version = ">=1.25.4,<2.2.0 || >2.2.0,<3", markers = "python_version >= \"3.10\""} [package.extras] -crt = ["awscrt (==0.20.11)"] +crt = ["awscrt (==0.21.2)"] [[package]] name = "cachecontrol" @@ -3092,4 +3092,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = "^3.12.2" -content-hash = "b271104f669ce0a8e78fb09299b61cf0502cc81a18213dda00f77c759b6e0209" +content-hash = "9d6309a76755b2639d787f99944c1ead0bd93ab9f2f13208f88c51cecb5e0081" diff --git a/pyproject.toml b/pyproject.toml index 5a9dc8727..915f23610 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,7 +39,7 @@ markdown = "^3.5.2" async-timeout = "^4.0.3" bleach = "^6.1.0" boto3 = "^1.34.150" -botocore = "^1.34.150" +botocore = "^1.34.156" cachetools = "^5.4.0" cffi = "^1.16.0" cryptography = "^43.0.0" From 21da7ef7db724c597067d3fd81779734a356b85d Mon Sep 17 00:00:00 2001 From: Carlo Costino Date: Thu, 8 Aug 2024 13:49:49 -0400 Subject: [PATCH 08/13] Update docs/debug-issues-with-staging-api.md Some additional formatting. --- docs/debug-issues-with-staging-api.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/debug-issues-with-staging-api.md b/docs/debug-issues-with-staging-api.md index e80a2ac32..517eab4d9 100644 --- a/docs/debug-issues-with-staging-api.md +++ b/docs/debug-issues-with-staging-api.md @@ -16,8 +16,8 @@ Additionally, update `API_HOST_NAME` and `NOTIFY_ENVIRONMENT`: ### Retrieving Environment Variables for Staging -You can retrieve the values needed for these by using the cf CLI (Cloud Foundry CLI tool) and making sure -you’re targeting the notify-staging space. +You can retrieve the values needed for these by using the `cf` CLI (Cloud Foundry CLI tool) and making sure +you’re targeting the `notify-staging` space. 1. `cf login -a [api.fr.cloud.gov](http://api.fr.cloud.gov/) --sso` 2. select `notify-staging` From 28bc4befbef2f2e85caf48de5248e3065b28777c Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Thu, 8 Aug 2024 16:29:35 -0400 Subject: [PATCH 09/13] Small changes to the data viz colors and chart sizing --- app/assets/javascripts/activityChart.js | 2 +- app/assets/javascripts/totalMessagesChart.js | 4 ++-- app/assets/sass/uswds/_data-visualization.scss | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/activityChart.js b/app/assets/javascripts/activityChart.js index 75913c145..b5139f157 100644 --- a/app/assets/javascripts/activityChart.js +++ b/app/assets/javascripts/activityChart.js @@ -4,7 +4,7 @@ const COLORS = { delivered: '#0076d6', - failed: '#fa9441', + failed: '#c6cace', text: '#666' }; diff --git a/app/assets/javascripts/totalMessagesChart.js b/app/assets/javascripts/totalMessagesChart.js index ff409c969..755d46f6e 100644 --- a/app/assets/javascripts/totalMessagesChart.js +++ b/app/assets/javascripts/totalMessagesChart.js @@ -21,7 +21,7 @@ var svg = d3.select("#totalMessageChart"); var width = chartContainer.clientWidth; - var height = 64; + var height = 50; // Ensure the width is set correctly if (width === 0) { @@ -62,7 +62,7 @@ .attr("x", 0) // Initially set to 0, will be updated during animation .attr("y", 0) .attr("height", height) - .attr("fill", '#fa9441') + .attr("fill", '#c6cace') .attr("width", 0) // Start with width 0 for animation .on('mouseover', function(event) { tooltip.style('display', 'block') diff --git a/app/assets/sass/uswds/_data-visualization.scss b/app/assets/sass/uswds/_data-visualization.scss index 6139933be..7740aad3f 100644 --- a/app/assets/sass/uswds/_data-visualization.scss +++ b/app/assets/sass/uswds/_data-visualization.scss @@ -2,7 +2,7 @@ $delivered: color('blue-50v'); $pending: color('green-cool-40v'); -$failed: color('orange-30v'); +$failed: color('gray-cool-20'); .chart-container { display: flex; From a395ad028171bcbfb090f3e635455f2f7d246fe5 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Thu, 8 Aug 2024 16:30:34 -0400 Subject: [PATCH 10/13] Multiples of 8 --- app/assets/javascripts/totalMessagesChart.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/totalMessagesChart.js b/app/assets/javascripts/totalMessagesChart.js index 755d46f6e..9a0c8b325 100644 --- a/app/assets/javascripts/totalMessagesChart.js +++ b/app/assets/javascripts/totalMessagesChart.js @@ -21,7 +21,7 @@ var svg = d3.select("#totalMessageChart"); var width = chartContainer.clientWidth; - var height = 50; + var height = 48; // Ensure the width is set correctly if (width === 0) { From 3e472e0a972a4f975b2149c137f0b7cf9b3e08da Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Thu, 8 Aug 2024 16:35:09 -0400 Subject: [PATCH 11/13] Test update --- tests/javascripts/totalMessagesChart.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/javascripts/totalMessagesChart.test.js b/tests/javascripts/totalMessagesChart.test.js index 22bc39500..b5cf3e6a8 100644 --- a/tests/javascripts/totalMessagesChart.test.js +++ b/tests/javascripts/totalMessagesChart.test.js @@ -57,7 +57,7 @@ test('SVG element is correctly set up', done => { setTimeout(() => { const svg = document.getElementById('totalMessageChart'); expect(svg.getAttribute('width')).toBe('600'); - expect(svg.getAttribute('height')).toBe('64'); + expect(svg.getAttribute('height')).toBe('48'); done(); }, 1000); // Ensure enough time for the DOM updates }); From 45bc6a91732afebe96f5e531d13347b02724e55f Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Thu, 8 Aug 2024 16:53:56 -0400 Subject: [PATCH 12/13] test updates --- app/assets/javascripts/totalMessagesChart.js | 2 +- tests/javascripts/totalMessagesChart.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/totalMessagesChart.js b/app/assets/javascripts/totalMessagesChart.js index 9a0c8b325..bebd6e106 100644 --- a/app/assets/javascripts/totalMessagesChart.js +++ b/app/assets/javascripts/totalMessagesChart.js @@ -62,7 +62,7 @@ .attr("x", 0) // Initially set to 0, will be updated during animation .attr("y", 0) .attr("height", height) - .attr("fill", '#c6cace') + .attr("fill", '#C7CACE') .attr("width", 0) // Start with width 0 for animation .on('mouseover', function(event) { tooltip.style('display', 'block') diff --git a/tests/javascripts/totalMessagesChart.test.js b/tests/javascripts/totalMessagesChart.test.js index b5cf3e6a8..87c671430 100644 --- a/tests/javascripts/totalMessagesChart.test.js +++ b/tests/javascripts/totalMessagesChart.test.js @@ -159,7 +159,7 @@ test('SVG bars are created and animated correctly', done => { // Initial check const sentBar = svg.querySelector('rect[fill="#0076d6"]'); - const remainingBar = svg.querySelector('rect[fill="#fa9441"]'); + const remainingBar = svg.querySelector('rect[fill="#C7CACE"]'); expect(sentBar).not.toBeNull(); expect(remainingBar).not.toBeNull(); From 832932f3b8ca75b33f7c6d77efc8c405a4277906 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Fri, 9 Aug 2024 10:33:51 -0400 Subject: [PATCH 13/13] Updates to styles and js --- app/assets/javascripts/activityChart.js | 2 +- app/assets/javascripts/totalMessagesChart.js | 2 +- app/assets/sass/uswds/_data-visualization.scss | 4 ++++ app/templates/views/dashboard/dashboard.html | 4 ---- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/activityChart.js b/app/assets/javascripts/activityChart.js index b5139f157..75913c145 100644 --- a/app/assets/javascripts/activityChart.js +++ b/app/assets/javascripts/activityChart.js @@ -4,7 +4,7 @@ const COLORS = { delivered: '#0076d6', - failed: '#c6cace', + failed: '#fa9441', text: '#666' }; diff --git a/app/assets/javascripts/totalMessagesChart.js b/app/assets/javascripts/totalMessagesChart.js index bebd6e106..a3c10d7a3 100644 --- a/app/assets/javascripts/totalMessagesChart.js +++ b/app/assets/javascripts/totalMessagesChart.js @@ -14,7 +14,7 @@ document.getElementById('message').innerText = `${sms_sent.toLocaleString()} sent / ${sms_remaining_messages.toLocaleString()} remaining`; // Calculate minimum width for "Messages Sent" as 1% of the total chart width - var minSentPercentage = 0.01; // Minimum width as a percentage of total messages (1% in this case) + var minSentPercentage = 0.02; // Minimum width as a percentage of total messages (1% in this case) var minSentValue = totalMessages * minSentPercentage; var displaySent = Math.max(sms_sent, minSentValue); var displayRemaining = totalMessages - displaySent; diff --git a/app/assets/sass/uswds/_data-visualization.scss b/app/assets/sass/uswds/_data-visualization.scss index 7740aad3f..48c5f1256 100644 --- a/app/assets/sass/uswds/_data-visualization.scss +++ b/app/assets/sass/uswds/_data-visualization.scss @@ -11,6 +11,10 @@ $failed: color('gray-cool-20'); } } +#totalMessageChartContainer { + max-width: 600px; +} + .bar { border-radius: units(0.5); &.delivered, &.usage { diff --git a/app/templates/views/dashboard/dashboard.html b/app/templates/views/dashboard/dashboard.html index eeca2b213..2c1af4909 100644 --- a/app/templates/views/dashboard/dashboard.html +++ b/app/templates/views/dashboard/dashboard.html @@ -28,10 +28,6 @@
-

- What counts as 1 text message part?
- See Tracking usage. -

Activity snapshot