From 55f5cbd382437f0c6c780300f50179c807b6e30a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 2 Aug 2023 02:06:38 +0000 Subject: [PATCH 01/53] Bump cryptography from 41.0.2 to 41.0.3 in /src Bumps [cryptography](https://github.com/pyca/cryptography) from 41.0.2 to 41.0.3. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](https://github.com/pyca/cryptography/compare/41.0.2...41.0.3) --- updated-dependencies: - dependency-name: cryptography dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- src/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/requirements.txt b/src/requirements.txt index 75bdf8438..5216999cb 100644 --- a/src/requirements.txt +++ b/src/requirements.txt @@ -7,7 +7,7 @@ certifi==2023.5.7 ; python_version >= '3.6' cfenv==0.5.3 cffi==1.15.1 charset-normalizer==3.1.0 ; python_full_version >= '3.7.0' -cryptography==41.0.2 ; python_version >= '3.7' +cryptography==41.0.3 ; python_version >= '3.7' defusedxml==0.7.1 ; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4' dj-database-url==2.0.0 dj-email-url==1.0.6 From b8efec75c4e3f203843aa11dac5a28457ce712e4 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 9 Aug 2023 14:58:30 -0700 Subject: [PATCH 02/53] Adding myself to fixtures --- src/registrar/fixtures.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 01de666a4..2c94a1eb4 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -67,6 +67,11 @@ class UserFixture: "first_name": "Paul", "last_name": "Kuykendall", }, + { + "username": "2a88a97b-be96-4aad-b99e-0b605b492c78", + "first_name": "Rebecca", + "last_name": "Hsieh", + }, ] STAFF = [ @@ -90,6 +95,11 @@ class UserFixture: "first_name": "Paul-Analyst", "last_name": "Kuykendall-Analyst", }, + { + "username": "e474e7a9-71ca-449d-833c-8a6e094dd117", + "first_name": "Rebecca-Analyst", + "last_name": "Hsieh-Analyst", + }, ] STAFF_PERMISSIONS = [ From b156c86aef2bab1a518c0d54c249017ae1300e3a Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Thu, 10 Aug 2023 16:15:02 -0400 Subject: [PATCH 03/53] remove broad css selector for body color --- src/registrar/assets/sass/_theme/_admin.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 4488ed398..2e5da018e 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -105,7 +105,6 @@ html[data-theme="light"] { } // Dark mode django (bug due to scss cascade) and USWDS tables - body, .change-list .usa-table, .change-list .usa-table--striped tbody tr:nth-child(odd) td { color: var(--body-fg)!important; @@ -114,7 +113,6 @@ html[data-theme="light"] { // Firefox needs this to be specifically set html[data-theme="dark"] { - body, .change-list .usa-table, .change-list .usa-table--striped tbody tr:nth-child(odd) td { color: var(--body-fg)!important; From 88e1d419da6306753fc3a2698695b2a110e25a88 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Thu, 10 Aug 2023 16:27:00 -0400 Subject: [PATCH 04/53] Fix bug by checking to see if first item in the result set for a change_list_results table row is a form before performing the checkbox slice/markup switch --- .../templates/admin/change_list_results.html | 114 +++++++++++------- 1 file changed, 68 insertions(+), 46 deletions(-) diff --git a/src/registrar/templates/admin/change_list_results.html b/src/registrar/templates/admin/change_list_results.html index a9c5e5b4d..34f733ad4 100644 --- a/src/registrar/templates/admin/change_list_results.html +++ b/src/registrar/templates/admin/change_list_results.html @@ -13,57 +13,49 @@ Load our custom filters to extract info from the django generated markup. {% endif %} {% if results %}
- +
-{# .gov - hardcode the select all checkbox #} - -{# .gov - don't let django generate the select all checkbox #} -{% for header in result_headers|slice:"1:" %} +{% if results.0.form %} + {# .gov - hardcode the select all checkbox #} + + {# .gov - don't let django generate the select all checkbox #} + {% for header in result_headers|slice:"1:" %} + {% endfor %} -{% endfor %} + + + - - - + {% comment %} + .gov - hardcode the row checkboxes using the custom filters to extract + the value attribute's value, and a label based on the anchor elements's + text. Then edit the for loop to keep django from generating the row select + checkboxes. + {% endcomment %} -{% comment %} -{% for result in results %} -{% if result.form.non_field_errors %} - -{% endif %} -{% for item in result %}{{ item }}{% endfor %} -{% endfor %} -{% endcomment %} - -{% comment %} -.gov - hardcode the row checkboxes using the custom filters to extract -the value attribute's value, and a label based on the anchor elements's -text. Then edit the for loop to keep django from generating the row select -checkboxes. -{% endcomment %} -{% for result in results %} + {% for result in results %} {% if result.form.non_field_errors %} {% endif %} @@ -82,7 +74,37 @@ checkboxes. {{ item }} {% endfor %} -{% endfor %} + {% endfor %} + +{% else %} + + {% for header in result_headers %} + {% endfor %} + + + + + + {% for result in results %} + {% if result.form.non_field_errors %} + + {% endif %} + {% for item in result %}{{ item }}{% endfor %} + {% endfor %} + +{% endif %}
-
- - - - -
-
-		 +
+ + + + +
+
+ 		+ {% if header.sortable %} + {% if header.sort_priority > 0 %} +
+ + {% if num_sorted_fields > 1 %}{{ header.sort_priority }}{% endif %} + +
+ {% endif %} + {% endif %} +
{% if header.sortable %}{{ header.text|capfirst }}{% else %}{{ header.text|capfirst }}{% endif %}
+
+ 		- {% if header.sortable %} - {% if header.sort_priority > 0 %} -
- - {% if num_sorted_fields > 1 %}{{ header.sort_priority }}{% endif %} - -
- {% endif %} - {% endif %} -
{% if header.sortable %}{{ header.text|capfirst }}{% else %}{{ header.text|capfirst }}{% endif %}
-
-
{{ result.form.non_field_errors }}
{{ result.form.non_field_errors }}
+ {% if header.sortable %} + {% if header.sort_priority > 0 %} +
+ + {% if num_sorted_fields > 1 %}{{ header.sort_priority }}{% endif %} + +
+ {% endif %} + {% endif %} +
{% if header.sortable %}{{ header.text|capfirst }}{% else %}{{ header.text|capfirst }}{% endif %}
+
+
{{ result.form.non_field_errors }}
From 5e64848a2ecb63bd01d071f2c41df8c04618213e Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Fri, 11 Aug 2023 00:51:44 -0400 Subject: [PATCH 05/53] Update developer-onboarding.md Small edits, add link to team onboarding doc, replacing an legacy 18F charter --- .github/ISSUE_TEMPLATE/developer-onboarding.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/developer-onboarding.md b/.github/ISSUE_TEMPLATE/developer-onboarding.md index 11b2d86fc..8d0f9c2d8 100644 --- a/.github/ISSUE_TEMPLATE/developer-onboarding.md +++ b/.github/ISSUE_TEMPLATE/developer-onboarding.md @@ -24,13 +24,13 @@ There are several tools we use locally that you will need to have. ### Steps for the onboardee - [ ] Setup [commit signing in Github](#setting-up-commit-signing) and with git locally. - [ ] [Create a cloud.gov account](https://cloud.gov/docs/getting-started/accounts/) -- [ ] Have an admin add you to the CISA Github organization and Dotgov Team. +- [ ] Email github@cisa.dhs.gov (cc: Cameron) to add you to the [CISA Github organization](https://github.com/getgov) and [.gov Team](https://github.com/orgs/cisagov/teams/gov). - [ ] Ensure you can login to your cloud.gov account via the CLI ```bash cf login -a api.fr.cloud.gov --sso ``` - [ ] Have an admin add you to cloud.gov org and set up your [sandbox developer space](#setting-up-developer-sandbox). Ensure you can deploy to your sandbox space. -- [ ] Have an admin add you to our login.gov sandbox team (`.gov registrar poc`) via the [dashboard](https://dashboard.int.identitysandbox.gov/). +- [ ] Have an admin add you to our login.gov sandbox team (`.gov Registrar`) via the [dashboard](https://dashboard.int.identitysandbox.gov/). **Note:** As mentioned in the [Login documentation](https://developers.login.gov/testing/), the sandbox Login account is different account from your regular, production Login account. If you have not created a Login account for the sandbox before, you will need to create a new account first. @@ -39,12 +39,12 @@ cf login -a api.fr.cloud.gov --sso ### Steps for the onboarder - [ ] Add the onboardee to cloud.gov org (cisa-getgov-prototyping) - [ ] Setup a [developer specific space for the new developer](#setting-up-developer-sandbox) -- [ ] Add the onboardee to our login.gov sandbox team (`.gov registrar poc`) via the [dashboard](https://dashboard.int.identitysandbox.gov/) +- [ ] Add the onboardee to our login.gov sandbox team (`.gov Registrar`) via the [dashboard](https://dashboard.int.identitysandbox.gov/) ## Documents to Review -- [ ] [Team Charter](https://docs.google.com/document/d/1xhMKlW8bMcxyF7ipsOYxw1SQYVi-lWPkcDHSUS6miNg/edit), in particular our Github Policy +- [ ] [Team Onboarding](https://docs.google.com/document/d/1ukbpW4LSqkb_CCt8LWfpehP03qqfyYfvK3Fl21NaEq8/edit?usp=sharing) - [ ] [Architecture Decision Records](https://github.com/cisagov/dotgov/tree/main/docs/architecture/decisions) - [ ] [Contributing Policy](https://github.com/cisagov/dotgov/tree/main/CONTRIBUTING.md) From a2f0fd9522d3f039e73f0dd0317ef8b7be41ecb1 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 11 Aug 2023 12:18:24 -0400 Subject: [PATCH 06/53] revert USDWDS table markup which was removed by mistake --- src/registrar/templates/admin/change_list_results.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/admin/change_list_results.html b/src/registrar/templates/admin/change_list_results.html index 34f733ad4..fb77bd785 100644 --- a/src/registrar/templates/admin/change_list_results.html +++ b/src/registrar/templates/admin/change_list_results.html @@ -13,7 +13,7 @@ Load our custom filters to extract info from the django generated markup. {% endif %} {% if results %}
- +
From 02556418f882b4a45b18547d0354e580b18e4a62 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Fri, 11 Aug 2023 12:24:06 -0400 Subject: [PATCH 07/53] explain the 'results doesn't have a form as its first element', which is far away from its if --- src/registrar/templates/admin/change_list_results.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/admin/change_list_results.html b/src/registrar/templates/admin/change_list_results.html index fb77bd785..9ee3f9f59 100644 --- a/src/registrar/templates/admin/change_list_results.html +++ b/src/registrar/templates/admin/change_list_results.html @@ -76,7 +76,7 @@ Load our custom filters to extract info from the django generated markup. {% endfor %} -{% else %} +{% else %} {# results doesn't have a form as its first element #} {% for header in result_headers %}
From 9c26a971632c3d8ace2f0d9cbc0a59f9a5e78ffc Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 11 Aug 2023 12:12:37 -0700 Subject: [PATCH 08/53] Add troubleshooting notes for m1 puppeteer issue --- docs/developer/README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/developer/README.md b/docs/developer/README.md index b6938298c..f048f99cf 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -18,9 +18,14 @@ If you're new to Django, see [Getting Started with Django](https://www.djangopro Visit the running application at [http://localhost:8080](http://localhost:8080). -Note: If you are using Windows, you may need to change your [line endings](https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings). If not, you may not be able to run manage.py. +**Note:** If you are using Windows, you may need to change your [line endings](https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings). If not, you may not be able to run manage.py. * Unix based operating systems (like macOS or Linux) handle line separators [differently than Windows does](https://superuser.com/questions/374028/how-are-n-and-r-handled-differently-on-linux-and-windows). This can break bash scripts in particular. In the case of manage.py, it uses *#!/usr/bin/env python* to access the Python executable. Since the script is still thinking in terms of unix line seperators, it may look for the executable *python\r* rather than *python* (since Windows cannot read the carriage return on its own) - thus leading to the error `usr/bin/env: 'python\r' no such file or directory` * If you'd rather not change this globally, add a `.gitattributes` file in the project root with `* text eol=lf` as the text content, and [refresh the repo](https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings#refreshing-a-repository-after-changing-line-endings) +**Note:** If you are using a Mac with a M1 chip, and see this error `The chromium binary is not available for arm64.` or an error invovling `puppeteer`, try adding this line below into your `.bashrc` or `.zshrc` + +``` +export DOCKER_DEFAULT_PLATFORM=linux/amd64 +``` ## Branch Conventions From 5f6a7cd044dc0d4b4d7c9feae4e74dfb306f2b39 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Sun, 13 Aug 2023 19:40:30 -0600 Subject: [PATCH 09/53] Fixes the logout bug It works! --- src/registrar/config/settings.py | 2 +- src/registrar/config/urls.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index f6873b226..f16efc18f 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -285,7 +285,7 @@ SERVER_EMAIL = "root@get.gov" # Content-Security-Policy configuration # this can be restrictive because we have few external scripts -allowed_sources = ("'self'",) +allowed_sources = ("'self'", "https://idp.int.identitysandbox.gov", "https://idp.int.identitysandbox.gov/openid_connect/logout") CSP_DEFAULT_SRC = allowed_sources # Most things fall back to default-src, but these two do not and should be # explicitly set diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index c21d0206c..6159b387b 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -45,6 +45,10 @@ for step, view in [ urlpatterns = [ path("", views.index, name="home"), + path( + "admin/logout/", + RedirectView.as_view(url="/openid/logout", permanent=False), + ), path("admin/", admin.site.urls), path( "application//edit/", From ec7f70e8aef4335d2239874eb8dc63fe76a1194c Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 14 Aug 2023 11:35:42 -0400 Subject: [PATCH 10/53] Fix table header contract in dark mode --- src/registrar/assets/sass/_theme/_admin.scss | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 2e5da018e..fb7e1e0da 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -106,16 +106,22 @@ html[data-theme="light"] { // Dark mode django (bug due to scss cascade) and USWDS tables .change-list .usa-table, - .change-list .usa-table--striped tbody tr:nth-child(odd) td { - color: var(--body-fg)!important; + .change-list .usa-table--striped tbody tr:nth-child(odd) td, + .change-list .usa-table--borderless thead th, + .change-list .usa-table thead td, + .change-list .usa-table thead th { + color: var(--body-fg); } } // Firefox needs this to be specifically set html[data-theme="dark"] { .change-list .usa-table, - .change-list .usa-table--striped tbody tr:nth-child(odd) td { - color: var(--body-fg)!important; + .change-list .usa-table--striped tbody tr:nth-child(odd) td, + .change-list .usa-table--borderless thead th, + .change-list .usa-table thead td, + .change-list .usa-table thead th { + color: var(--body-fg); } } From a3ab395c30a45bb474e9a10e8229a8feeae7841e Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Mon, 14 Aug 2023 09:09:04 -0700 Subject: [PATCH 11/53] updated PR template --- .github/pull_request_template.md | 95 +++++++++++++++++++++++++++++--- 1 file changed, 86 insertions(+), 9 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 1eeef397b..3b707182c 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,13 +1,90 @@ -# # +# Ticket -## đź—Ł Description ## +Resolves #00 - - +## Changes -## đź’­ Motivation and context ## + +- Change 1 +- Change 2 - - - - \ No newline at end of file + + +## Context for reviewers + + + +## Setup + + + +## Code Review Verification Steps + +### As the original developer, I have + +#### Satisfied acceptance criteria and met development standards + +- [ ] Met the acceptance criteria, or will meet them in a subsequent PR +- [ ] Created/modified automated tests +- [ ] Added at least 2 developers as PR reviewers (only 1 will need to approve) +- [ ] Messaged on Slack or in standup to notify the team that a PR is ready for review + +#### Validated user-facing changes (if applicable) + +- [ ] Checked keyboard navigability +- [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI) +- [ ] Add at least 1 designer as PR reviewer + +### As code reviewer(s), I have + +#### Reviewed, tested, and left feedback about the changes + +- [ ] Pulled this branch locally and tested it +- [ ] Reviewed this code and left comments +- [ ] Checked that all code is adequately covered by tests +- [ ] Made it clear which comments need to be addressed before this work is merged +- [ ] Considered marking this as accepted even if there are small changes needed + +#### Validated user-facing changes as a developer + +- [ ] Checked keyboard navigability +- [ ] Meets all designs and user flows provided by design/product +- [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI) + +### As a designer reviewer, I have + +#### Verified that the changes match the design intention + +- [ ] Checked in the design translated visually +- [ ] Checked behavior +- [ ] Checked different states (empty, one, some, error) +- [ ] Checked for landmarks, page heading structure, and links +- [ ] Tried to break the intended flow + +#### Validated user-facing changes as a designer + +- [ ] Checked keyboard navigability +- [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI) + +## Screenshots + + From e1f27d79286ac4b9a4742b1684404c5f0ceb27f3 Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Mon, 14 Aug 2023 09:10:53 -0700 Subject: [PATCH 12/53] reduced the size of the ticket header --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 3b707182c..b1936f94b 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,4 +1,4 @@ -# Ticket +## Ticket Resolves #00 From 17ac251c36f5100c4cc93c0cd3fa15ebe584c254 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 14 Aug 2023 10:17:29 -0600 Subject: [PATCH 13/53] Minor cleanup --- src/registrar/config/settings.py | 4 ++-- src/registrar/config/urls.py | 7 +------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index f16efc18f..a221d88fa 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -285,12 +285,12 @@ SERVER_EMAIL = "root@get.gov" # Content-Security-Policy configuration # this can be restrictive because we have few external scripts -allowed_sources = ("'self'", "https://idp.int.identitysandbox.gov", "https://idp.int.identitysandbox.gov/openid_connect/logout") +allowed_sources = ("'self'") CSP_DEFAULT_SRC = allowed_sources # Most things fall back to default-src, but these two do not and should be # explicitly set CSP_FRAME_ANCESTORS = allowed_sources -CSP_FORM_ACTION = allowed_sources +CSP_FORM_ACTION = ("'self'", "https://idp.int.identitysandbox.gov/openid_connect/logout") # Content-Length header is set by django.middleware.common.CommonMiddleware diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 6159b387b..ad3883f4f 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -47,7 +47,7 @@ urlpatterns = [ path("", views.index, name="home"), path( "admin/logout/", - RedirectView.as_view(url="/openid/logout", permanent=False), + RedirectView.as_view(pattern_name="logout", permanent=False), ), path("admin/", admin.site.urls), path( @@ -125,11 +125,6 @@ if not settings.DEBUG: path( "admin/login/", RedirectView.as_view(pattern_name="login", permanent=False) ), - # redirect to login.gov - path( - "admin/logout/", - RedirectView.as_view(pattern_name="logout", permanent=False), - ), ] # we normally would guard these with `if settings.DEBUG` but tests run with From 6082d8205b733b8987594602711be205ebddf647 Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Mon, 14 Aug 2023 09:17:43 -0700 Subject: [PATCH 14/53] minor rewording and typo fixes --- .github/pull_request_template.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index b1936f94b..d90d83f92 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -26,7 +26,10 @@ Resolves #00 echo "Code goes here" ``` - Example 2: if the PR was to add a new link with a redirect, this section could simply say go to /path/to/start/page. Click the blue link in the and see the user is redirected to + Example 2: If the PR was to add a new link with a redirect, this section could simply be: + -go to /path/to/start/page + -click the blue link in the + -notice user is redirected to --> ## Code Review Verification Steps @@ -66,7 +69,7 @@ Resolves #00 #### Verified that the changes match the design intention -- [ ] Checked in the design translated visually +- [ ] Checked that the design translated visually - [ ] Checked behavior - [ ] Checked different states (empty, one, some, error) - [ ] Checked for landmarks, page heading structure, and links From c295c22c8d00e5a33b94ef3f366343b686f3afaa Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Mon, 14 Aug 2023 11:28:28 -0500 Subject: [PATCH 15/53] Content Updates --- src/registrar/templates/application_other_contacts.html | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index 805cb6927..2adf0b99c 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -2,7 +2,13 @@ {% load static field_helpers %} {% block form_instructions %} -

We’d like to contact other employees in your organization about your domain request. For example, they could be involved in managing your organization or its technical infrastructure. This information will help us assess your eligibility for a .gov domain. These contacts should be in addition to you and your authorizing official. They should be employees of your organization.

+

To help us assess your eligibility for a .gov domain, please provide contact information for at least two other employees from your organization. +

    +
  • They must be employees who are clearly and publicly affiliated with your organization and familiar with your domain request.
    • +
  • They do not need to be involved with the technical management of your domain (although they can be).
    • +
  • We typically don’t reach out to these employees, but if contact is necessary, our practice is to coordinate with you, the requestor.
    • +
+

{% endblock %} From 24ba547e1433a93025a7f3d66bcc2de0a5efa733 Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Mon, 14 Aug 2023 09:31:46 -0700 Subject: [PATCH 16/53] Added items from old code review checklist --- .github/pull_request_template.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index d90d83f92..7848e2648 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -42,9 +42,18 @@ Resolves #00 - [ ] Created/modified automated tests - [ ] Added at least 2 developers as PR reviewers (only 1 will need to approve) - [ ] Messaged on Slack or in standup to notify the team that a PR is ready for review +- [ ] Changes to “how we do things” are documented in READMEs and or onboarding guide + +Code standards are met: + +- [ ] All new functions and methods are commented using plain language +- [ ] Did dependency updates in Pipfile also get changed in requirements.txt? +- [ ] Interactions with external systems are wrapped in try/except +- [ ] Error handling exists for unusual or missing values #### Validated user-facing changes (if applicable) +- [ ] New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing - [ ] Checked keyboard navigability - [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI) - [ ] Add at least 1 designer as PR reviewer @@ -55,12 +64,20 @@ Resolves #00 - [ ] Pulled this branch locally and tested it - [ ] Reviewed this code and left comments +- [ ] All new functions and methods are commented using plain language - [ ] Checked that all code is adequately covered by tests - [ ] Made it clear which comments need to be addressed before this work is merged -- [ ] Considered marking this as accepted even if there are small changes needed + +Code standards are met: + +- [ ] All new functions and methods are commented using plain language +- [ ] Did dependency updates in Pipfile also get changed in requirements.txt? +- [ ] Interactions with external systems are wrapped in try/except +- [ ] Error handling exists for unusual or missing values #### Validated user-facing changes as a developer +- [ ] New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing - [ ] Checked keyboard navigability - [ ] Meets all designs and user flows provided by design/product - [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI) From 8dc43211915556d2234ec30307089b00928ad4e7 Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Mon, 14 Aug 2023 09:34:54 -0700 Subject: [PATCH 17/53] changed subheader --- .github/pull_request_template.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 7848e2648..9f8eb1ea1 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -44,7 +44,7 @@ Resolves #00 - [ ] Messaged on Slack or in standup to notify the team that a PR is ready for review - [ ] Changes to “how we do things” are documented in READMEs and or onboarding guide -Code standards are met: +* Code standards are met: - [ ] All new functions and methods are commented using plain language - [ ] Did dependency updates in Pipfile also get changed in requirements.txt? @@ -68,7 +68,7 @@ Code standards are met: - [ ] Checked that all code is adequately covered by tests - [ ] Made it clear which comments need to be addressed before this work is merged -Code standards are met: +* Code standards are met: - [ ] All new functions and methods are commented using plain language - [ ] Did dependency updates in Pipfile also get changed in requirements.txt? From f7c9c1c42c4372ced3a59bf334cbc40901ddba2a Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 14 Aug 2023 12:39:34 -0400 Subject: [PATCH 18/53] Fix the approve() sets status to None bug by calling the transition methods on the current obj (as opposed to the old obj) in django admin --- src/registrar/admin.py | 27 +++++--- src/registrar/models/domain_application.py | 78 ++++++---------------- src/registrar/tests/test_admin.py | 27 ++++++++ 3 files changed, 65 insertions(+), 67 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 182543c19..a3e8e6d2c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -274,22 +274,27 @@ class DomainApplicationAdmin(ListHeaderAdmin): pass elif obj.status == models.DomainApplication.SUBMITTED: # This is an fsm in model which will throw an error if the - # transition condition is violated, so we call it on the - # original object which has the right status value, and pass - # the updated object which contains the up-to-date data - # for the side effects (like an email send). Same - # comment applies to original_obj method calls below. - original_obj.submit(updated_domain_application=obj) + # transition condition is violated, so we roll back the + # status to what it was before the admn user changed it and + # let the fsm method set it. Same comment applies to + # transition method calls below. + obj.status = original_obj.status + obj.submit() elif obj.status == models.DomainApplication.IN_REVIEW: - original_obj.in_review(updated_domain_application=obj) + obj.status = original_obj.status + obj.in_review() elif obj.status == models.DomainApplication.ACTION_NEEDED: - original_obj.action_needed(updated_domain_application=obj) + obj.status = original_obj.status + obj.action_needed() elif obj.status == models.DomainApplication.APPROVED: - original_obj.approve(updated_domain_application=obj) + obj.status = original_obj.status + obj.approve() elif obj.status == models.DomainApplication.WITHDRAWN: - original_obj.withdraw() + obj.status = original_obj.status + obj.withdraw() elif obj.status == models.DomainApplication.REJECTED: - original_obj.reject(updated_domain_application=obj) + obj.status = original_obj.status + obj.reject() else: logger.warning("Unknown status selected in django admin") diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 1f3d23d8c..67f1ee5d9 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -504,15 +504,10 @@ class DomainApplication(TimeStampedModel): @transition( field="status", source=[STARTED, ACTION_NEEDED, WITHDRAWN], target=SUBMITTED ) - def submit(self, updated_domain_application=None): + def submit(self): """Submit an application that is started. - As a side effect, an email notification is sent. - - This method is called in admin.py on the original application - which has the correct status value, but is passed the changed - application which has the up-to-date data that we'll use - in the email.""" + As a side effect, an email notification is sent.""" # check our conditions here inside the `submit` method so that we # can raise more informative exceptions @@ -528,46 +523,31 @@ class DomainApplication(TimeStampedModel): if not DraftDomain.string_could_be_domain(self.requested_domain.name): raise ValueError("Requested domain is not a valid domain name.") - if updated_domain_application is not None: - # A DomainApplication is being passed to this method (ie from admin) - updated_domain_application._send_status_update_email( - "submission confirmation", - "emails/submission_confirmation.txt", - "emails/submission_confirmation_subject.txt", - ) - else: - # Or this method is called with the right application - # for context, ie from views/application.py - self._send_status_update_email( - "submission confirmation", - "emails/submission_confirmation.txt", - "emails/submission_confirmation_subject.txt", - ) + self._send_status_update_email( + "submission confirmation", + "emails/submission_confirmation.txt", + "emails/submission_confirmation_subject.txt", + ) @transition(field="status", source=SUBMITTED, target=IN_REVIEW) - def in_review(self, updated_domain_application): + def in_review(self): """Investigate an application that has been submitted. - As a side effect, an email notification is sent. + As a side effect, an email notification is sent.""" - This method is called in admin.py on the original application - which has the correct status value, but is passed the changed - application which has the up-to-date data that we'll use - in the email.""" - - updated_domain_application._send_status_update_email( + self._send_status_update_email( "application in review", "emails/status_change_in_review.txt", "emails/status_change_in_review_subject.txt", ) @transition(field="status", source=[IN_REVIEW, REJECTED], target=ACTION_NEEDED) - def action_needed(self, updated_domain_application): + def action_needed(self): """Send back an application that is under investigation or rejected. - As a side effect, an email notification is sent, similar to in_review""" + As a side effect, an email notification is sent.""" - updated_domain_application._send_status_update_email( + self._send_status_update_email( "action needed", "emails/status_change_action_needed.txt", "emails/status_change_action_needed_subject.txt", @@ -576,19 +556,13 @@ class DomainApplication(TimeStampedModel): @transition( field="status", source=[SUBMITTED, IN_REVIEW, REJECTED], target=APPROVED ) - def approve(self, updated_domain_application=None): + def approve(self): """Approve an application that has been submitted. This has substantial side-effects because it creates another database object for the approved Domain and makes the user who created the application into an admin on that domain. It also triggers an email - notification. - - This method is called in admin.py on the original application - which has the correct status value, but is passed the changed - application which has the up-to-date data that we'll use - in the email. - """ + notification.""" # create the domain Domain = apps.get_model("registrar.Domain") @@ -607,31 +581,23 @@ class DomainApplication(TimeStampedModel): user=self.creator, domain=created_domain, role=UserDomainRole.Roles.ADMIN ) - if updated_domain_application is not None: - # A DomainApplication is being passed to this method (ie from admin) - updated_domain_application._send_status_update_email( - "application approved", - "emails/status_change_approved.txt", - "emails/status_change_approved_subject.txt", - ) - else: - self._send_status_update_email( - "application approved", - "emails/status_change_approved.txt", - "emails/status_change_approved_subject.txt", - ) + self._send_status_update_email( + "application approved", + "emails/status_change_approved.txt", + "emails/status_change_approved_subject.txt", + ) @transition(field="status", source=[SUBMITTED, IN_REVIEW], target=WITHDRAWN) def withdraw(self): """Withdraw an application that has been submitted.""" @transition(field="status", source=[IN_REVIEW, APPROVED], target=REJECTED) - def reject(self, updated_domain_application): + def reject(self): """Reject an application that has been submitted. As a side effect, an email notification is sent, similar to in_review""" - updated_domain_application._send_status_update_email( + self._send_status_update_email( "action needed", "emails/status_change_rejected.txt", "emails/status_change_rejected_subject.txt", diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 3c07e7608..627e4c627 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -152,6 +152,33 @@ class TestDomainApplicationAdmin(TestCase): # Perform assertions on the mock call itself mock_client_instance.send_email.assert_called_once() + + def test_save_model_sets_approved_domain(self): + # make sure there is no user with this email + EMAIL = "mayor@igorville.gov" + User.objects.filter(email=EMAIL).delete() + + # Create a sample application + application = completed_application(status=DomainApplication.IN_REVIEW) + + # Create a mock request + request = self.factory.post( + "/admin/registrar/domainapplication/{}/change/".format(application.pk) + ) + + # Create an instance of the model admin + model_admin = DomainApplicationAdmin(DomainApplication, self.site) + + # Modify the application's property + application.status = DomainApplication.APPROVED + + # Use the model admin's save_model method + model_admin.save_model(request, application, form=None, change=True) + + # Test that approved domain exists and equals requested domain + self.assertEqual( + application.requested_domain.name, application.approved_domain.name + ) @boto3_mocking.patching def test_save_model_sends_action_needed_email(self): From f6f6f54ec7a9109e0e76e3f3c29b9e8d66cd01d2 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Mon, 14 Aug 2023 11:44:09 -0500 Subject: [PATCH 19/53] Content Updates: Authorizing Official --- src/registrar/templates/application_authorizing_official.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/application_authorizing_official.html b/src/registrar/templates/application_authorizing_official.html index dead1974f..0674b3f7b 100644 --- a/src/registrar/templates/application_authorizing_official.html +++ b/src/registrar/templates/application_authorizing_official.html @@ -6,13 +6,13 @@ Who is the authorizing official for your organization? -

Your authorizing official is the person within your organization who can authorize your domain request. This is generally the highest-ranking or highest-elected official in your organization.

+

Your authorizing official is the person within your organization who can authorize your domain request. This must be a person in a role of significant, executive responsibility within the organization.

{% include "includes/ao_example.html" %}
-

We might contact your authorizing official, or their office, to double check that they approve this request. Read more about who can serve as an authorizing official.

+

We typically don’t reach out to the authorizing official, but if contact is necessary, our practice is to coordinate with you, the requestor. Read more about who can serve as an authorizing official.

{% endblock %} From e1d78bda5a44d5e272a8fe4982ac1d579cfcfbf4 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 14 Aug 2023 12:58:47 -0400 Subject: [PATCH 20/53] lint --- src/registrar/admin.py | 2 +- src/registrar/tests/test_admin.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a3e8e6d2c..96b8aaa33 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -275,7 +275,7 @@ class DomainApplicationAdmin(ListHeaderAdmin): elif obj.status == models.DomainApplication.SUBMITTED: # This is an fsm in model which will throw an error if the # transition condition is violated, so we roll back the - # status to what it was before the admn user changed it and + # status to what it was before the admin user changed it and # let the fsm method set it. Same comment applies to # transition method calls below. obj.status = original_obj.status diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 627e4c627..5f78eac3c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -152,7 +152,7 @@ class TestDomainApplicationAdmin(TestCase): # Perform assertions on the mock call itself mock_client_instance.send_email.assert_called_once() - + def test_save_model_sets_approved_domain(self): # make sure there is no user with this email EMAIL = "mayor@igorville.gov" From a74ee20bcf072af49c585bf9ae793b71af9bcb23 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Mon, 14 Aug 2023 12:34:57 -0500 Subject: [PATCH 21/53] AO wording adjustment --- src/registrar/templates/application_authorizing_official.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_authorizing_official.html b/src/registrar/templates/application_authorizing_official.html index 0674b3f7b..60fce85f8 100644 --- a/src/registrar/templates/application_authorizing_official.html +++ b/src/registrar/templates/application_authorizing_official.html @@ -6,7 +6,7 @@ Who is the authorizing official for your organization? -

Your authorizing official is the person within your organization who can authorize your domain request. This must be a person in a role of significant, executive responsibility within the organization.

+

Your authorizing official is the person within your organization who can authorize your domain request. This person must be in a role of significant, executive responsibility within the organization.

{% include "includes/ao_example.html" %} From 1c60724469dbb83b98b6f88068fc4e64c0c12106 Mon Sep 17 00:00:00 2001 From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com> Date: Mon, 14 Aug 2023 12:58:02 -0500 Subject: [PATCH 22/53] Wording adjustments: Other employees --- src/registrar/templates/application_other_contacts.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/application_other_contacts.html b/src/registrar/templates/application_other_contacts.html index 2adf0b99c..74568b592 100644 --- a/src/registrar/templates/application_other_contacts.html +++ b/src/registrar/templates/application_other_contacts.html @@ -2,10 +2,10 @@ {% load static field_helpers %} {% block form_instructions %} -

To help us assess your eligibility for a .gov domain, please provide contact information for at least two other employees from your organization. +

To help us assess your eligibility for a .gov domain, please provide contact information for other employees from your organization.

    -
  • They must be employees who are clearly and publicly affiliated with your organization and familiar with your domain request.
    • -
  • They do not need to be involved with the technical management of your domain (although they can be).
    • +
  • They should be clearly and publicly affiliated with your organization and familiar with your domain request.
    • +
  • They don't need to be involved with the technical management of your domain (although they can be).
  • We typically don’t reach out to these employees, but if contact is necessary, our practice is to coordinate with you, the requestor.

From 14e3548f81b98222ae9c648d461400c65d0a0963 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 14 Aug 2023 14:39:36 -0400 Subject: [PATCH 23/53] Fix body.dashboard color in dark mode (affects Recent Actions panel in admin index) --- src/registrar/assets/sass/_theme/_admin.scss | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index fb7e1e0da..1a8049086 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -109,7 +109,8 @@ html[data-theme="light"] { .change-list .usa-table--striped tbody tr:nth-child(odd) td, .change-list .usa-table--borderless thead th, .change-list .usa-table thead td, - .change-list .usa-table thead th { + .change-list .usa-table thead th, + body.dashboard { color: var(--body-fg); } } @@ -120,7 +121,8 @@ html[data-theme="dark"] { .change-list .usa-table--striped tbody tr:nth-child(odd) td, .change-list .usa-table--borderless thead th, .change-list .usa-table thead td, - .change-list .usa-table thead th { + .change-list .usa-table thead th, + body.dashboard { color: var(--body-fg); } } From 765502f322b0de8c5dac96926d7b60674567bc74 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Mon, 14 Aug 2023 14:48:16 -0400 Subject: [PATCH 24/53] Edit user fictures to add optional emails --- docs/developer/README.md | 2 ++ src/registrar/fixtures.py | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/docs/developer/README.md b/docs/developer/README.md index b6938298c..9ef62bba9 100644 --- a/docs/developer/README.md +++ b/docs/developer/README.md @@ -84,6 +84,7 @@ The endpoint /admin can be used to view and manage site content, including but n ``` 5. In the browser, navigate to /admin. To verify that all is working correctly, under "domain applications" you should see fake domains with various fake statuses. +6. Add an optional email key/value pair ### Adding an Analyst to /admin Analysts are a variant of the admin role with limited permissions. The process for adding an Analyst is much the same as adding an admin: @@ -105,6 +106,7 @@ Analysts are a variant of the admin role with limited permissions. The process f ``` 5. In the browser, navigate to /admin. To verify that all is working correctly, verify that you can only see a sub-section of the modules and some are set to view-only. +6. Add an optional email key/value pair Do note that if you wish to have both an analyst and admin account, append `-Analyst` to your first and last name, or use a completely different first/last name to avoid confusion. Example: `Bob-Analyst` ## Adding to CODEOWNERS (optional) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 2c94a1eb4..2291001de 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -79,6 +79,7 @@ class UserFixture: "username": "319c490d-453b-43d9-bc4d-7d6cd8ff6844", "first_name": "Rachid-Analyst", "last_name": "Mrad-Analyst", + "email": "rachid.mrad@gmail.com", }, { "username": "b6a15987-5c88-4e26-8de2-ca71a0bdb2cd", @@ -129,6 +130,8 @@ class UserFixture: user.is_superuser = True user.first_name = admin["first_name"] user.last_name = admin["last_name"] + if "email" in admin.keys(): + user.email = admin["email"] user.is_staff = True user.is_active = True user.save() @@ -146,6 +149,8 @@ class UserFixture: user.is_superuser = False user.first_name = staff["first_name"] user.last_name = staff["last_name"] + if "email" in admin.keys(): + user.email = admin["email"] user.is_staff = True user.is_active = True From a67e13b04bfbe41a998e204bfa24fa2e83855fdf Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Mon, 14 Aug 2023 12:06:06 -0700 Subject: [PATCH 25/53] added mutliple browsers and note for applicant vs analyst --- .github/pull_request_template.md | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 9f8eb1ea1..0bb9ef041 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -44,7 +44,7 @@ Resolves #00 - [ ] Messaged on Slack or in standup to notify the team that a PR is ready for review - [ ] Changes to “how we do things” are documented in READMEs and or onboarding guide -* Code standards are met: +##### Code standards are met (Original Developer) - [ ] All new functions and methods are commented using plain language - [ ] Did dependency updates in Pipfile also get changed in requirements.txt? @@ -64,16 +64,15 @@ Resolves #00 - [ ] Pulled this branch locally and tested it - [ ] Reviewed this code and left comments -- [ ] All new functions and methods are commented using plain language - [ ] Checked that all code is adequately covered by tests - [ ] Made it clear which comments need to be addressed before this work is merged -* Code standards are met: +##### Code standards are met (Code reviewer) - [ ] All new functions and methods are commented using plain language -- [ ] Did dependency updates in Pipfile also get changed in requirements.txt? - [ ] Interactions with external systems are wrapped in try/except - [ ] Error handling exists for unusual or missing values +- [ ] (Rarely needed) Did dependency updates in Pipfile also get changed in requirements.txt? #### Validated user-facing changes as a developer @@ -82,6 +81,14 @@ Resolves #00 - [ ] Meets all designs and user flows provided by design/product - [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI) +- [ ] Tested with multiple browsers, the suggestion is to use ones that the developer didn't (check off which ones were used) + - [ ] Chrome + - [ ] Microsoft Edge + - [ ] FireFox + - [ ] Safari + +- [ ] (Rarely needed) Test as both an analyst and applicant user + ### As a designer reviewer, I have #### Verified that the changes match the design intention @@ -97,6 +104,14 @@ Resolves #00 - [ ] Checked keyboard navigability - [ ] Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI) +- [ ] Tested with multiple browsers (check off which ones were used) + - [ ] Chrome + - [ ] Microsoft Edge + - [ ] FireFox + - [ ] Safari + +- [ ] (Rarely needed) Test as both an analyst and applicant user + ## Screenshots ## Changes @@ -10,7 +15,7 @@ Resolves #00 ## Context for reviewers From 2a47df7eab4ca3b00db9b4f030f5e434efa74a17 Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Mon, 14 Aug 2023 14:22:06 -0700 Subject: [PATCH 33/53] fixed typo --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 3c4313f55..73fabb9f2 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -96,7 +96,7 @@ All other changes require just a single approving review.--> - [ ] (Rarely needed) Tested as both an analyst and applicant user -**Note:** Multiple code reviews can share the checklists above, a second reviewers should not make a duplicate checklist +**Note:** Multiple code reviewers can share the checklists above, a second reviewers should not make a duplicate checklist ### As a designer reviewer, I have From 6faa21168abca9577b26442dfe6c112c75af3641 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 14 Aug 2023 20:16:04 -0700 Subject: [PATCH 34/53] Temporarily remove Export Domains section --- src/registrar/templates/home.html | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 792beec43..c34c244c3 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -125,13 +125,14 @@

You don't have any archived domains

-
+ +
From 94ab136058dd8dac5c8ca0b8cf1c6607987eda9d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 15 Aug 2023 07:26:34 -0600 Subject: [PATCH 35/53] Update src/registrar/config/settings.py allowed_sources was not a tuple: https://django-csp.readthedocs.io/en/latest/configuration.html Co-authored-by: Neil MartinsenBurrell --- src/registrar/config/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 39917ad42..f6873b226 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -285,7 +285,7 @@ SERVER_EMAIL = "root@get.gov" # Content-Security-Policy configuration # this can be restrictive because we have few external scripts -allowed_sources = ("'self'") +allowed_sources = ("'self'",) CSP_DEFAULT_SRC = allowed_sources # Most things fall back to default-src, but these two do not and should be # explicitly set From 3a6acd9c2958fc4819ec5f2f49218334741ca24c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 15 Aug 2023 07:56:09 -0600 Subject: [PATCH 36/53] Added translation Requires {% load i18n %} within this scope as per docs: https://docs.djangoproject.com/en/4.1/topics/i18n/translation/#internationalization-in-template-code --- src/registrar/templates/admin/base_site.html | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/registrar/templates/admin/base_site.html b/src/registrar/templates/admin/base_site.html index 72bd7af21..6b641722f 100644 --- a/src/registrar/templates/admin/base_site.html +++ b/src/registrar/templates/admin/base_site.html @@ -1,5 +1,6 @@ {% extends "admin/base.html" %} {% load static %} +{% load i18n %} {% block title %}{% if subtitle %}{{ subtitle }} | {% endif %}{{ title }} | {{ site_title|default:_('Django site admin') }}{% endblock %} @@ -13,20 +14,25 @@ {% include "admin/color_theme_toggle.html" %} {% endif %} {% endblock %} +{% comment %} + This was copied from the 'userlinks' template, with a few minor changes. + You can find that here: + https://github.com/django/django/blob/d25f3892114466d689fd6936f79f3bd9a9acc30e/django/contrib/admin/templates/admin/base.html#L59 +{% endcomment %} {% block userlinks %} {% if site_url %} - View site / + {% translate 'View site' %} / {% endif %} {% if user.is_active and user.is_staff %} {% url 'django-admindocs-docroot' as docsroot %} {% if docsroot %} - Documentation / + {% translate 'Documentation' %} / {% endif %} {% endif %} {% if user.has_usable_password %} - Change password / + {% translate 'Change password' %} / {% endif %} - Log out + {% translate 'Log out' %} {% include "admin/color_theme_toggle.html" %} {% endblock %} {% block nav-global %}{% endblock %} \ No newline at end of file From df04f2c31255cadf4a3b83214bddb115f42ad6d8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 15 Aug 2023 09:20:34 -0600 Subject: [PATCH 37/53] Update urls.py --- src/registrar/config/urls.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 67ab630e1..133a44a96 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -118,17 +118,6 @@ urlpatterns = [ ), ] -# What is the purpose of this? -# This behaviour gets overwritten, so this doesn't do anything... -# Login in particular -if not settings.DEBUG: - urlpatterns += [ - # redirect to login.gov - path( - "admin/login/", RedirectView.as_view(pattern_name="login", permanent=False) - ), - ] - # we normally would guard these with `if settings.DEBUG` but tests run with # DEBUG = False even when these apps have been loaded because settings.DEBUG # was actually True. Instead, let's add these URLs any time we are able to From 32abc8fd3073b30cfe0115bae3fbba7c31c95749 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 15 Aug 2023 09:24:45 -0600 Subject: [PATCH 38/53] Removed unused import --- src/registrar/config/urls.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 133a44a96..0f136c932 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -4,7 +4,6 @@ For more information see: https://docs.djangoproject.com/en/4.0/topics/http/urls/ """ -from django.conf import settings from django.contrib import admin from django.urls import include, path from django.views.generic import RedirectView From 18b34af9daf4c74cab6d8ddb5693e1524ea7090a Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 15 Aug 2023 11:33:17 -0700 Subject: [PATCH 39/53] Update zap for false positives --- src/zap.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/zap.conf b/src/zap.conf index e5e7b4d04..d44447b8e 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -30,6 +30,8 @@ # UNCLEAR WHY THIS ONE IS FAILING. Giving 404 error. 10027 OUTOFSCOPE http://app:8080/public/js/uswds-init.min.js # get-gov.js contains suspicious word "from" as in `Array.from()` +10027 OUTOFSCOPE http://app:8080/public/src/registrar/templates/home.html +# Contains suspicious word "TODO" which isn't that suspicious 10027 OUTOFSCOPE http://app:8080/public/js/get-gov.js 10028 FAIL (Open Redirect - Passive/beta) 10029 FAIL (Cookie Poisoning - Passive/beta) From 170e07eeff0fe8ba6ce2023701c219bba15f8172 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 15 Aug 2023 11:42:48 -0700 Subject: [PATCH 40/53] Remove from Zap and change wording --- src/registrar/templates/home.html | 2 +- src/zap.conf | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index c34c244c3..30724de8e 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -125,7 +125,7 @@

You don't have any archived domains

- + From a0cf217551f062ff1434fb4cb7b2a4289cb5d7ba Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 15 Aug 2023 11:57:20 -0700 Subject: [PATCH 42/53] Update to just note --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 32ef48bcf..7b901e747 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -129,7 +129,7 @@ From 068694fa77154cf7b0b86578c5096215a8b338b1 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 15 Aug 2023 12:04:02 -0700 Subject: [PATCH 43/53] Testing out a diff urlpattern to not use todo wording --- src/registrar/templates/home.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 7b901e747..638367904 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -129,7 +129,7 @@ From 962d70dccc6661e685ea45f78ff2f28e69e39544 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 15 Aug 2023 12:08:18 -0700 Subject: [PATCH 44/53] Add back todo path and attempt zap again --- src/registrar/templates/home.html | 2 +- src/zap.conf | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/home.html b/src/registrar/templates/home.html index 638367904..30724de8e 100644 --- a/src/registrar/templates/home.html +++ b/src/registrar/templates/home.html @@ -129,7 +129,7 @@ diff --git a/src/zap.conf b/src/zap.conf index e5e7b4d04..c1782760b 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -30,6 +30,8 @@ # UNCLEAR WHY THIS ONE IS FAILING. Giving 404 error. 10027 OUTOFSCOPE http://app:8080/public/js/uswds-init.min.js # get-gov.js contains suspicious word "from" as in `Array.from()` +10027 OUTOFSCOPE http://app:8080/todo +# Ignore wording of "TODO" 10027 OUTOFSCOPE http://app:8080/public/js/get-gov.js 10028 FAIL (Open Redirect - Passive/beta) 10029 FAIL (Cookie Poisoning - Passive/beta) From d4de131ecdebb048d4c9b2f937d9cdd9dcb310f0 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 15 Aug 2023 12:13:29 -0700 Subject: [PATCH 45/53] Add comment to the top correctly this time --- src/zap.conf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/zap.conf b/src/zap.conf index c1782760b..1f9e831fb 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -30,9 +30,9 @@ # UNCLEAR WHY THIS ONE IS FAILING. Giving 404 error. 10027 OUTOFSCOPE http://app:8080/public/js/uswds-init.min.js # get-gov.js contains suspicious word "from" as in `Array.from()` -10027 OUTOFSCOPE http://app:8080/todo -# Ignore wording of "TODO" 10027 OUTOFSCOPE http://app:8080/public/js/get-gov.js +# Ignore wording of "TODO" +10027 OUTOFSCOPE http://app:8080/todo 10028 FAIL (Open Redirect - Passive/beta) 10029 FAIL (Cookie Poisoning - Passive/beta) 10030 FAIL (User Controllable Charset - Passive/beta) From b8beccd24ac779a792e6f7a1c89b9551e49eb841 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 15 Aug 2023 12:24:23 -0700 Subject: [PATCH 46/53] Switch to tab instead of spaces --- src/zap.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zap.conf b/src/zap.conf index 1f9e831fb..adf51c72c 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -32,7 +32,7 @@ # get-gov.js contains suspicious word "from" as in `Array.from()` 10027 OUTOFSCOPE http://app:8080/public/js/get-gov.js # Ignore wording of "TODO" -10027 OUTOFSCOPE http://app:8080/todo +10027 OUTOFSCOPE http://app:8080/todo 10028 FAIL (Open Redirect - Passive/beta) 10029 FAIL (Cookie Poisoning - Passive/beta) 10030 FAIL (User Controllable Charset - Passive/beta) From 1dc0d22a1e0e269aa870b4f47ad29dc9a2914e24 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 15 Aug 2023 12:36:55 -0700 Subject: [PATCH 47/53] Update path --- src/zap.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/zap.conf b/src/zap.conf index adf51c72c..bdd6b017d 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -32,7 +32,7 @@ # get-gov.js contains suspicious word "from" as in `Array.from()` 10027 OUTOFSCOPE http://app:8080/public/js/get-gov.js # Ignore wording of "TODO" -10027 OUTOFSCOPE http://app:8080/todo +10027 OUTOFSCOPE http://app:8080.*$ 10028 FAIL (Open Redirect - Passive/beta) 10029 FAIL (Cookie Poisoning - Passive/beta) 10030 FAIL (User Controllable Charset - Passive/beta) From f3d4a90d884d361dba6b1c7a414b50171873fe8c Mon Sep 17 00:00:00 2001 From: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> Date: Tue, 15 Aug 2023 12:40:22 -0700 Subject: [PATCH 48/53] removed # for subheader --- .github/pull_request_template.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 73fabb9f2..f65007b2b 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -50,7 +50,7 @@ All other changes require just a single approving review.--> - [ ] Changes to “how we do things” are documented in READMEs and or onboarding guide - [ ] If any model was updated to modify/add/delete columns, makemigrations was ran and the assoicated migrations file has been commited. -##### Ensured code standards are met (Original Developer) +#### Ensured code standards are met (Original Developer) - [ ] All new functions and methods are commented using plain language - [ ] Did dependency updates in Pipfile also get changed in requirements.txt? @@ -74,7 +74,7 @@ All other changes require just a single approving review.--> - [ ] Made it clear which comments need to be addressed before this work is merged - [ ] If any model was updated to modify/add/delete columns, makemigrations was ran and the assoicated migrations file has been commited. -##### Ensured code standards are met (Code reviewer) +#### Ensured code standards are met (Code reviewer) - [ ] All new functions and methods are commented using plain language - [ ] Interactions with external systems are wrapped in try/except From 4c9ddf727ed330e919b2f1409f49bc16f272d7a5 Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Wed, 16 Aug 2023 11:53:55 -0400 Subject: [PATCH 49/53] Fix contrast issue with filter headers in django admin --- src/registrar/assets/sass/_theme/_admin.scss | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 1a8049086..b87257344 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -110,7 +110,9 @@ html[data-theme="light"] { .change-list .usa-table--borderless thead th, .change-list .usa-table thead td, .change-list .usa-table thead th, - body.dashboard { + body.dashboard, + body.change-list, + body.change-form { color: var(--body-fg); } } @@ -122,7 +124,9 @@ html[data-theme="dark"] { .change-list .usa-table--borderless thead th, .change-list .usa-table thead td, .change-list .usa-table thead th, - body.dashboard { + body.dashboard, + body.change-list, + body.change-form { color: var(--body-fg); } } From 5e24e3473db83d419e7221318cd1cacdef3de43c Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Wed, 16 Aug 2023 14:05:52 -0400 Subject: [PATCH 50/53] lint --- src/registrar/fixtures.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/fixtures.py b/src/registrar/fixtures.py index 2291001de..0b1b8926d 100644 --- a/src/registrar/fixtures.py +++ b/src/registrar/fixtures.py @@ -130,7 +130,7 @@ class UserFixture: user.is_superuser = True user.first_name = admin["first_name"] user.last_name = admin["last_name"] - if "email" in admin.keys(): + if "email" in admin.keys(): user.email = admin["email"] user.is_staff = True user.is_active = True @@ -149,7 +149,7 @@ class UserFixture: user.is_superuser = False user.first_name = staff["first_name"] user.last_name = staff["last_name"] - if "email" in admin.keys(): + if "email" in admin.keys(): user.email = admin["email"] user.is_staff = True user.is_active = True From 051b852cd9f8d663344d21731edc51a8627073eb Mon Sep 17 00:00:00 2001 From: rachidatecs <107004823+rachidatecs@users.noreply.github.com> Date: Wed, 16 Aug 2023 14:56:37 -0400 Subject: [PATCH 51/53] Update docs/architecture/decisions/0021-django-admin.md Co-authored-by: Neil MartinsenBurrell --- docs/architecture/decisions/0021-django-admin.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/decisions/0021-django-admin.md b/docs/architecture/decisions/0021-django-admin.md index bca51c2ef..add6992cd 100644 --- a/docs/architecture/decisions/0021-django-admin.md +++ b/docs/architecture/decisions/0021-django-admin.md @@ -34,7 +34,7 @@ In contrast to building an admin interface from scratch where development activi involve _building up_, leveraging Django Admin will require carefully _pairing back_ the functionalities available to users such as analysts. -On accessibility: Django admin is almost fully accessible out-of-the-box, the expections being tables, checkboxes, and +On accessibility: Django admin is almost fully accessible out-of-the-box, the exceptions being tables, checkboxes, and color contrast. We have remedied the first 2 with template overrides and the 3rd with theming (see below). On USWDS and theming: Django admin brings its own high level design framework. We have determined that theming on top of Django (scss) From 4c937f1b0922d9da78d811a55e2d8ce184a7764b Mon Sep 17 00:00:00 2001 From: rachidatecs Date: Thu, 17 Aug 2023 16:00:14 -0400 Subject: [PATCH 52/53] Update contributing doc with branch naming convention and approvals information --- CONTRIBUTING.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d745f76c7..ab15c660f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,6 +9,22 @@ There are a handful of things we do not commit to the repository: - Compliance documentation that includes IP addresses - Secrets of any kind +## Branch naming convention + +For developers, you can auto-deploy your code to your sandbox (if applicable) by naming your branch thusly: jsd/123-feature-description +Where 'jsd' stands for your initials and sandbox environment name (if you were called John Smith Doe), and 123 matches the ticket number if applicable. + +## Approvals + +When a code change is made that is not user facing, then the following is required: +- a developer approves the PR + +When a code change is made that is user facing, beyond content updates, then the following are required: +- a developer approves the PR +- a designer approves the PR or checks off all relevant items in this checklist + +Content or document updates require a single person to approve. + ## Project Management We use [Github Projects](https://docs.github.com/en/issues/planning-and-tracking-with-projects/learning-about-projects/about-projects) for project management and tracking. From 587702496280c29704de08d4c27729d01067a368 Mon Sep 17 00:00:00 2001 From: Alysia Broddrick Date: Fri, 18 Aug 2023 16:20:02 -0700 Subject: [PATCH 53/53] changed received to submitted --- src/registrar/templates/application_status.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/application_status.html b/src/registrar/templates/application_status.html index 2488bb449..99f6a1d4c 100644 --- a/src/registrar/templates/application_status.html +++ b/src/registrar/templates/application_status.html @@ -22,7 +22,7 @@ {% if domainapplication.status == 'approved' %} Approved {% elif domainapplication.status == 'in review' %} In Review {% elif domainapplication.status == 'rejected' %} Rejected - {% elif domainapplication.status == 'submitted' %} Received + {% elif domainapplication.status == 'submitted' %} Submitted {% else %}ERROR Please contact technical support/dev {% endif %}