From 072738cf03113b3535894a171271334eb0b6630c Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:04:37 -0800 Subject: [PATCH 01/37] Add alert to user domain role delete confirmation page --- src/registrar/admin.py | 4 ++++ .../admin/user_domain_role_delete_confirmation.html | 13 +++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e89147b11..f0e435090 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1369,9 +1369,13 @@ class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): change_form_template = "django/admin/user_domain_role_change_form.html" + # Override for the delete confirmation page on the domain table (bulk delete action) + delete_selected_confirmation_template = "django/admin/user_domain_role_delete_confirmation.html" + # Fixes a bug where non-superusers are redirected to the main page def delete_view(self, request, object_id, extra_context=None): """Custom delete_view implementation that specifies redirect behaviour""" + self.delete_confirmation_template = "django/admin/user_domain_role_delete_confirmation.html" response = super().delete_view(request, object_id, extra_context) if isinstance(response, HttpResponseRedirect) and not request.user.has_perm("registrar.full_access_permission"): diff --git a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html new file mode 100644 index 000000000..807502a31 --- /dev/null +++ b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html @@ -0,0 +1,13 @@ +{% extends 'admin/delete_confirmation.html' %} +{% load i18n static %} + +{% block content %} + + {{ block.super }} +{% endblock %} From ea31249ef80a0bfd7db00e15578720a4a0598a0c Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 23 Jan 2025 16:06:15 -0800 Subject: [PATCH 02/37] Move alert to content subtitle --- .../django/admin/user_domain_role_delete_confirmation.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html index 807502a31..171f4c3ea 100644 --- a/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html +++ b/src/registrar/templates/django/admin/user_domain_role_delete_confirmation.html @@ -1,7 +1,7 @@ {% extends 'admin/delete_confirmation.html' %} {% load i18n static %} -{% block content %} +{% block content_subtitle %} From 117bfa9b169a07876354323f1b6b3ba1c5b0ee97 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Fri, 31 Jan 2025 14:07:16 -0800 Subject: [PATCH 15/37] Add space to domain manager deleted email template --- .../templates/emails/domain_manager_deleted_notification.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/domain_manager_deleted_notification.txt b/src/registrar/templates/emails/domain_manager_deleted_notification.txt index b16b74dec..af0b92a8c 100644 --- a/src/registrar/templates/emails/domain_manager_deleted_notification.txt +++ b/src/registrar/templates/emails/domain_manager_deleted_notification.txt @@ -1,7 +1,7 @@ {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} Hi,{% if domain_manager and domain_manager.first_name %} {{ domain_manager.first_name }}.{% endif %} -A domain manager was removed from {{ domain.name }}. +A domain manager was removed from {{ domain.name }}. REMOVED BY: {{ removed_by.email }} REMOVED ON: {{ date }} MANAGER REMOVED: {{ manager_removed.email }} From fa27f26942814a308e6f571685ee5aaf20f0e437 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Mon, 3 Feb 2025 16:03:18 -0500 Subject: [PATCH 16/37] yaml file and docs --- .github/workflows/delete-and-recreate-db.yaml | 91 +++++++++++++++++++ docs/developer/workflows/README.md | 7 ++ .../workflows/delete-and-recreate-db.md | 13 +++ 3 files changed, 111 insertions(+) create mode 100644 .github/workflows/delete-and-recreate-db.yaml create mode 100644 docs/developer/workflows/README.md create mode 100644 docs/developer/workflows/delete-and-recreate-db.md diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml new file mode 100644 index 000000000..96a698392 --- /dev/null +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -0,0 +1,91 @@ +# This workflow can be run from the CLI +# gh workflow run reset-db.yaml -f environment=ENVIRONMENT + +name: Reset database +run-name: Reset database for ${{ github.event.inputs.environment }} + +on: + workflow_dispatch: + inputs: + environment: + type: choice + description: Which environment should we flush and re-load data for? + options: + - el + - ad + - ms + - ag + - litterbox + - hotgov + - cb + - bob + - meoward + - backup + - ky + - es + - nl + - rh + - za + - gd + - rb + - ko + - ab + - rjm + - dk + +jobs: + reset-db: + runs-on: ubuntu-latest + env: + CF_USERNAME: CF_${{ github.event.inputs.environment }}_USERNAME + CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD + DESTINATION_ENVIRONMENT: ${{ github.event.inputs.environment}} + steps: + - name: Dekete and Recreate Database + env: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + run: | + # install cf cli and other tools + wget -q -O - https://packages.cloudfoundry.org/debian/cli.cloudfoundry.org.key | sudo gpg --dearmor -o /usr/share/keyrings/cli.cloudfoundry.org.gpg + echo "deb [signed-by=/usr/share/keyrings/cli.cloudfoundry.org.gpg] https://packages.cloudfoundry.org/debian stable main" | sudo tee /etc/apt/sources.list.d/cloudfoundry-cli.list + + sudo apt-get update + sudo apt-get install cf8-cli + cf api api.fr.cloud.gov + cf auth "$CF_USERNAME" "$CF_PASSWORD" + cf target -o cisa-dotgov -s $DESTINATION_ENVIRONMENT + + + + #unbind the service + cf unbind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database + #delete the service key + yes Y | cf delete-service-key getgov-$DESTINATION_ENVIRONMENT-database SERVICE_CONNECT + #delete the service + yes Y | cf delete-service getgov-$DESTINATION_ENVIRONMENT-database + #create it again + cf create-service aws-rds micro-psql getgov-$DESTINATION_ENVIRONMENT-database + # wait for it be created (up to 5 mins) + # this checks the creation cf service getgov-$DESTINATION_ENVIRONMENT-database + # the below command with check “status” line using cf service command mentioned above. if it says “create in progress” it will keep waiting otherwise the next steps fail + + until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' + do + echo "Database not up yet, waiting..." + sleep 30 + done + + #rebind the service + cf bind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database + #restage the app or it will not connect to the database right for the next commands + cf restage getgov-$DESTINATION_ENVIRONMENT + #wait for the above command to finish + #if it is taking way to long and the annoying “instance starting” line that keeps repeating, then run following two commands in a separate window. This will interrupt the death loop where it keeps hitting an error with it failing health checks + #create the cache table and run migrations + cf run-task getgov-$DESTINATION_ENVIRONMENT --command 'python manage.py createcachetable' --name createcachetable + cf run-task getgov-$DESTINATION_ENVIRONMENT --command 'python manage.py migrate' --name migrate + + #check that your cloud.gov logs show this is done before you run the following command (or be like me and you have to run the command again because you were impatient. Running this before the migrate finishes will cause an error) + #load fixtures + cf run-task getgov-$DESTINATION_ENVIRONMENT --command 'python manage.py load' --name loaddata diff --git a/docs/developer/workflows/README.md b/docs/developer/workflows/README.md new file mode 100644 index 000000000..ea06a8a31 --- /dev/null +++ b/docs/developer/workflows/README.md @@ -0,0 +1,7 @@ +# Workflow + +======================== + +This directory contains files related to workflows + +Delete And Recreate Database is in [docs/ops](../workflows/delete-and-recreate-db.md/). \ No newline at end of file diff --git a/docs/developer/workflows/delete-and-recreate-db.md b/docs/developer/workflows/delete-and-recreate-db.md new file mode 100644 index 000000000..667ea6fe4 --- /dev/null +++ b/docs/developer/workflows/delete-and-recreate-db.md @@ -0,0 +1,13 @@ +## Delete And Recreate Database + +This script destroys recreates a database. This is another troubleshooting tool for issues with the database. It + +1. unbinds the database +2. deletes it +3. recreates it +4. binds it back to the sandbox +5. runs migrations + +Addition Info in this slack thread: + +[Slack thread](https://cisa-corp.slack.com/archives/C05BGB4L5NF/p1725495150772119) From 9b4ae79170f6098384b352ab354de83e9705580c Mon Sep 17 00:00:00 2001 From: asaki222 Date: Mon, 3 Feb 2025 16:04:18 -0500 Subject: [PATCH 17/37] fixed text --- docs/developer/workflows/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/workflows/README.md b/docs/developer/workflows/README.md index ea06a8a31..6cff81add 100644 --- a/docs/developer/workflows/README.md +++ b/docs/developer/workflows/README.md @@ -1,4 +1,4 @@ -# Workflow +# Workflows Docs ======================== From ffa564b21ec865e0f90d1a855c359184a4f17ed5 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Mon, 3 Feb 2025 13:38:45 -0800 Subject: [PATCH 18/37] Add requested whitespace --- .../templates/emails/domain_manager_deleted_notification.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/templates/emails/domain_manager_deleted_notification.txt b/src/registrar/templates/emails/domain_manager_deleted_notification.txt index af0b92a8c..fbb1e47cc 100644 --- a/src/registrar/templates/emails/domain_manager_deleted_notification.txt +++ b/src/registrar/templates/emails/domain_manager_deleted_notification.txt @@ -2,6 +2,7 @@ Hi,{% if domain_manager and domain_manager.first_name %} {{ domain_manager.first_name }}.{% endif %} A domain manager was removed from {{ domain.name }}. + REMOVED BY: {{ removed_by.email }} REMOVED ON: {{ date }} MANAGER REMOVED: {{ manager_removed.email }} From 4736165d448dc8ed2bd8a3918e462fc462f9f85d Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Mon, 3 Feb 2025 14:48:06 -0800 Subject: [PATCH 19/37] Add tests for custome delete page content --- .../tests/test_admin_domain_invitation.py | 96 +++++++++++++++++++ .../tests/test_admin_userdomainrole.py | 91 ++++++++++++++++++ src/registrar/views/domain.py | 2 +- 3 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 src/registrar/tests/test_admin_domain_invitation.py create mode 100644 src/registrar/tests/test_admin_userdomainrole.py diff --git a/src/registrar/tests/test_admin_domain_invitation.py b/src/registrar/tests/test_admin_domain_invitation.py new file mode 100644 index 000000000..03215b9cd --- /dev/null +++ b/src/registrar/tests/test_admin_domain_invitation.py @@ -0,0 +1,96 @@ +from django.contrib.admin.sites import AdminSite +from django.test import RequestFactory, Client +from api.tests.common import less_console_noise_decorator +from django.urls import reverse +from django_webtest import WebTest # type: ignore +from registrar.admin import ( + DomainAdmin, +) +from registrar.models import ( + Domain, + DomainInvitation, + User, + Host, + DomainInformation, +) +from .common import create_superuser, MockEppLib, GenericTestHelper + + +class TestDomainInvitationAdminAsStaff(MockEppLib, WebTest): + """Test DomainInvitationAdmin class as staff user. + + Notes: + all tests share staffuser; do not change staffuser model in tests + tests have available staffuser, client, and admin + """ + + # csrf checks do not work with WebTest. + # We disable them here. TODO for another ticket. + csrf_checks = False + + @classmethod + def setUpClass(self): + super().setUpClass() + self.site = AdminSite() + self.admin = DomainAdmin(model=DomainInvitation, admin_site=self.site) + self.factory = RequestFactory() + self.superuser = create_superuser() + + def setUp(self): + self.client = Client(HTTP_HOST="localhost:8080") + self.client.force_login(self.superuser) + super().setUp() + self.app.set_user(self.superuser.username) + + def tearDown(self): + super().tearDown() + DomainInformation.objects.all().delete() + Host.objects.all().delete() + DomainInvitation.objects.all().delete() + + @classmethod + def tearDownClass(self): + User.objects.all().delete() + super().tearDownClass() + + @less_console_noise_decorator + def test_custom_delete_confirmation_page(self): + """Tests if custom alerts display on Domain Invitation delete page""" + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_invitation, _ = DomainInvitation.objects.get_or_create(domain=domain) + + domain_invitation_change_page = self.app.get( + reverse("admin:registrar_domaininvitation_change", args=[domain_invitation.pk]) + ) + + self.assertContains(domain_invitation_change_page, "domain-invitation-test.gov") + # click the "Delete" link + confirmation_page = domain_invitation_change_page.click("Delete", index=0) + + custom_alert_content = "If you cancel the domain invitation here" + self.assertContains(confirmation_page, custom_alert_content) + + @less_console_noise_decorator + def test_custom_selected_delete_confirmation_page(self): + """Tests if custom alerts display on Domain Invitation selected delete page from Domain Invitation table""" + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_invitation, _ = DomainInvitation.objects.get_or_create(domain=domain) + + # Get the index. The post expects the index to be encoded as a string + index = f"{domain_invitation.id}" + + test_helper = GenericTestHelper( + factory=self.factory, + user=self.superuser, + admin=self.admin, + url=reverse("admin:registrar_domaininvitation_changelist"), + model=Domain, + client=self.client, + ) + + # Simulate selecting a single record, then clicking "Delete selected domains" + response = test_helper.get_table_delete_confirmation_page("0", index) + + # Check for custom alert message + custom_alert_content = "If you cancel the domain invitation here" + self.assertContains(response, custom_alert_content) diff --git a/src/registrar/tests/test_admin_userdomainrole.py b/src/registrar/tests/test_admin_userdomainrole.py new file mode 100644 index 000000000..19c58f9b2 --- /dev/null +++ b/src/registrar/tests/test_admin_userdomainrole.py @@ -0,0 +1,91 @@ +from django.contrib.admin.sites import AdminSite +from django.test import RequestFactory, Client +from api.tests.common import less_console_noise_decorator +from django.urls import reverse +from django_webtest import WebTest # type: ignore +from registrar.admin import ( + DomainAdmin, +) +from registrar.models import Domain, DomainInvitation, User, Host, DomainInformation, UserDomainRole +from .common import create_superuser, MockEppLib, GenericTestHelper + + +class TestUserDomainRoleAsStaff(MockEppLib, WebTest): + """Test UserDomainRole class as staff user. + + Notes: + all tests share staffuser; do not change staffuser model in tests + tests have available staffuser, client, and admin + """ + + # csrf checks do not work with WebTest. + # We disable them here. TODO for another ticket. + csrf_checks = False + + @classmethod + def setUpClass(self): + super().setUpClass() + self.site = AdminSite() + self.admin = DomainAdmin(model=DomainInvitation, admin_site=self.site) + self.factory = RequestFactory() + self.superuser = create_superuser() + + def setUp(self): + self.client = Client(HTTP_HOST="localhost:8080") + self.client.force_login(self.superuser) + super().setUp() + self.app.set_user(self.superuser.username) + + def tearDown(self): + super().tearDown() + UserDomainRole.objects.all().delete() + DomainInformation.objects.all().delete() + Host.objects.all().delete() + DomainInvitation.objects.all().delete() + + @classmethod + def tearDownClass(self): + User.objects.all().delete() + super().tearDownClass() + + @less_console_noise_decorator + def test_custom_delete_confirmation_page(self): + """Tests if custom alerts display on User Domain Role delete page""" + domain, _ = Domain.objects.get_or_create(name="user-domain-role-test.gov", state=Domain.State.READY) + domain_role, _ = UserDomainRole.objects.get_or_create(domain=domain, user=self.superuser) + + domain_invitation_change_page = self.app.get( + reverse("admin:registrar_userdomainrole_change", args=[domain_role.pk]) + ) + + self.assertContains(domain_invitation_change_page, "user-domain-role-test.gov") + # click the "Delete" link + confirmation_page = domain_invitation_change_page.click("Delete", index=0) + + custom_alert_content = "If you remove someone from a domain here" + self.assertContains(confirmation_page, custom_alert_content) + + @less_console_noise_decorator + def test_custom_selected_delete_confirmation_page(self): + """Tests if custom alerts display on selected delete page from User Domain Roles table""" + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_role, _ = UserDomainRole.objects.get_or_create(domain=domain, user=self.superuser) + + # Get the index. The post expects the index to be encoded as a string + index = f"{domain_role.id}" + + test_helper = GenericTestHelper( + factory=self.factory, + user=self.superuser, + admin=self.admin, + url=reverse("admin:registrar_userdomainrole_changelist"), + model=Domain, + client=self.client, + ) + + # Simulate selecting a single record, then clicking "Delete selected domains" + response = test_helper.get_table_delete_confirmation_page("0", index) + + # Check for custom alert message + custom_alert_content = "If you remove someone from a domain here" + self.assertContains(response, custom_alert_content) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 43931a31d..bcdddb1ee 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -1353,7 +1353,7 @@ class DomainDeleteUserView(UserDomainRolePermissionDeleteView): ) except EmailSendingError: logger.warning( - "Could not sent notification email to %s for domain %s", + "Could not send notification email to %s for domain %s", email, domain.name, exc_info=True, From cdbffa395c74325cc9f43a2e568bd37d51d8ca54 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 4 Feb 2025 07:20:33 -0500 Subject: [PATCH 20/37] canceled and retrieved domain invitations not displayed in domain overview --- src/registrar/models/domain.py | 5 +++++ src/registrar/templates/includes/summary_item.html | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 0f0b3f112..4154c5575 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -9,6 +9,7 @@ from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignor from django.db import models, IntegrityError from django.utils import timezone from typing import Any +from registrar.models.domain_invitation import DomainInvitation from registrar.models.host import Host from registrar.models.host_ip import HostIP from registrar.utility.enums import DefaultEmail @@ -1176,6 +1177,10 @@ class Domain(TimeStampedModel, DomainHelper): elif self.state == self.State.UNKNOWN or self.state == self.State.DNS_NEEDED: return "DNS needed" return self.state.capitalize() + + def active_invitations(self): + """Returns only the active invitations (those with status 'invited').""" + return self.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED) def map_epp_contact_to_public_contact(self, contact: eppInfo.InfoContactResultData, contact_id, contact_type): """Maps the Epp contact representation to a PublicContact object. diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 26e56fea7..d062a7b4e 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -113,10 +113,10 @@ {% endif %} {% endif %} - {% if value.invitations.all %} + {% if value.active_invitations.all %}

Invited domain managers

    - {% for item in value.invitations.all %} + {% for item in value.active_invitations.all %}
  • {{ item.email }}
    • {% endfor %}
From 394fc265d7294ef8eb472d7a5cdad5391820c5d9 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Tue, 4 Feb 2025 09:45:53 -0500 Subject: [PATCH 21/37] updated script --- .github/workflows/delete-and-recreate-db.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index 96a698392..e52397d32 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -84,8 +84,7 @@ jobs: #if it is taking way to long and the annoying “instance starting” line that keeps repeating, then run following two commands in a separate window. This will interrupt the death loop where it keeps hitting an error with it failing health checks #create the cache table and run migrations cf run-task getgov-$DESTINATION_ENVIRONMENT --command 'python manage.py createcachetable' --name createcachetable - cf run-task getgov-$DESTINATION_ENVIRONMENT --command 'python manage.py migrate' --name migrate + cf run-task getgov-$DESTINATION_ENVIRONMENT --wait --command 'python manage.py migrate' --name migrate - #check that your cloud.gov logs show this is done before you run the following command (or be like me and you have to run the command again because you were impatient. Running this before the migrate finishes will cause an error) #load fixtures - cf run-task getgov-$DESTINATION_ENVIRONMENT --command 'python manage.py load' --name loaddata + cf run-task getgov-$DESTINATION_ENVIRONMENT --wait --command 'python manage.py load' --name loaddata From 62e7b0914425960787885f9205ec036eab01e7c9 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Tue, 4 Feb 2025 09:49:42 -0500 Subject: [PATCH 22/37] fixed typos --- docs/developer/workflows/delete-and-recreate-db.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/developer/workflows/delete-and-recreate-db.md b/docs/developer/workflows/delete-and-recreate-db.md index 667ea6fe4..98a2d445a 100644 --- a/docs/developer/workflows/delete-and-recreate-db.md +++ b/docs/developer/workflows/delete-and-recreate-db.md @@ -1,6 +1,6 @@ ## Delete And Recreate Database -This script destroys recreates a database. This is another troubleshooting tool for issues with the database. It +This script destroys and recreates a database. This is another troubleshooting tool for issues with the database. 1. unbinds the database 2. deletes it @@ -11,3 +11,4 @@ This script destroys recreates a database. This is another troubleshooting tool Addition Info in this slack thread: [Slack thread](https://cisa-corp.slack.com/archives/C05BGB4L5NF/p1725495150772119) +[Script](../../../../manage.get.gov/.github/workflows/delete-and-recreate-db.yaml) From 5bd8ec6761ea011c1b98b63ed8e115fc4cc5245f Mon Sep 17 00:00:00 2001 From: asaki222 Date: Tue, 4 Feb 2025 10:46:23 -0500 Subject: [PATCH 23/37] changed the file path --- docs/developer/workflows/delete-and-recreate-db.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/developer/workflows/delete-and-recreate-db.md b/docs/developer/workflows/delete-and-recreate-db.md index 98a2d445a..970449aa6 100644 --- a/docs/developer/workflows/delete-and-recreate-db.md +++ b/docs/developer/workflows/delete-and-recreate-db.md @@ -10,5 +10,5 @@ This script destroys and recreates a database. This is another troubleshooting t Addition Info in this slack thread: -[Slack thread](https://cisa-corp.slack.com/archives/C05BGB4L5NF/p1725495150772119) -[Script](../../../../manage.get.gov/.github/workflows/delete-and-recreate-db.yaml) +- [Slack thread](https://cisa-corp.slack.com/archives/C05BGB4L5NF/p1725495150772119) +- [Script](.github/workflows/delete-and-recreate-db.yaml) From 1a3e83b507d0e5f1bcb8779950fb158539d6d61d Mon Sep 17 00:00:00 2001 From: asaki222 Date: Tue, 4 Feb 2025 10:49:12 -0500 Subject: [PATCH 24/37] removed script link --- docs/developer/workflows/delete-and-recreate-db.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/developer/workflows/delete-and-recreate-db.md b/docs/developer/workflows/delete-and-recreate-db.md index 970449aa6..7b378ce47 100644 --- a/docs/developer/workflows/delete-and-recreate-db.md +++ b/docs/developer/workflows/delete-and-recreate-db.md @@ -11,4 +11,3 @@ This script destroys and recreates a database. This is another troubleshooting t Addition Info in this slack thread: - [Slack thread](https://cisa-corp.slack.com/archives/C05BGB4L5NF/p1725495150772119) -- [Script](.github/workflows/delete-and-recreate-db.yaml) From b1f2dddd99a0f2592d4f75536d92888447d44c96 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 4 Feb 2025 12:16:28 -0500 Subject: [PATCH 25/37] fixed portfolio members table, assigned domains --- src/registrar/models/domain.py | 4 ++-- src/registrar/views/portfolio_members_json.py | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 4154c5575..649b3f93d 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1177,10 +1177,10 @@ class Domain(TimeStampedModel, DomainHelper): elif self.state == self.State.UNKNOWN or self.state == self.State.DNS_NEEDED: return "DNS needed" return self.state.capitalize() - + def active_invitations(self): """Returns only the active invitations (those with status 'invited').""" - return self.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED) + return self.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED) def map_epp_contact_to_public_contact(self, contact: eppInfo.InfoContactResultData, contact_id, contact_type): """Maps the Epp contact representation to a PublicContact object. diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index a45ad66e9..29dc6a71c 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -123,7 +123,11 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): # Subquery to get concatenated domain information for each email domain_invitations = ( - DomainInvitation.objects.filter(email=OuterRef("email"), domain__domain_info__portfolio=portfolio) + DomainInvitation.objects.filter( + email=OuterRef("email"), + domain__domain_info__portfolio=portfolio, + status=DomainInvitation.DomainInvitationStatus.INVITED, + ) .annotate( concatenated_info=Concat(F("domain__id"), Value(":"), F("domain__name"), output_field=CharField()) ) From 3f7d1b0524824033d6535672f225f3239989f5cb Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 4 Feb 2025 12:24:46 -0500 Subject: [PATCH 26/37] unblock invitation when previously retrieved invitation --- src/registrar/models/utility/portfolio_helper.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 8c42b80c7..7c82413ae 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -1,5 +1,6 @@ from registrar.utility import StrEnum from django.db import models +from django.db.models import Q from django.apps import apps from django.forms import ValidationError from registrar.utility.waffle import flag_is_active_for_user @@ -136,9 +137,10 @@ def validate_user_portfolio_permission(user_portfolio_permission): "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) - existing_invitations = PortfolioInvitation.objects.exclude( - portfolio=user_portfolio_permission.portfolio - ).filter(email=user_portfolio_permission.user.email) + existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email).exclude( + Q(portfolio=user_portfolio_permission.portfolio) + | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) + ) if existing_invitations.exists(): raise ValidationError( "This user is already assigned to a portfolio invitation. " From 8723e35262c9bf0868557f997e185fb4bde0b7be Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 4 Feb 2025 18:35:45 -0500 Subject: [PATCH 27/37] unit test added --- .../models/utility/portfolio_helper.py | 4 +- .../tests/test_views_members_json.py | 16 +++ src/registrar/tests/test_views_portfolio.py | 119 +++++++++++++++--- 3 files changed, 120 insertions(+), 19 deletions(-) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 7c82413ae..0864bded0 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -197,8 +197,8 @@ def validate_portfolio_invitation(portfolio_invitation): if not flag_is_active_for_user(user, "multiple_portfolios"): existing_permissions = UserPortfolioPermission.objects.filter(user=user) - existing_invitations = PortfolioInvitation.objects.exclude(id=portfolio_invitation.id).filter( - email=portfolio_invitation.email + existing_invitations = PortfolioInvitation.objects.filter(email=portfolio_invitation.email).exclude( + Q(id=portfolio_invitation.id) | Q(status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED) ) if existing_permissions.exists(): diff --git a/src/registrar/tests/test_views_members_json.py b/src/registrar/tests/test_views_members_json.py index ceae1e35f..c505421ec 100644 --- a/src/registrar/tests/test_views_members_json.py +++ b/src/registrar/tests/test_views_members_json.py @@ -372,6 +372,21 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): domain=domain3, ) + # create another domain in the portfolio + # but make sure the domain invitation is canceled + domain4 = Domain.objects.create( + name="somedomain4.com", + ) + DomainInformation.objects.create( + creator=self.user, + domain=domain4, + ) + DomainInvitation.objects.create( + email=self.email6, + domain=domain4, + status=DomainInvitation.DomainInvitationStatus.CANCELED, + ) + response = self.app.get(reverse("get_portfolio_members_json"), params={"portfolio": self.portfolio.id}) self.assertEqual(response.status_code, 200) data = response.json @@ -381,6 +396,7 @@ class GetPortfolioMembersJsonTest(MockEppLib, WebTest): self.assertIn("somedomain1.com", domain_names) self.assertIn("thissecondinvitetestsasubqueryinjson@lets.notbreak", domain_names) self.assertNotIn("somedomain3.com", domain_names) + self.assertNotIn("somedomain4.com", domain_names) @less_console_noise_decorator @override_flag("organization_feature", active=True) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 33f334f7f..a324fc822 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -22,7 +22,7 @@ from registrar.models.utility.portfolio_helper import UserPortfolioPermissionCho from registrar.tests.test_views import TestWithUser from registrar.utility.email import EmailSendingError from registrar.utility.errors import MissingEmailError -from .common import MockSESClient, completed_domain_request, create_test_user, create_user +from .common import MockEppLib, MockSESClient, completed_domain_request, create_test_user, create_user from waffle.testutils import override_flag from django.contrib.sessions.middleware import SessionMiddleware import boto3_mocking # type: ignore @@ -3049,34 +3049,35 @@ class TestRequestingEntity(WebTest): self.assertContains(response, "kepler, AL") -class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): +class TestPortfolioInviteNewMemberView(MockEppLib, WebTest): - @classmethod - def setUpClass(cls): - super().setUpClass() + def setUp(self): + super().setUp() + + self.user = create_test_user() # Create Portfolio - cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Test Portfolio") + self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Test Portfolio") # Add an invited member who has been invited to manage domains - cls.invited_member_email = "invited@example.com" - cls.invitation = PortfolioInvitation.objects.create( - email=cls.invited_member_email, - portfolio=cls.portfolio, + self.invited_member_email = "invited@example.com" + self.invitation = PortfolioInvitation.objects.create( + email=self.invited_member_email, + portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], additional_permissions=[ UserPortfolioPermissionChoices.VIEW_MEMBERS, ], ) - cls.new_member_email = "newmember@example.com" + self.new_member_email = "newmember@example.com" - AllowedEmail.objects.get_or_create(email=cls.new_member_email) + AllowedEmail.objects.get_or_create(email=self.new_member_email) # Assign permissions to the user making requests UserPortfolioPermission.objects.create( - user=cls.user, - portfolio=cls.portfolio, + user=self.user, + portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], additional_permissions=[ UserPortfolioPermissionChoices.VIEW_MEMBERS, @@ -3084,14 +3085,13 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): ], ) - @classmethod - def tearDownClass(cls): + def tearDown(self): PortfolioInvitation.objects.all().delete() UserPortfolioPermission.objects.all().delete() Portfolio.objects.all().delete() User.objects.all().delete() AllowedEmail.objects.all().delete() - super().tearDownClass() + super().tearDown() @boto3_mocking.patching @less_console_noise_decorator @@ -3136,6 +3136,91 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): # Check that an email was sent self.assertTrue(mock_client.send_email.called) + @boto3_mocking.patching + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + @patch("registrar.views.portfolios.send_portfolio_invitation_email") + def test_member_invite_for_previously_removed_user(self, mock_send_email): + """Tests the member invitation flow for an existing member which was previously removed.""" + self.client.force_login(self.user) + + # invite, then retrieve an existing user, then remove the user from the portfolio + retrieved_member_email = "retrieved@example.com" + retrieved_user = User.objects.create( + username="retrieved_user", + first_name="Retrieved", + last_name="User", + email=retrieved_member_email, + phone="8003111234", + title="retrieved", + ) + + retrieved_invitation = PortfolioInvitation.objects.create( + email=retrieved_member_email, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_MEMBERS, + ], + status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, + ) + retrieved_invitation.retrieve() + retrieved_invitation.save() + upp = UserPortfolioPermission.objects.filter( + user=retrieved_user, + portfolio=self.portfolio, + ) + upp.delete() + + # Simulate a session to ensure continuity + session_id = self.client.session.session_key + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + mock_client_class = MagicMock() + mock_client = mock_client_class.return_value + + with boto3_mocking.clients.handler_for("sesv2", mock_client_class): + # Simulate submission of member invite for previously retrieved/removed member + final_response = self.client.post( + reverse("new-member"), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permissions": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "domain_permissions": UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS.value, + "member_permissions": "no_access", + "email": retrieved_member_email, + }, + ) + + # Ensure the final submission is successful + self.assertEqual(final_response.status_code, 302) # Redirects + + # Validate Database Changes + # Validate that portfolio invitation was created and retrieved + self.assertFalse( + PortfolioInvitation.objects.filter( + email=retrieved_member_email, + portfolio=self.portfolio, + status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, + ).exists() + ) + # at least one retrieved invitation + self.assertTrue( + PortfolioInvitation.objects.filter( + email=retrieved_member_email, + portfolio=self.portfolio, + status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED, + ).exists() + ) + # Ensure exactly one UserPortfolioPermission exists for the retrieved user + self.assertEqual( + UserPortfolioPermission.objects.filter(user=retrieved_user, portfolio=self.portfolio).count(), + 1, + "Expected exactly one UserPortfolioPermission for the retrieved user." + ) + + @boto3_mocking.patching @less_console_noise_decorator @override_flag("organization_feature", active=True) From 5f545daed876bbbba1dfbe5f065c6c7e8857ab3d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 4 Feb 2025 19:42:23 -0500 Subject: [PATCH 28/37] lint --- src/registrar/tests/test_views_portfolio.py | 78 ++++++++++----------- 1 file changed, 36 insertions(+), 42 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index a274e777b..8d06e35da 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -3148,7 +3148,6 @@ class TestPortfolioInviteNewMemberView(MockEppLib, WebTest): # Check that an email was sent self.assertTrue(mock_client.send_email.called) - @boto3_mocking.patching @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @@ -3189,49 +3188,44 @@ class TestPortfolioInviteNewMemberView(MockEppLib, WebTest): session_id = self.client.session.session_key self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - mock_client_class = MagicMock() - mock_client = mock_client_class.return_value + # Simulate submission of member invite for previously retrieved/removed member + final_response = self.client.post( + reverse("new-member"), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permissions": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "domain_permissions": UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS.value, + "member_permissions": "no_access", + "email": retrieved_member_email, + }, + ) - with boto3_mocking.clients.handler_for("sesv2", mock_client_class): - # Simulate submission of member invite for previously retrieved/removed member - final_response = self.client.post( - reverse("new-member"), - { - "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, - "domain_request_permissions": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, - "domain_permissions": UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS.value, - "member_permissions": "no_access", - "email": retrieved_member_email, - }, - ) - - # Ensure the final submission is successful - self.assertEqual(final_response.status_code, 302) # Redirects - - # Validate Database Changes - # Validate that portfolio invitation was created and retrieved - self.assertFalse( - PortfolioInvitation.objects.filter( - email=retrieved_member_email, - portfolio=self.portfolio, - status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, - ).exists() - ) - # at least one retrieved invitation - self.assertTrue( - PortfolioInvitation.objects.filter( - email=retrieved_member_email, - portfolio=self.portfolio, - status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED, - ).exists() - ) - # Ensure exactly one UserPortfolioPermission exists for the retrieved user - self.assertEqual( - UserPortfolioPermission.objects.filter(user=retrieved_user, portfolio=self.portfolio).count(), - 1, - "Expected exactly one UserPortfolioPermission for the retrieved user." - ) + # Ensure the final submission is successful + self.assertEqual(final_response.status_code, 302) # Redirects + # Validate Database Changes + # Validate that portfolio invitation was created and retrieved + self.assertFalse( + PortfolioInvitation.objects.filter( + email=retrieved_member_email, + portfolio=self.portfolio, + status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, + ).exists() + ) + # at least one retrieved invitation + self.assertTrue( + PortfolioInvitation.objects.filter( + email=retrieved_member_email, + portfolio=self.portfolio, + status=PortfolioInvitation.PortfolioInvitationStatus.RETRIEVED, + ).exists() + ) + # Ensure exactly one UserPortfolioPermission exists for the retrieved user + self.assertEqual( + UserPortfolioPermission.objects.filter(user=retrieved_user, portfolio=self.portfolio).count(), + 1, + "Expected exactly one UserPortfolioPermission for the retrieved user.", + ) @boto3_mocking.patching @less_console_noise_decorator From 1bba542a557290056166b424e7df4bd66fca0808 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Thu, 6 Feb 2025 12:36:41 -0500 Subject: [PATCH 29/37] changes --- .github/workflows/delete-and-recreate-db.yaml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index e52397d32..caf6a1996 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -41,7 +41,7 @@ jobs: CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD DESTINATION_ENVIRONMENT: ${{ github.event.inputs.environment}} steps: - - name: Dekete and Recreate Database + - name: Delete and Recreate Database env: cf_username: ${{ secrets[env.CF_USERNAME] }} cf_password: ${{ secrets[env.CF_PASSWORD] }} @@ -58,33 +58,33 @@ jobs: - #unbind the service + # unbind the service cf unbind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database #delete the service key yes Y | cf delete-service-key getgov-$DESTINATION_ENVIRONMENT-database SERVICE_CONNECT #delete the service yes Y | cf delete-service getgov-$DESTINATION_ENVIRONMENT-database - #create it again + # create it again cf create-service aws-rds micro-psql getgov-$DESTINATION_ENVIRONMENT-database # wait for it be created (up to 5 mins) # this checks the creation cf service getgov-$DESTINATION_ENVIRONMENT-database # the below command with check “status” line using cf service command mentioned above. if it says “create in progress” it will keep waiting otherwise the next steps fail - until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' + timeout 30 until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' do echo "Database not up yet, waiting..." sleep 30 done - #rebind the service + # rebind the service cf bind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database #restage the app or it will not connect to the database right for the next commands cf restage getgov-$DESTINATION_ENVIRONMENT - #wait for the above command to finish - #if it is taking way to long and the annoying “instance starting” line that keeps repeating, then run following two commands in a separate window. This will interrupt the death loop where it keeps hitting an error with it failing health checks - #create the cache table and run migrations + # wait for the above command to finish + # if it is taking way to long and the annoying “instance starting” line that keeps repeating, then run following two commands in a separate window. This will interrupt the death loop where it keeps hitting an error with it failing health checks + # create the cache table and run migrations cf run-task getgov-$DESTINATION_ENVIRONMENT --command 'python manage.py createcachetable' --name createcachetable cf run-task getgov-$DESTINATION_ENVIRONMENT --wait --command 'python manage.py migrate' --name migrate - #load fixtures + # load fixtures cf run-task getgov-$DESTINATION_ENVIRONMENT --wait --command 'python manage.py load' --name loaddata From a2151f4b0375ae2e745fe59e2465343dc741142e Mon Sep 17 00:00:00 2001 From: asaki222 Date: Thu, 6 Feb 2025 12:43:13 -0500 Subject: [PATCH 30/37] updates --- .github/workflows/delete-and-recreate-db.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index caf6a1996..a24690f8e 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -70,11 +70,11 @@ jobs: # this checks the creation cf service getgov-$DESTINATION_ENVIRONMENT-database # the below command with check “status” line using cf service command mentioned above. if it says “create in progress” it will keep waiting otherwise the next steps fail - timeout 30 until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' + timeout 10 bash -c 'until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' do echo "Database not up yet, waiting..." sleep 30 - done + done' # rebind the service cf bind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database From 75ae3d3114fd8606e9f2d14999c6654556db65ab Mon Sep 17 00:00:00 2001 From: asaki222 Date: Thu, 6 Feb 2025 12:46:27 -0500 Subject: [PATCH 31/37] trying again --- .github/workflows/delete-and-recreate-db.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index a24690f8e..6a0b7be94 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -70,11 +70,11 @@ jobs: # this checks the creation cf service getgov-$DESTINATION_ENVIRONMENT-database # the below command with check “status” line using cf service command mentioned above. if it says “create in progress” it will keep waiting otherwise the next steps fail - timeout 10 bash -c 'until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' + timeout 10 bash -c "until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' do - echo "Database not up yet, waiting..." + echo 'Database not up yet, waiting...' sleep 30 - done' + done" # rebind the service cf bind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database From 640805833b0073b4fcd52c7fe9a5a0e4aeaaa69f Mon Sep 17 00:00:00 2001 From: asaki222 Date: Thu, 6 Feb 2025 13:50:09 -0500 Subject: [PATCH 32/37] increased timeout --- .github/workflows/delete-and-recreate-db.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index 6a0b7be94..7d19dad09 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -70,7 +70,7 @@ jobs: # this checks the creation cf service getgov-$DESTINATION_ENVIRONMENT-database # the below command with check “status” line using cf service command mentioned above. if it says “create in progress” it will keep waiting otherwise the next steps fail - timeout 10 bash -c "until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' + timeout 480 bash -c "until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' do echo 'Database not up yet, waiting...' sleep 30 From 8c35346a58c18f88826b8dae6734f0e43812db5b Mon Sep 17 00:00:00 2001 From: asaki222 Date: Thu, 6 Feb 2025 13:51:19 -0500 Subject: [PATCH 33/37] increased timeout and fixed spacing --- .github/workflows/delete-and-recreate-db.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index 7d19dad09..a61ee626e 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -70,11 +70,11 @@ jobs: # this checks the creation cf service getgov-$DESTINATION_ENVIRONMENT-database # the below command with check “status” line using cf service command mentioned above. if it says “create in progress” it will keep waiting otherwise the next steps fail - timeout 480 bash -c "until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' - do - echo 'Database not up yet, waiting...' - sleep 30 - done" + timeout 480 bash -c "until cf service getgov-$DESTINATION_ENVIRONMENT-database | grep -q 'The service instance status is succeeded' + do + echo 'Database not up yet, waiting...' + sleep 30 + done" # rebind the service cf bind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database From 5247bc3c8701efa5136dc515a15e2624e40c2ca6 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Thu, 6 Feb 2025 14:33:58 -0500 Subject: [PATCH 34/37] missed a comment space --- .github/workflows/delete-and-recreate-db.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index a61ee626e..ecdf54bbc 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -62,7 +62,7 @@ jobs: cf unbind-service getgov-$DESTINATION_ENVIRONMENT getgov-$DESTINATION_ENVIRONMENT-database #delete the service key yes Y | cf delete-service-key getgov-$DESTINATION_ENVIRONMENT-database SERVICE_CONNECT - #delete the service + # delete the service yes Y | cf delete-service getgov-$DESTINATION_ENVIRONMENT-database # create it again cf create-service aws-rds micro-psql getgov-$DESTINATION_ENVIRONMENT-database From ba9deb7a9353d71edea884add37ff92076ecbf08 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 6 Feb 2025 12:00:08 -0800 Subject: [PATCH 35/37] Move admin tests to admin.py --- src/registrar/tests/test_admin.py | 108 +++++++++++++++++- .../tests/test_admin_domain_invitation.py | 96 ---------------- .../tests/test_admin_userdomainrole.py | 91 --------------- 3 files changed, 104 insertions(+), 191 deletions(-) delete mode 100644 src/registrar/tests/test_admin_domain_invitation.py delete mode 100644 src/registrar/tests/test_admin_userdomainrole.py diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 387319663..9c840fb99 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -54,6 +54,7 @@ from registrar.models import ( from .common import ( MockDbForSharedTests, AuditedAdminMockData, + MockEppLib, completed_domain_request, generic_domain_object, less_console_noise, @@ -120,23 +121,35 @@ class TestFsmModelResource(TestCase): fsm_field_mock.save.assert_not_called() -class TestDomainInvitationAdmin(TestCase): +class TestDomainInvitationAdmin(MockEppLib, WebTest): """Tests for the DomainInvitationAdmin class as super user Notes: all tests share superuser; do not change this model in tests tests have available superuser, client, and admin """ + # csrf checks do not work with WebTest. + # We disable them here. TODO for another ticket. + csrf_checks = False + + @classmethod + def setUpClass(self): + super().setUpClass() + self.site = AdminSite() + self.factory = RequestFactory() + self.superuser = create_superuser() def setUp(self): - self.factory = RequestFactory() + super().setUp() self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite()) - self.superuser = create_superuser() self.domain = Domain.objects.create(name="example.com") self.portfolio = Portfolio.objects.create(organization_name="new portfolio", creator=self.superuser) DomainInformation.objects.create(domain=self.domain, portfolio=self.portfolio, creator=self.superuser) """Create a client object""" self.client = Client(HTTP_HOST="localhost:8080") + self.client.force_login(self.superuser) + self.app.set_user(self.superuser.username) + def tearDown(self): """Delete all DomainInvitation objects""" @@ -1065,6 +1078,49 @@ class TestDomainInvitationAdmin(TestCase): self.assertEqual(DomainInvitation.objects.count(), 0) self.assertEqual(PortfolioInvitation.objects.count(), 1) + @less_console_noise_decorator + def test_custom_delete_confirmation_page(self): + """Tests if custom alerts display on Domain Invitation delete page""" + self.client.force_login(self.superuser) + self.app.set_user(self.superuser.username) + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_invitation, _ = DomainInvitation.objects.get_or_create(domain=domain) + + domain_invitation_change_page = self.app.get( + reverse("admin:registrar_domaininvitation_change", args=[domain_invitation.pk]) + ) + + self.assertContains(domain_invitation_change_page, "domain-invitation-test.gov") + # click the "Delete" link + confirmation_page = domain_invitation_change_page.click("Delete", index=0) + + custom_alert_content = "If you cancel the domain invitation here" + self.assertContains(confirmation_page, custom_alert_content) + + @less_console_noise_decorator + def test_custom_selected_delete_confirmation_page(self): + """Tests if custom alerts display on Domain Invitation selected delete page from Domain Invitation table""" + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_invitation, _ = DomainInvitation.objects.get_or_create(domain=domain) + + # Get the index. The post expects the index to be encoded as a string + index = f"{domain_invitation.id}" + + test_helper = GenericTestHelper( + factory=self.factory, + user=self.superuser, + admin=self.admin, + url=reverse("admin:registrar_domaininvitation_changelist"), + model=Domain, + client=self.client, + ) + + # Simulate selecting a single record, then clicking "Delete selected domains" + response = test_helper.get_table_delete_confirmation_page("0", index) + + # Check for custom alert message + custom_alert_content = "If you cancel the domain invitation here" + self.assertContains(response, custom_alert_content) class TestUserPortfolioPermissionAdmin(TestCase): """Tests for the PortfolioInivtationAdmin class""" @@ -1922,7 +1978,7 @@ class TestDomainInformationAdmin(TestCase): self.test_helper.assert_table_sorted("-4", ("-creator__first_name", "-creator__last_name")) -class TestUserDomainRoleAdmin(TestCase): +class TestUserDomainRoleAdmin(MockEppLib, WebTest): """Tests for the UserDomainRoleAdmin class as super user Notes: @@ -1949,6 +2005,8 @@ class TestUserDomainRoleAdmin(TestCase): """Setup environment for a mock admin user""" super().setUp() self.client = Client(HTTP_HOST="localhost:8080") + self.client.force_login(self.superuser) + self.app.set_user(self.superuser.username) def tearDown(self): """Delete all Users, Domains, and UserDomainRoles""" @@ -2111,6 +2169,48 @@ class TestUserDomainRoleAdmin(TestCase): # We only need to check for the end of the HTML string self.assertContains(response, "Joe Jones AntarcticPolarBears@example.com", count=1) + @less_console_noise_decorator + def test_custom_delete_confirmation_page(self): + """Tests if custom alerts display on User Domain Role delete page""" + domain, _ = Domain.objects.get_or_create(name="user-domain-role-test.gov", state=Domain.State.READY) + domain_role, _ = UserDomainRole.objects.get_or_create(domain=domain, user=self.superuser) + + domain_invitation_change_page = self.app.get( + reverse("admin:registrar_userdomainrole_change", args=[domain_role.pk]) + ) + + self.assertContains(domain_invitation_change_page, "user-domain-role-test.gov") + # click the "Delete" link + confirmation_page = domain_invitation_change_page.click("Delete", index=0) + + custom_alert_content = "If you remove someone from a domain here" + self.assertContains(confirmation_page, custom_alert_content) + + @less_console_noise_decorator + def test_custom_selected_delete_confirmation_page(self): + """Tests if custom alerts display on selected delete page from User Domain Roles table""" + domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) + domain_role, _ = UserDomainRole.objects.get_or_create(domain=domain, user=self.superuser) + + # Get the index. The post expects the index to be encoded as a string + index = f"{domain_role.id}" + + test_helper = GenericTestHelper( + factory=self.factory, + user=self.superuser, + admin=self.admin, + url=reverse("admin:registrar_userdomainrole_changelist"), + model=Domain, + client=self.client, + ) + + # Simulate selecting a single record, then clicking "Delete selected domains" + response = test_helper.get_table_delete_confirmation_page("0", index) + + # Check for custom alert message + custom_alert_content = "If you remove someone from a domain here" + self.assertContains(response, custom_alert_content) + class TestListHeaderAdmin(TestCase): """Tests for the ListHeaderAdmin class as super user diff --git a/src/registrar/tests/test_admin_domain_invitation.py b/src/registrar/tests/test_admin_domain_invitation.py deleted file mode 100644 index 03215b9cd..000000000 --- a/src/registrar/tests/test_admin_domain_invitation.py +++ /dev/null @@ -1,96 +0,0 @@ -from django.contrib.admin.sites import AdminSite -from django.test import RequestFactory, Client -from api.tests.common import less_console_noise_decorator -from django.urls import reverse -from django_webtest import WebTest # type: ignore -from registrar.admin import ( - DomainAdmin, -) -from registrar.models import ( - Domain, - DomainInvitation, - User, - Host, - DomainInformation, -) -from .common import create_superuser, MockEppLib, GenericTestHelper - - -class TestDomainInvitationAdminAsStaff(MockEppLib, WebTest): - """Test DomainInvitationAdmin class as staff user. - - Notes: - all tests share staffuser; do not change staffuser model in tests - tests have available staffuser, client, and admin - """ - - # csrf checks do not work with WebTest. - # We disable them here. TODO for another ticket. - csrf_checks = False - - @classmethod - def setUpClass(self): - super().setUpClass() - self.site = AdminSite() - self.admin = DomainAdmin(model=DomainInvitation, admin_site=self.site) - self.factory = RequestFactory() - self.superuser = create_superuser() - - def setUp(self): - self.client = Client(HTTP_HOST="localhost:8080") - self.client.force_login(self.superuser) - super().setUp() - self.app.set_user(self.superuser.username) - - def tearDown(self): - super().tearDown() - DomainInformation.objects.all().delete() - Host.objects.all().delete() - DomainInvitation.objects.all().delete() - - @classmethod - def tearDownClass(self): - User.objects.all().delete() - super().tearDownClass() - - @less_console_noise_decorator - def test_custom_delete_confirmation_page(self): - """Tests if custom alerts display on Domain Invitation delete page""" - domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) - domain_invitation, _ = DomainInvitation.objects.get_or_create(domain=domain) - - domain_invitation_change_page = self.app.get( - reverse("admin:registrar_domaininvitation_change", args=[domain_invitation.pk]) - ) - - self.assertContains(domain_invitation_change_page, "domain-invitation-test.gov") - # click the "Delete" link - confirmation_page = domain_invitation_change_page.click("Delete", index=0) - - custom_alert_content = "If you cancel the domain invitation here" - self.assertContains(confirmation_page, custom_alert_content) - - @less_console_noise_decorator - def test_custom_selected_delete_confirmation_page(self): - """Tests if custom alerts display on Domain Invitation selected delete page from Domain Invitation table""" - domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) - domain_invitation, _ = DomainInvitation.objects.get_or_create(domain=domain) - - # Get the index. The post expects the index to be encoded as a string - index = f"{domain_invitation.id}" - - test_helper = GenericTestHelper( - factory=self.factory, - user=self.superuser, - admin=self.admin, - url=reverse("admin:registrar_domaininvitation_changelist"), - model=Domain, - client=self.client, - ) - - # Simulate selecting a single record, then clicking "Delete selected domains" - response = test_helper.get_table_delete_confirmation_page("0", index) - - # Check for custom alert message - custom_alert_content = "If you cancel the domain invitation here" - self.assertContains(response, custom_alert_content) diff --git a/src/registrar/tests/test_admin_userdomainrole.py b/src/registrar/tests/test_admin_userdomainrole.py deleted file mode 100644 index 19c58f9b2..000000000 --- a/src/registrar/tests/test_admin_userdomainrole.py +++ /dev/null @@ -1,91 +0,0 @@ -from django.contrib.admin.sites import AdminSite -from django.test import RequestFactory, Client -from api.tests.common import less_console_noise_decorator -from django.urls import reverse -from django_webtest import WebTest # type: ignore -from registrar.admin import ( - DomainAdmin, -) -from registrar.models import Domain, DomainInvitation, User, Host, DomainInformation, UserDomainRole -from .common import create_superuser, MockEppLib, GenericTestHelper - - -class TestUserDomainRoleAsStaff(MockEppLib, WebTest): - """Test UserDomainRole class as staff user. - - Notes: - all tests share staffuser; do not change staffuser model in tests - tests have available staffuser, client, and admin - """ - - # csrf checks do not work with WebTest. - # We disable them here. TODO for another ticket. - csrf_checks = False - - @classmethod - def setUpClass(self): - super().setUpClass() - self.site = AdminSite() - self.admin = DomainAdmin(model=DomainInvitation, admin_site=self.site) - self.factory = RequestFactory() - self.superuser = create_superuser() - - def setUp(self): - self.client = Client(HTTP_HOST="localhost:8080") - self.client.force_login(self.superuser) - super().setUp() - self.app.set_user(self.superuser.username) - - def tearDown(self): - super().tearDown() - UserDomainRole.objects.all().delete() - DomainInformation.objects.all().delete() - Host.objects.all().delete() - DomainInvitation.objects.all().delete() - - @classmethod - def tearDownClass(self): - User.objects.all().delete() - super().tearDownClass() - - @less_console_noise_decorator - def test_custom_delete_confirmation_page(self): - """Tests if custom alerts display on User Domain Role delete page""" - domain, _ = Domain.objects.get_or_create(name="user-domain-role-test.gov", state=Domain.State.READY) - domain_role, _ = UserDomainRole.objects.get_or_create(domain=domain, user=self.superuser) - - domain_invitation_change_page = self.app.get( - reverse("admin:registrar_userdomainrole_change", args=[domain_role.pk]) - ) - - self.assertContains(domain_invitation_change_page, "user-domain-role-test.gov") - # click the "Delete" link - confirmation_page = domain_invitation_change_page.click("Delete", index=0) - - custom_alert_content = "If you remove someone from a domain here" - self.assertContains(confirmation_page, custom_alert_content) - - @less_console_noise_decorator - def test_custom_selected_delete_confirmation_page(self): - """Tests if custom alerts display on selected delete page from User Domain Roles table""" - domain, _ = Domain.objects.get_or_create(name="domain-invitation-test.gov", state=Domain.State.READY) - domain_role, _ = UserDomainRole.objects.get_or_create(domain=domain, user=self.superuser) - - # Get the index. The post expects the index to be encoded as a string - index = f"{domain_role.id}" - - test_helper = GenericTestHelper( - factory=self.factory, - user=self.superuser, - admin=self.admin, - url=reverse("admin:registrar_userdomainrole_changelist"), - model=Domain, - client=self.client, - ) - - # Simulate selecting a single record, then clicking "Delete selected domains" - response = test_helper.get_table_delete_confirmation_page("0", index) - - # Check for custom alert message - custom_alert_content = "If you remove someone from a domain here" - self.assertContains(response, custom_alert_content) From bcf2f0946c2f7f45f50ccea440797250b8ab4fd3 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 6 Feb 2025 13:07:30 -0800 Subject: [PATCH 36/37] Fix linting --- src/registrar/tests/test_admin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index adec70aeb..b488ee655 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -128,6 +128,7 @@ class TestDomainInvitationAdmin(MockEppLib, WebTest): all tests share superuser; do not change this model in tests tests have available superuser, client, and admin """ + # csrf checks do not work with WebTest. # We disable them here. TODO for another ticket. csrf_checks = False @@ -150,7 +151,6 @@ class TestDomainInvitationAdmin(MockEppLib, WebTest): self.client.force_login(self.superuser) self.app.set_user(self.superuser.username) - def tearDown(self): """Delete all DomainInvitation objects""" PortfolioInvitation.objects.all().delete() @@ -1128,6 +1128,7 @@ class TestDomainInvitationAdmin(MockEppLib, WebTest): custom_alert_content = "If you cancel the domain invitation here" self.assertContains(response, custom_alert_content) + class TestUserPortfolioPermissionAdmin(TestCase): """Tests for the PortfolioInivtationAdmin class""" From 40f21b86ee279f1ace236aca37204b57127b3bd6 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Fri, 7 Feb 2025 09:47:56 -0800 Subject: [PATCH 37/37] Remove unused library --- src/registrar/tests/test_admin.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index b488ee655..9447d211f 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -54,7 +54,6 @@ from registrar.models import ( from .common import ( MockDbForSharedTests, AuditedAdminMockData, - MockEppLib, completed_domain_request, generic_domain_object, less_console_noise, @@ -121,7 +120,7 @@ class TestFsmModelResource(TestCase): fsm_field_mock.save.assert_not_called() -class TestDomainInvitationAdmin(MockEppLib, WebTest): +class TestDomainInvitationAdmin(WebTest): """Tests for the DomainInvitationAdmin class as super user Notes: @@ -2073,7 +2072,7 @@ class TestDomainInformationAdmin(TestCase): self.test_helper.assert_table_sorted("-4", ("-creator__first_name", "-creator__last_name")) -class TestUserDomainRoleAdmin(MockEppLib, WebTest): +class TestUserDomainRoleAdmin(WebTest): """Tests for the UserDomainRoleAdmin class as super user Notes: