From d84a7890224ca1ec52c2e5097e8c0c1104589cdd Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 26 Dec 2024 17:10:37 -0800 Subject: [PATCH 01/77] Renewal form --- src/registrar/config/urls.py | 5 ++ src/registrar/fixtures/fixtures_domains.py | 2 +- src/registrar/models/domain.py | 5 ++ src/registrar/templates/domain_base.html | 9 +- src/registrar/templates/domain_detail.html | 4 +- src/registrar/templates/domain_sidebar.html | 10 ++- src/registrar/templatetags/custom_filters.py | 1 + src/registrar/views/__init__.py | 1 + src/registrar/views/domain.py | 92 ++++++++++++++++++-- 9 files changed, 117 insertions(+), 12 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 66708c571..2bf7b9e5f 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -345,6 +345,11 @@ urlpatterns = [ views.DomainSecurityEmailView.as_view(), name="domain-security-email", ), + path( + "domain//renewal", + views.DomainRenewalView.as_view(), + name="domain-renewal", + ), path( "domain//users/add", views.DomainAddUserView.as_view(), diff --git a/src/registrar/fixtures/fixtures_domains.py b/src/registrar/fixtures/fixtures_domains.py index 4606024d0..4d4115180 100644 --- a/src/registrar/fixtures/fixtures_domains.py +++ b/src/registrar/fixtures/fixtures_domains.py @@ -44,7 +44,7 @@ class DomainFixture(DomainRequestFixture): cls._approve_domain_requests(users) @staticmethod - def _generate_fake_expiration_date(days_in_future=365): + def _generate_fake_expiration_date(days_in_future=40): """Generates a fake expiration date between 1 and 365 days in the future.""" current_date = timezone.now().date() # nosec return current_date + timedelta(days=random.randint(1, days_in_future)) # nosec diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 6eb2fac07..3fa6a61b2 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1167,6 +1167,11 @@ class Domain(TimeStampedModel, DomainHelper): threshold_date = now + timedelta(days=60) return now < self.expiration_date <= threshold_date + ###dummy method for testing for domain renewal form fail or success banner + + def update_expiration(self, success=True): + return success + def state_display(self, request=None): """Return the display status of the domain.""" if self.is_expired() and (self.state != self.State.UNKNOWN): diff --git a/src/registrar/templates/domain_base.html b/src/registrar/templates/domain_base.html index 9f7e8d2e6..de8e88791 100644 --- a/src/registrar/templates/domain_base.html +++ b/src/registrar/templates/domain_base.html @@ -1,5 +1,7 @@ {% extends "base.html" %} {% load static %} +{% load static url_helpers %} + {% block title %}{{ domain.name }} | {% endblock %} @@ -53,8 +55,11 @@ {% endif %} {% block domain_content %} - + {% if request.path|endswith:"renewal"%} +

Renew {{domain.name}}

+ {%else%}

Domain Overview

+ {% endif%} {% endblock %} {# domain_content #} {% endif %} @@ -62,4 +67,4 @@ -{% endblock %} {# content #} +{% endblock %} {# content #} \ No newline at end of file diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index a5b8e52cb..b168f7e82 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -50,7 +50,9 @@ {% if domain.get_state_help_text %}
{% if has_domain_renewal_flag and domain.is_expiring and is_domain_manager %} - This domain will expire soon. Renew to maintain access. + This domain will expire soon. + {% url 'domain-renewal' pk=domain.id as url %} + Renew to maintain access. {% elif has_domain_renewal_flag and domain.is_expiring and is_portfolio_user %} This domain will expire soon. Contact one of the listed domain managers to renew the domain. {% else %} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 289f544ce..dc97f5ca1 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -79,8 +79,14 @@ {% with url_name="domain-users" %} {% include "includes/domain_sidenav_item.html" with item_text="Domain managers" %} {% endwith %} - + + {% if has_domain_renewal_flag and is_domain_manager and domain.is_expiring %} + {% with url_name="domain-renewal" %} + {% include "includes/domain_sidenav_item.html" with item_text="Renewal form" %} + {% endwith %} + {% endif %} + {% endif %} -
+ \ No newline at end of file diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 6140130c8..6f3894ea5 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -200,6 +200,7 @@ def is_domain_subpage(path): "domain-users-add", "domain-request-delete", "domain-user-delete", + "domain-renewal", "invitation-cancel", ] return get_url_name(path) in url_names diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index a80b16b1a..4e3faced1 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -14,6 +14,7 @@ from .domain import ( DomainInvitationCancelView, DomainDeleteUserView, PrototypeDomainDNSRecordView, + DomainRenewalView, ) from .user_profile import UserProfileView, FinishProfileSetupView from .health import * diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index cb3da1f83..99a173517 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -12,7 +12,7 @@ from django.contrib import messages from django.contrib.messages.views import SuccessMessageMixin from django.db import IntegrityError from django.http import HttpResponseRedirect -from django.shortcuts import redirect +from django.shortcuts import redirect, render from django.urls import reverse from django.views.generic.edit import FormMixin from django.conf import settings @@ -307,6 +307,90 @@ class DomainView(DomainBaseView): self._update_session_with_domain() +class DomainRenewalView(DomainBaseView): + """Domain detail overview page.""" + + template_name = "domain_renewal.html" + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + + default_emails = [DefaultEmail.PUBLIC_CONTACT_DEFAULT.value, DefaultEmail.LEGACY_DEFAULT.value] + context["hidden_security_emails"] = default_emails + + security_email = self.object.get_security_email() + user = self.request.user + if security_email is None or security_email in default_emails: + context["security_email"] = None + context["user"] = user + return context + + def can_access_domain_via_portfolio(self, pk): + """Most views should not allow permission to portfolio users. + If particular views allow permissions, they will need to override + this function.""" + portfolio = self.request.session.get("portfolio") + if self.request.user.has_any_domains_portfolio_permission(portfolio): + if Domain.objects.filter(id=pk).exists(): + domain = Domain.objects.get(id=pk) + if domain.domain_info.portfolio == portfolio: + return True + return False + + def in_editable_state(self, pk): + """Override in_editable_state from DomainPermission + Allow detail page to be viewable""" + + requested_domain = None + if Domain.objects.filter(id=pk).exists(): + requested_domain = Domain.objects.get(id=pk) + + # return true if the domain exists, this will allow the detail page to load + if requested_domain: + return True + return False + + def _get_domain(self, request): + """ + override get_domain for this view so that domain overview + always resets the cache for the domain object + """ + self.session = request.session + self.object = self.get_object() + self._update_session_with_domain() + + def post(self, request, pk): + domain = Domain.objects.filter(id=pk).first() + + # Check if the checkbox is checked + is_policy_acknowledged = request.POST.get("is_policy_acknowledged", None) + if is_policy_acknowledged != "on": + print("!!! Checkbox is NOT acknowledged") + messages.error( + request, "Check the box if you read and agree to the requirements for operating a .gov domain." + ) + return render( + request, + "domain_renewal.html", + { + "domain": domain, + "form": request.POST, + }, + ) + + print("*** Checkbox is acknowledged") + if "submit_button" in request.POST: + print("*** Submit button clicked") + updated_expiration = domain.update_expiration(success=True) + print("*** Updated expiration result:", updated_expiration) + + if updated_expiration is True: + messages.success(request, "This domain has been renewed for one year") + else: + messages.error(request, "This domain has not been renewed") + return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) + + class DomainOrgNameAddressView(DomainFormBaseView): """Organization view""" @@ -807,11 +891,7 @@ class DomainDNSSECView(DomainFormBaseView): has_dnssec_records = self.object.dnssecdata is not None # Create HTML for the modal button - modal_button = ( - '' - ) + modal_button = '' context["modal_button"] = modal_button context["has_dnssec_records"] = has_dnssec_records From b5658a357b6aa9b49c7c37393ae1f8893dbebbeb Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 26 Dec 2024 17:15:07 -0800 Subject: [PATCH 02/77] Fix linter --- src/registrar/models/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 3fa6a61b2..715f1b9da 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1167,7 +1167,7 @@ class Domain(TimeStampedModel, DomainHelper): threshold_date = now + timedelta(days=60) return now < self.expiration_date <= threshold_date - ###dummy method for testing for domain renewal form fail or success banner + # Dummy method for testing for domain renewal form fail or success banner def update_expiration(self, success=True): return success From 8b9eb93b682a3f8b714c42558b90db6b94bc7c57 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Mon, 30 Dec 2024 11:07:33 -0500 Subject: [PATCH 03/77] added back domain renewal form --- src/registrar/templates/domain_renewal.html | 127 ++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 src/registrar/templates/domain_renewal.html diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html new file mode 100644 index 000000000..c179941d6 --- /dev/null +++ b/src/registrar/templates/domain_renewal.html @@ -0,0 +1,127 @@ +{% extends "domain_base.html" %} +{% load static url_helpers %} +{% load custom_filters %} + +{% block domain_content %} + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} + + {{ block.super }} +
+

Confirm the following information for accuracy

+

Review these details below. We + require that you maintain accurate information for the domain. + The details you provide will only be used to support eh administration of .gov and won't be made public. +

+

If you would like to retire your domain instead, please + contact us.

+

Required fields are marked with an asterisk (*). +

+
+ + {% url 'user-profile' as url %} + {% include "includes/summary_item.html" with title='Your Contact Information' value=user edit_link=url editable=is_editable contact='true' %} + + + + {% if analyst_action != 'edit' or analyst_action_location != domain.pk %} + {% if is_portfolio_user and not is_domain_manager %} +
+
+

+ You don't have access to manage {{domain.name}}. If you need to make updates, contact one of the listed domain managers. +

+
+
+ {% endif %} + {% endif %} + + {% url 'domain-security-email' pk=domain.id as url %} + {% if security_email is not None and security_email not in hidden_security_emails%} + {% include "includes/summary_item.html" with title='Security email' value=security_email edit_link=url editable=is_editable %} + {% else %} + {% include "includes/summary_item.html" with title='Security email' value='None provided' edit_link=url editable=is_editable %} + {% endif %} + {% url 'domain-users' pk=domain.id as url %} + {% if portfolio %} + {% include "includes/summary_item.html" with title='Domain managers' domain_permissions=True value=domain edit_link=url editable=is_editable %} + {% else %} + {% include "includes/summary_item.html" with title='Domain managers' list=True users=True value=domain.permissions.all edit_link=url editable=is_editable %} + {% endif %} + + +
+ +
+ +

+ Acknowledgement of .gov domain requirements +

+ + + {% if messages %} +
    + {% for message in messages %} +

    {{ message }}

    + {% endfor %} + + {% endif %} + + +
    + {% csrf_token %} +
    +
    + + + + + +{% endblock %} {# domain_content #} \ No newline at end of file From 19e5700a4db2dd3d46ab4490ec37911c0c678f14 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 30 Dec 2024 09:05:40 -0800 Subject: [PATCH 04/77] Add in the domain renewal template oops --- src/registrar/templates/domain_renewal.html | 127 ++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 src/registrar/templates/domain_renewal.html diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html new file mode 100644 index 000000000..eda7a8bdd --- /dev/null +++ b/src/registrar/templates/domain_renewal.html @@ -0,0 +1,127 @@ +{% extends "domain_base.html" %} +{% load static url_helpers %} +{% load custom_filters %} + +{% block domain_content %} + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} + + {{ block.super }} +
    +

    Confirm the following information for accuracy

    +

    Review these details below. We + require that you maintain accurate information for the domain. + The details you provide will only be used to support eh administration of .gov and won't be made public. +

    +

    If you would like to retire your domain instead, please + contact us.

    +

    Required fields are marked with an asterisk (*). +

    +
    + + {% url 'user-profile' as url %} + {% include "includes/summary_item.html" with title='Your Contact Information' value=user edit_link=url editable=is_editable contact='true' %} + + + + {% if analyst_action != 'edit' or analyst_action_location != domain.pk %} + {% if is_portfolio_user and not is_domain_manager %} +
    +
    +

    + You don't have access to manage {{domain.name}}. If you need to make updates, contact one of the listed domain managers. +

    +
    +
    + {% endif %} + {% endif %} + + {% url 'domain-security-email' pk=domain.id as url %} + {% if security_email is not None and security_email not in hidden_security_emails%} + {% include "includes/summary_item.html" with title='Security email' value=security_email edit_link=url editable=is_editable %} + {% else %} + {% include "includes/summary_item.html" with title='Security email' value='None provided' edit_link=url editable=is_editable %} + {% endif %} + {% url 'domain-users' pk=domain.id as url %} + {% if portfolio %} + {% include "includes/summary_item.html" with title='Domain managers' domain_permissions=True value=domain edit_link=url editable=is_editable %} + {% else %} + {% include "includes/summary_item.html" with title='Domain managers' list=True users=True value=domain.permissions.all edit_link=url editable=is_editable %} + {% endif %} + + +
    + +
    + +

    + Acknowledgement of .gov domain requirements +

    +     + + {% if messages %} +
      + {% for message in messages %} +

      {{ message }}

      + {% endfor %} + + {% endif %} + + +
      + {% csrf_token %} +
      +
      + + + + + +{% endblock %} {# domain_content #} \ No newline at end of file From e23d81f7e7d4096e76450b0cca84908846484872 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Mon, 30 Dec 2024 12:44:52 -0500 Subject: [PATCH 05/77] added renew method --- src/registrar/models/domain.py | 6 +++++- src/registrar/views/domain.py | 11 ++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 715f1b9da..e170c8668 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -322,28 +322,32 @@ class Domain(TimeStampedModel, DomainHelper): """ # If no date is specified, grab the registry_expiration_date + print("checking if there is a date") try: exp_date = self.registry_expiration_date except KeyError: # if no expiration date from registry, set it to today logger.warning("current expiration date not set; setting to today") exp_date = date.today() - + print("we got the date", exp_date) # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=exp_date, period=epp.Period(length, unit)) try: # update expiration date in registry, and set the updated # expiration date in the registrar, and in the cache + print("we are in the second try") self._cache["ex_date"] = registry.send(request, cleaned=True).res_data[0].ex_date self.expiration_date = self._cache["ex_date"] self.save() except RegistryError as err: # if registry error occurs, log the error, and raise it as well + print("registry error") logger.error(f"registry error renewing domain: {err}") raise (err) except Exception as e: # exception raised during the save to registrar + print("this is the last error") logger.error(f"error updating expiration date in registrar: {e}") raise (e) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 99a173517..1c1996c65 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -381,12 +381,13 @@ class DomainRenewalView(DomainBaseView): print("*** Checkbox is acknowledged") if "submit_button" in request.POST: print("*** Submit button clicked") - updated_expiration = domain.update_expiration(success=True) - print("*** Updated expiration result:", updated_expiration) - - if updated_expiration is True: + # updated_expiration = domain.update_expiration(success=True) + # print("*** Updated expiration result:", updated_expiration) + try: + domain.renew_domain() messages.success(request, "This domain has been renewed for one year") - else: + except Exception as e: + print(f'An error occured: {e}') messages.error(request, "This domain has not been renewed") return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) From a128086d6f5e2cf51dda942dfc92c7dceb92a45d Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 31 Dec 2024 10:13:53 -0800 Subject: [PATCH 06/77] Update print statements and exp timing --- src/registrar/models/domain.py | 16 +++++++++------- src/registrar/views/domain.py | 5 +++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index e170c8668..b7145ec0c 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -322,32 +322,34 @@ class Domain(TimeStampedModel, DomainHelper): """ # If no date is specified, grab the registry_expiration_date - print("checking if there is a date") + print("*** Checking if there is a date") try: exp_date = self.registry_expiration_date except KeyError: # if no expiration date from registry, set it to today - logger.warning("current expiration date not set; setting to today") - exp_date = date.today() - print("we got the date", exp_date) + logger.warning("*** Current expiration date not set; setting to 35 days ") + # exp_date = date.today() + exp_date = date.today() - timedelta(days=35) + print(exp_date) + print("*** The exp_date is", exp_date) # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=exp_date, period=epp.Period(length, unit)) try: # update expiration date in registry, and set the updated # expiration date in the registrar, and in the cache - print("we are in the second try") + print("** In renew_domain in 2nd try statement") self._cache["ex_date"] = registry.send(request, cleaned=True).res_data[0].ex_date self.expiration_date = self._cache["ex_date"] self.save() except RegistryError as err: # if registry error occurs, log the error, and raise it as well - print("registry error") + print("*** Registry error") logger.error(f"registry error renewing domain: {err}") raise (err) except Exception as e: # exception raised during the save to registrar - print("this is the last error") + print("*** In renew_domain, in the last Exception statement") logger.error(f"error updating expiration date in registrar: {e}") raise (e) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1c1996c65..dd8e9928b 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -384,11 +384,12 @@ class DomainRenewalView(DomainBaseView): # updated_expiration = domain.update_expiration(success=True) # print("*** Updated expiration result:", updated_expiration) try: + print("*** Did we get into the try statement") domain.renew_domain() messages.success(request, "This domain has been renewed for one year") except Exception as e: - print(f'An error occured: {e}') - messages.error(request, "This domain has not been renewed") + print(f"An error occured: {e}") + messages.error(request, "*** This domain has not been renewed") return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) From c145ecbfa29d925af2806343eb8d8534677d8ac5 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Tue, 31 Dec 2024 13:55:18 -0500 Subject: [PATCH 07/77] put back the fixtures command --- src/registrar/fixtures/fixtures_domains.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/fixtures/fixtures_domains.py b/src/registrar/fixtures/fixtures_domains.py index 4d4115180..4606024d0 100644 --- a/src/registrar/fixtures/fixtures_domains.py +++ b/src/registrar/fixtures/fixtures_domains.py @@ -44,7 +44,7 @@ class DomainFixture(DomainRequestFixture): cls._approve_domain_requests(users) @staticmethod - def _generate_fake_expiration_date(days_in_future=40): + def _generate_fake_expiration_date(days_in_future=365): """Generates a fake expiration date between 1 and 365 days in the future.""" current_date = timezone.now().date() # nosec return current_date + timedelta(days=random.randint(1, days_in_future)) # nosec From e5f9696bac942a946c92430ff36dc6eba489b608 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Fri, 3 Jan 2025 10:43:35 -0500 Subject: [PATCH 08/77] added tests --- src/registrar/tests/test_views_domain.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index ba237e1e7..767445810 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -529,6 +529,21 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): ) self.assertContains(detail_page, "Renew to maintain access") + @override_flag("domain_renewal", active=True) + def test_domain_renewal_sidebar_and_form(self): + self.client.force_login(self.user) + with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( + Domain, "is_expired", self.custom_is_expired + ): + detail_page = self.client.get( + reverse("domain", kwargs={"pk": self.expiringdomain.id}), + ) + self.assertContains(detail_page, "Renewal form") + response = self.client.get(reverse("domain-renewal",kwargs={"pk": self.expiringdomain.id})) + self.assertEqual(response.status_code, 200) + self.assertContains(response, f"Renew {self.expiringdomain.name}") + + class TestDomainManagers(TestDomainOverview): @classmethod From 4feb29b269a39417e35fae2589b7026fbee757c3 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 3 Jan 2025 10:47:42 -0800 Subject: [PATCH 09/77] Add in tests for making edit is clickable and goes to right route --- src/registrar/tests/test_views_domain.py | 57 +++++++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 767445810..281b7291e 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -539,10 +539,63 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): reverse("domain", kwargs={"pk": self.expiringdomain.id}), ) self.assertContains(detail_page, "Renewal form") - response = self.client.get(reverse("domain-renewal",kwargs={"pk": self.expiringdomain.id})) + response = self.client.get(reverse("domain-renewal", kwargs={"pk": self.expiringdomain.id})) self.assertEqual(response.status_code, 200) self.assertContains(response, f"Renew {self.expiringdomain.name}") - + + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_your_contact_info_edit(self): + with less_console_noise(): + # Start on the Renewal page for the domain + renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) + + # Verify we see "Your Contact Information" on the renewal form + self.assertContains(renewal_page, "Your Contact Information") + + # Verify that the "Edit" button for Your Contact is there and links to correct URL + edit_button_url = reverse("user-profile") + self.assertContains(renewal_page, f'href="{edit_button_url}"') + + # Simulate clicking on edit button + edit_page = renewal_page.click(href=edit_button_url, index=1) + self.assertEqual(edit_page.status_code, 200) + self.assertContains(edit_page, "Review the details below and update any required information") + + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_security_contact_edit(self): + with less_console_noise(): + # Start on the Renewal page for the domain + renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) + + # Verify we see "Security email" on the renewal form + self.assertContains(renewal_page, "Security email") + + # Verify that the "Edit" button for Security email is there and links to correct URL + edit_button_url = reverse("domain-security-email", kwargs={"pk": self.domain_with_ip.id}) + self.assertContains(renewal_page, f'href="{edit_button_url}"') + + # Simulate clicking on edit button + edit_page = renewal_page.click(href=edit_button_url, index=1) + self.assertEqual(edit_page.status_code, 200) + self.assertContains(edit_page, "A security contact should be capable of evaluating") + + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_domain_manager_edit(self): + with less_console_noise(): + # Start on the Renewal page for the domain + renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) + + # Verify we see "Domain managers" on the renewal form + self.assertContains(renewal_page, "Domain managers") + + # Verify that the "Edit" button for Domain managers is there and links to correct URL + edit_button_url = reverse("domain-users", kwargs={"pk": self.domain_with_ip.id}) + self.assertContains(renewal_page, f'href="{edit_button_url}"') + + # Simulate clicking on edit button + edit_page = renewal_page.click(href=edit_button_url, index=1) + self.assertEqual(edit_page.status_code, 200) + self.assertContains(edit_page, "Domain managers can update all information related to a domain") class TestDomainManagers(TestDomainOverview): From 3f3ba22a119db0568861440f88dc0632d2ae4bfd Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 3 Jan 2025 11:28:48 -0800 Subject: [PATCH 10/77] Update text to click on link --- src/registrar/tests/test_views_domain.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 281b7291e..02d7aa9ac 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -535,11 +535,24 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( Domain, "is_expired", self.custom_is_expired ): + # Grab the detail page detail_page = self.client.get( reverse("domain", kwargs={"pk": self.expiringdomain.id}), ) + + # Make sure we see the link as a domain manager + self.assertContains(detail_page, "Renew to maintain access") + + # Make sure we can see Renewal form on the sidebar since it's expiring self.assertContains(detail_page, "Renewal form") - response = self.client.get(reverse("domain-renewal", kwargs={"pk": self.expiringdomain.id})) + + # Grab link to the renewal page + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.expiringdomain.id}) + self.assertContains(detail_page, f'href="{renewal_form_url}"') + + # Simulate clicking the link + response = self.client.get(renewal_form_url) + self.assertEqual(response.status_code, 200) self.assertContains(response, f"Renew {self.expiringdomain.name}") From dca70f4195237db300508421ecc59b0a43bab908 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Sat, 4 Jan 2025 14:17:55 -0800 Subject: [PATCH 11/77] Unit test for submit and recieving error message --- src/registrar/tests/test_views_domain.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 02d7aa9ac..50b8f31f6 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -9,6 +9,7 @@ from api.tests.common import less_console_noise_decorator from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .common import MockEppLib, MockSESClient, create_user # type: ignore from django_webtest import WebTest # type: ignore +from django.contrib.messages import get_messages import boto3_mocking # type: ignore from registrar.utility.errors import ( @@ -610,6 +611,25 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertEqual(edit_page.status_code, 200) self.assertContains(edit_page, "Domain managers can update all information related to a domain") + @override_flag("domain_renewal", active=True) + def test_ack_checkbox_not_checked(self): + + # Grab the renewal URL + renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) + + # Test clicking the checkbox + response = self.client.post(renewal_url, data={"submit_button": "next"}) + + # Verify the error message is displayed + # Retrieves messages obj (used in the post call) + messages = list(get_messages(response.wsgi_request)) + # Check we only get 1 error message + self.assertEqual(len(messages), 1) + # Check that the 1 error msg also is the right text + self.assertEqual( + str(messages[0]), + "Check the box if you read and agree to the requirements for operating a .gov domain.", + ) class TestDomainManagers(TestDomainOverview): @classmethod From bad8a8c98a75c1df161321c6a49d4b087d19e291 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Sat, 4 Jan 2025 14:48:21 -0800 Subject: [PATCH 12/77] Fix padding on form and addin subtext for security email --- src/registrar/templates/domain_renewal.html | 7 ++++--- src/registrar/templates/includes/summary_item.html | 8 +++++--- src/registrar/tests/test_views_domain.py | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html index eda7a8bdd..4bf69dbca 100644 --- a/src/registrar/templates/domain_renewal.html +++ b/src/registrar/templates/domain_renewal.html @@ -26,6 +26,7 @@ {{ block.super }}

      Confirm the following information for accuracy

      +

      HELLO

      Review these details below. We require that you maintain accurate information for the domain. The details you provide will only be used to support eh administration of .gov and won't be made public. @@ -55,9 +56,9 @@ {% url 'domain-security-email' pk=domain.id as url %} {% if security_email is not None and security_email not in hidden_security_emails%} - {% include "includes/summary_item.html" with title='Security email' value=security_email edit_link=url editable=is_editable %} + {% include "includes/summary_item.html" with title='Security email' value=security_email custom_text_for_value_none='We strongly recommend that you provide a security email. This email will allow the public to report observed or suspected security issues on your domain.' edit_link=url editable=is_editable %} {% else %} - {% include "includes/summary_item.html" with title='Security email' value='None provided' edit_link=url editable=is_editable %} + {% include "includes/summary_item.html" with title='Security email' value='None provided' custom_text_for_value_none='We strongly recommend that you provide a security email. This email will allow the public to report observed or suspected security issues on your domain.' edit_link=url editable=is_editable %} {% endif %} {% url 'domain-users' pk=domain.id as url %} {% if portfolio %} @@ -67,7 +68,7 @@ {% endif %} -

      +
      diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 15cc0f67f..facc8956a 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -128,11 +128,13 @@ {% endif %} {% else %}

      + {% if custom_text_for_value_none %} +

      {{ custom_text_for_value_none }}

      + {% endif %} {% if value %} {{ value }} - {% elif custom_text_for_value_none %} - {{ custom_text_for_value_none }} - {% else %} + {% endif %} + {% if not value and not custom_text_for_value_none %} None {% endif %}

      diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 50b8f31f6..fa4351de1 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -631,6 +631,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): "Check the box if you read and agree to the requirements for operating a .gov domain.", ) + class TestDomainManagers(TestDomainOverview): @classmethod def setUpClass(cls): From a3c50043c5a1d6fdc7154ad2944926d39181d9ac Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Sat, 4 Jan 2025 14:55:40 -0800 Subject: [PATCH 13/77] Removing print statements --- src/registrar/models/domain.py | 16 ++-------------- src/registrar/templates/domain_renewal.html | 1 - src/registrar/views/domain.py | 7 ------- 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index b7145ec0c..59e04e7c3 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -322,34 +322,27 @@ class Domain(TimeStampedModel, DomainHelper): """ # If no date is specified, grab the registry_expiration_date - print("*** Checking if there is a date") try: exp_date = self.registry_expiration_date except KeyError: # if no expiration date from registry, set it to today - logger.warning("*** Current expiration date not set; setting to 35 days ") - # exp_date = date.today() - exp_date = date.today() - timedelta(days=35) - print(exp_date) - print("*** The exp_date is", exp_date) + logger.warning("current expiration date not set; setting to today") + exp_date = date.today() # create RenewDomain request request = commands.RenewDomain(name=self.name, cur_exp_date=exp_date, period=epp.Period(length, unit)) try: # update expiration date in registry, and set the updated # expiration date in the registrar, and in the cache - print("** In renew_domain in 2nd try statement") self._cache["ex_date"] = registry.send(request, cleaned=True).res_data[0].ex_date self.expiration_date = self._cache["ex_date"] self.save() except RegistryError as err: # if registry error occurs, log the error, and raise it as well - print("*** Registry error") logger.error(f"registry error renewing domain: {err}") raise (err) except Exception as e: # exception raised during the save to registrar - print("*** In renew_domain, in the last Exception statement") logger.error(f"error updating expiration date in registrar: {e}") raise (e) @@ -1173,11 +1166,6 @@ class Domain(TimeStampedModel, DomainHelper): threshold_date = now + timedelta(days=60) return now < self.expiration_date <= threshold_date - # Dummy method for testing for domain renewal form fail or success banner - - def update_expiration(self, success=True): - return success - def state_display(self, request=None): """Return the display status of the domain.""" if self.is_expired() and (self.state != self.State.UNKNOWN): diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html index 4bf69dbca..07aeecd1a 100644 --- a/src/registrar/templates/domain_renewal.html +++ b/src/registrar/templates/domain_renewal.html @@ -26,7 +26,6 @@ {{ block.super }}

      Confirm the following information for accuracy

      -

      HELLO

      Review these details below. We require that you maintain accurate information for the domain. The details you provide will only be used to support eh administration of .gov and won't be made public. diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 44ac9e9d6..ab68addb7 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -365,7 +365,6 @@ class DomainRenewalView(DomainBaseView): # Check if the checkbox is checked is_policy_acknowledged = request.POST.get("is_policy_acknowledged", None) if is_policy_acknowledged != "on": - print("!!! Checkbox is NOT acknowledged") messages.error( request, "Check the box if you read and agree to the requirements for operating a .gov domain." ) @@ -378,17 +377,11 @@ class DomainRenewalView(DomainBaseView): }, ) - print("*** Checkbox is acknowledged") if "submit_button" in request.POST: - print("*** Submit button clicked") - # updated_expiration = domain.update_expiration(success=True) - # print("*** Updated expiration result:", updated_expiration) try: - print("*** Did we get into the try statement") domain.renew_domain() messages.success(request, "This domain has been renewed for one year") except Exception as e: - print(f"An error occured: {e}") messages.error(request, "*** This domain has not been renewed") return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) From 0118e1f00dd2abe550d851c5fb0ba77f81b0149e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 11:36:44 -0700 Subject: [PATCH 14/77] Add suborg data and clean duplicate script --- .../commands/clean_duplicate_suborgs.py | 123 ++++++++++++++++++ .../commands/create_federal_portfolio.py | 23 +++- 2 files changed, 139 insertions(+), 7 deletions(-) create mode 100644 src/registrar/management/commands/clean_duplicate_suborgs.py diff --git a/src/registrar/management/commands/clean_duplicate_suborgs.py b/src/registrar/management/commands/clean_duplicate_suborgs.py new file mode 100644 index 000000000..a5c7a87f0 --- /dev/null +++ b/src/registrar/management/commands/clean_duplicate_suborgs.py @@ -0,0 +1,123 @@ +import logging +from django.core.management import BaseCommand +from registrar.models import Suborganization, DomainRequest, DomainInformation +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper + + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Clean up duplicate suborganizations that differ only by spaces and capitalization" + + def handle(self, **kwargs): + # Find duplicates + duplicates = {} + all_suborgs = Suborganization.objects.all() + + for suborg in all_suborgs: + # Normalize name by removing extra spaces and converting to lowercase + normalized_name = " ".join(suborg.name.split()).lower() + + # First occurrence of this name + if normalized_name not in duplicates: + duplicates[normalized_name] = { + "keep": suborg, + "delete": [] + } + continue + + # Compare with our current best + current_best = duplicates[normalized_name]["keep"] + + # Check if all other fields match. + # If they don't, we should inspect this record manually. + fields_to_compare = ["portfolio", "city", "state_territory"] + fields_match = all( + getattr(suborg, field) == getattr(current_best, field) + for field in fields_to_compare + ) + if not fields_match: + logger.warning( + f"{TerminalColors.YELLOW}" + f"\nSkipping potential duplicate: {suborg.name} (id: {suborg.id})" + f"\nData mismatch with {current_best.name} (id: {current_best.id})" + f"{TerminalColors.ENDC}" + ) + continue + + # Determine if new suborg is better than current best. + # The fewest spaces and most capitals wins. + new_has_fewer_spaces = suborg.name.count(" ") < current_best.name.count(" ") + new_has_more_capitals = sum(1 for c in suborg.name if c.isupper()) > sum(1 for c in current_best.name if c.isupper()) + # TODO + # Split into words and count properly capitalized first letters + # new_proper_caps = sum( + # 1 for word in suborg.name.split() + # if word and word[0].isupper() + # ) + # current_proper_caps = sum( + # 1 for word in current_best.name.split() + # if word and word[0].isupper() + # ) + # new_has_better_caps = new_proper_caps > current_proper_caps + + if new_has_fewer_spaces or new_has_more_capitals: + # New suborg is better - demote the old one to the delete list + duplicates[normalized_name]["delete"].append(current_best) + duplicates[normalized_name]["keep"] = suborg + else: + # If it is not better, just delete the old one + duplicates[normalized_name]["delete"].append(suborg) + + # Filter out entries without duplicates + duplicates = {k: v for k, v in duplicates.items() if v.get("delete")} + if not duplicates: + logger.info(f"No duplicate suborganizations found.") + return + + # Show preview of changes + preview = "The following duplicates will be removed:\n" + for data in duplicates.values(): + best = data.get("keep") + preview += f"\nKeeping: '{best.name}' (id: {best.id})" + + for duplicate in data.get("delete"): + preview += f"\nRemoving: '{duplicate.name}' (id: {duplicate.id})" + preview += "\n" + + # Get confirmation and execute deletions + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + prompt_message=preview, + prompt_title="Clean up duplicate suborganizations?", + verify_message="*** WARNING: This will delete suborganizations! ***" + ): + try: + # Update all references to point to the right suborg before deletion + for record in duplicates.values(): + best_record = record.get("keep") + delete_ids = [dupe.id for dupe in record.get("delete")] + + # Update domain requests + DomainRequest.objects.filter( + sub_organization_id__in=delete_ids + ).update(sub_organization=best_record) + + # Update domain information + DomainInformation.objects.filter( + sub_organization_id__in=delete_ids + ).update(sub_organization=best_record) + + ids_to_delete = [ + dupe.id + for data in duplicates.values() + for dupe in data["delete"] + ] + + # Bulk delete all duplicates + delete_count, _ = Suborganization.objects.filter(id__in=ids_to_delete).delete() + logger.info(f"{TerminalColors.OKGREEN}Successfully deleted {delete_count} suborganizations{TerminalColors.ENDC}") + + except Exception as e: + logger.error(f"{TerminalColors.FAIL}Failed to clean up suborganizations: {str(e)}{TerminalColors.ENDC}") diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 9cf4d36ea..b8a0ed091 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -104,7 +104,11 @@ class Command(BaseCommand): also create new suborganizations""" portfolio, created = self.create_portfolio(federal_agency) if created: - self.create_suborganizations(portfolio, federal_agency) + valid_agencies = DomainInformation.objects.filter( + federal_agency=federal_agency, organization_name__isnull=False + ) + org_names = set(valid_agencies.values_list("organization_name", flat=True)) + self.create_suborganizations(portfolio, federal_agency, org_names) if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) @@ -155,13 +159,8 @@ class Command(BaseCommand): return portfolio, True - def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): + def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency, org_names: set): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" - valid_agencies = DomainInformation.objects.filter( - federal_agency=federal_agency, organization_name__isnull=False - ) - org_names = set(valid_agencies.values_list("organization_name", flat=True)) - if not org_names: message = ( "Could not add any suborganizations." @@ -232,6 +231,16 @@ class Command(BaseCommand): domain_request.portfolio = portfolio if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) + else: + # Fill in the requesting suborg fields if we have the data to do so + if domain_request.organization_name and domain_request.city and domain_request.state_territory: + domain_request.requested_suborganization = domain_request.organization_name + domain_request.suborganization_city = domain_request.city + domain_request.suborganization_state_territory = domain_request.state_territory + else: + message = f"No suborganization data found whatsoever for {domain_request}." + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) From d0183d4d149c643d638c3fd9856ef90ef715ae67 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:42:49 -0700 Subject: [PATCH 15/77] cleanup --- .../commands/clean_duplicate_suborgs.py | 23 +++++-------------- .../commands/create_federal_portfolio.py | 2 +- .../models/utility/generic_helper.py | 6 +++++ 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/registrar/management/commands/clean_duplicate_suborgs.py b/src/registrar/management/commands/clean_duplicate_suborgs.py index a5c7a87f0..2e44746ad 100644 --- a/src/registrar/management/commands/clean_duplicate_suborgs.py +++ b/src/registrar/management/commands/clean_duplicate_suborgs.py @@ -17,7 +17,7 @@ class Command(BaseCommand): for suborg in all_suborgs: # Normalize name by removing extra spaces and converting to lowercase - normalized_name = " ".join(suborg.name.split()).lower() + normalized_name = " ".join(suborg.name.trim().split()).lower() # First occurrence of this name if normalized_name not in duplicates: @@ -28,7 +28,8 @@ class Command(BaseCommand): continue # Compare with our current best - current_best = duplicates[normalized_name]["keep"] + duplicate_record = duplicates.get(normalized_name) + current_best = duplicate_record.get("keep") # Check if all other fields match. # If they don't, we should inspect this record manually. @@ -50,25 +51,13 @@ class Command(BaseCommand): # The fewest spaces and most capitals wins. new_has_fewer_spaces = suborg.name.count(" ") < current_best.name.count(" ") new_has_more_capitals = sum(1 for c in suborg.name if c.isupper()) > sum(1 for c in current_best.name if c.isupper()) - # TODO - # Split into words and count properly capitalized first letters - # new_proper_caps = sum( - # 1 for word in suborg.name.split() - # if word and word[0].isupper() - # ) - # current_proper_caps = sum( - # 1 for word in current_best.name.split() - # if word and word[0].isupper() - # ) - # new_has_better_caps = new_proper_caps > current_proper_caps - if new_has_fewer_spaces or new_has_more_capitals: # New suborg is better - demote the old one to the delete list - duplicates[normalized_name]["delete"].append(current_best) - duplicates[normalized_name]["keep"] = suborg + duplicate_record["delete"].append(current_best) + duplicate_record["keep"] = suborg else: # If it is not better, just delete the old one - duplicates[normalized_name]["delete"].append(suborg) + duplicate_record["delete"].append(suborg) # Filter out entries without duplicates duplicates = {k: v for k, v in duplicates.items() if v.get("delete")} diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index b8a0ed091..15740cca9 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -107,7 +107,7 @@ class Command(BaseCommand): valid_agencies = DomainInformation.objects.filter( federal_agency=federal_agency, organization_name__isnull=False ) - org_names = set(valid_agencies.values_list("organization_name", flat=True)) + org_names = set([agency.trim() for agency in valid_agencies.values_list("organization_name", flat=True)]) self.create_suborganizations(portfolio, federal_agency, org_names) if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 5e425f5a3..d1812d476 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,3 +343,9 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value + + +def normalize_string(string_to_normalize: str) -> str: + """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + new_string = " ".join(string_to_normalize.trim().split()) + return new_string.lower() From 6086b9587f10ab44aa0274a2289fa511663138ca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:43:48 -0700 Subject: [PATCH 16/77] Remove unrelated content --- .../commands/clean_duplicate_suborgs.py | 112 ------------------ .../models/utility/generic_helper.py | 6 - 2 files changed, 118 deletions(-) delete mode 100644 src/registrar/management/commands/clean_duplicate_suborgs.py diff --git a/src/registrar/management/commands/clean_duplicate_suborgs.py b/src/registrar/management/commands/clean_duplicate_suborgs.py deleted file mode 100644 index 2e44746ad..000000000 --- a/src/registrar/management/commands/clean_duplicate_suborgs.py +++ /dev/null @@ -1,112 +0,0 @@ -import logging -from django.core.management import BaseCommand -from registrar.models import Suborganization, DomainRequest, DomainInformation -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper - - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - help = "Clean up duplicate suborganizations that differ only by spaces and capitalization" - - def handle(self, **kwargs): - # Find duplicates - duplicates = {} - all_suborgs = Suborganization.objects.all() - - for suborg in all_suborgs: - # Normalize name by removing extra spaces and converting to lowercase - normalized_name = " ".join(suborg.name.trim().split()).lower() - - # First occurrence of this name - if normalized_name not in duplicates: - duplicates[normalized_name] = { - "keep": suborg, - "delete": [] - } - continue - - # Compare with our current best - duplicate_record = duplicates.get(normalized_name) - current_best = duplicate_record.get("keep") - - # Check if all other fields match. - # If they don't, we should inspect this record manually. - fields_to_compare = ["portfolio", "city", "state_territory"] - fields_match = all( - getattr(suborg, field) == getattr(current_best, field) - for field in fields_to_compare - ) - if not fields_match: - logger.warning( - f"{TerminalColors.YELLOW}" - f"\nSkipping potential duplicate: {suborg.name} (id: {suborg.id})" - f"\nData mismatch with {current_best.name} (id: {current_best.id})" - f"{TerminalColors.ENDC}" - ) - continue - - # Determine if new suborg is better than current best. - # The fewest spaces and most capitals wins. - new_has_fewer_spaces = suborg.name.count(" ") < current_best.name.count(" ") - new_has_more_capitals = sum(1 for c in suborg.name if c.isupper()) > sum(1 for c in current_best.name if c.isupper()) - if new_has_fewer_spaces or new_has_more_capitals: - # New suborg is better - demote the old one to the delete list - duplicate_record["delete"].append(current_best) - duplicate_record["keep"] = suborg - else: - # If it is not better, just delete the old one - duplicate_record["delete"].append(suborg) - - # Filter out entries without duplicates - duplicates = {k: v for k, v in duplicates.items() if v.get("delete")} - if not duplicates: - logger.info(f"No duplicate suborganizations found.") - return - - # Show preview of changes - preview = "The following duplicates will be removed:\n" - for data in duplicates.values(): - best = data.get("keep") - preview += f"\nKeeping: '{best.name}' (id: {best.id})" - - for duplicate in data.get("delete"): - preview += f"\nRemoving: '{duplicate.name}' (id: {duplicate.id})" - preview += "\n" - - # Get confirmation and execute deletions - if TerminalHelper.prompt_for_execution( - system_exit_on_terminate=True, - prompt_message=preview, - prompt_title="Clean up duplicate suborganizations?", - verify_message="*** WARNING: This will delete suborganizations! ***" - ): - try: - # Update all references to point to the right suborg before deletion - for record in duplicates.values(): - best_record = record.get("keep") - delete_ids = [dupe.id for dupe in record.get("delete")] - - # Update domain requests - DomainRequest.objects.filter( - sub_organization_id__in=delete_ids - ).update(sub_organization=best_record) - - # Update domain information - DomainInformation.objects.filter( - sub_organization_id__in=delete_ids - ).update(sub_organization=best_record) - - ids_to_delete = [ - dupe.id - for data in duplicates.values() - for dupe in data["delete"] - ] - - # Bulk delete all duplicates - delete_count, _ = Suborganization.objects.filter(id__in=ids_to_delete).delete() - logger.info(f"{TerminalColors.OKGREEN}Successfully deleted {delete_count} suborganizations{TerminalColors.ENDC}") - - except Exception as e: - logger.error(f"{TerminalColors.FAIL}Failed to clean up suborganizations: {str(e)}{TerminalColors.ENDC}") diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index d1812d476..5e425f5a3 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,9 +343,3 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value - - -def normalize_string(string_to_normalize: str) -> str: - """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" - new_string = " ".join(string_to_normalize.trim().split()) - return new_string.lower() From 345ca0307e161fe76e982b941b18064edd704f43 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:46:36 -0700 Subject: [PATCH 17/77] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 15740cca9..f73f1ec66 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -104,11 +104,7 @@ class Command(BaseCommand): also create new suborganizations""" portfolio, created = self.create_portfolio(federal_agency) if created: - valid_agencies = DomainInformation.objects.filter( - federal_agency=federal_agency, organization_name__isnull=False - ) - org_names = set([agency.trim() for agency in valid_agencies.values_list("organization_name", flat=True)]) - self.create_suborganizations(portfolio, federal_agency, org_names) + self.create_suborganizations(portfolio, federal_agency) if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) @@ -159,8 +155,13 @@ class Command(BaseCommand): return portfolio, True - def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency, org_names: set): + def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" + valid_agencies = DomainInformation.objects.filter( + federal_agency=federal_agency, organization_name__isnull=False + ) + org_names = set(valid_agencies.values_list("organization_name", flat=True)) + if not org_names: message = ( "Could not add any suborganizations." From 5e6a7987dfbd6ef9222c9adc66ead235f9433b8b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:47:57 -0700 Subject: [PATCH 18/77] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index f73f1ec66..5fac24f53 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -233,14 +233,9 @@ class Command(BaseCommand): if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) else: - # Fill in the requesting suborg fields if we have the data to do so - if domain_request.organization_name and domain_request.city and domain_request.state_territory: - domain_request.requested_suborganization = domain_request.organization_name - domain_request.suborganization_city = domain_request.city - domain_request.suborganization_state_territory = domain_request.state_territory - else: - message = f"No suborganization data found whatsoever for {domain_request}." - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + domain_request.requested_suborganization = domain_request.organization_name + domain_request.suborganization_city = domain_request.city + domain_request.suborganization_state_territory = domain_request.state_territory self.updated_portfolios.add(portfolio) From 9052d9ed3f3924293675879c9e6955ffb4cf06a4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:21:50 -0700 Subject: [PATCH 19/77] Use dedicated script --- .../commands/create_federal_portfolio.py | 5 --- .../commands/populate_requested_suborg.py | 37 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 src/registrar/management/commands/populate_requested_suborg.py diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 5fac24f53..9cf4d36ea 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -232,11 +232,6 @@ class Command(BaseCommand): domain_request.portfolio = portfolio if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) - else: - domain_request.requested_suborganization = domain_request.organization_name - domain_request.suborganization_city = domain_request.city - domain_request.suborganization_state_territory = domain_request.state_territory - self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py new file mode 100644 index 000000000..54f811413 --- /dev/null +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -0,0 +1,37 @@ +import logging +from django.core.management import BaseCommand +from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper +from registrar.models import DomainRequest + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand, PopulateScriptTemplate): + help = "Loops through each domain request object and populates the last_status_update and first_submitted_date" + + def handle(self, **kwargs): + """Loops through each DomainRequest object and populates + its last_status_update and first_submitted_date values""" + filter_conditions = {"portfolio__isnull": False, "sub_organization__isnull": True} + fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] + self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) + + def update_record(self, record: DomainRequest): + """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" + record.requested_suborganization = record.organization_name + record.suborganization_city = record.city + record.suborganization_state_territory = record.state_territory + message = ( + f"Updating {record} => requested_suborg: {record.organization_name}, " + f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." + ) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) + + def should_skip_record(self, record: DomainRequest) -> bool: + """Skips record update if we're missing org name, city, and state territory.""" + required_fields = [ + record.organization_name, + record.city, + record.state_territory + ] + return not all(bool(field) for field in required_fields) From 188c99a16971320c1a9b54d84103539f51c3a9bd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:39:56 -0700 Subject: [PATCH 20/77] Unit tests and documentation --- docs/operations/data_migration.md | 33 ++++++++ .../commands/populate_requested_suborg.py | 11 +-- src/registrar/tests/common.py | 8 +- .../tests/test_management_scripts.py | 77 ++++++++++++++++++- 4 files changed, 117 insertions(+), 12 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 0863aa0b7..93a5a4241 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -918,3 +918,36 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi - Parameters #1-#2: Either `--agency_name` or `--branch` must be specified. Not both. - Parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, you must specify at least one to run this script. + + +## Populate requested suborg +This script populates the "requested suborganization" fields on DomainRequest. +These fields are: requested_suborganization, suborganization_city, suborganization_state_territory. + +This is done by pulling from organization_name, city, and state_territory. +The script will only parse records with a portfolio but no suborganization attached to it. + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Upload your csv to the desired sandbox +[Follow these steps](#use-scp-to-transfer-data-to-sandboxes) to upload the federal_cio csv to a sandbox of your choice. + +#### Step 5: Running the script +To create a specific portfolio: +```./manage.py populate_requested_suborg``` + +### Running locally + +#### Step 1: Running the script +```docker-compose exec app ./manage.py populate_requested_suborg``` diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 54f811413..8bb0a77b9 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -7,7 +7,7 @@ logger = logging.getLogger(__name__) class Command(BaseCommand, PopulateScriptTemplate): - help = "Loops through each domain request object and populates the last_status_update and first_submitted_date" + help = "Loops through each domain request object and populates requested suborg info" def handle(self, **kwargs): """Loops through each DomainRequest object and populates @@ -26,12 +26,3 @@ class Command(BaseCommand, PopulateScriptTemplate): f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." ) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) - - def should_skip_record(self, record: DomainRequest) -> bool: - """Skips record update if we're missing org name, city, and state territory.""" - required_fields = [ - record.organization_name, - record.city, - record.state_territory - ] - return not all(bool(field) for field in required_fields) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 8eca0108e..b79cbe0c2 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1032,6 +1032,8 @@ def completed_domain_request( # noqa federal_agency=None, federal_type=None, action_needed_reason=None, + city=None, + state_territory=None, portfolio=None, organization_name=None, sub_organization=None, @@ -1076,7 +1078,7 @@ def completed_domain_request( # noqa organization_name=organization_name if organization_name else "Testorg", address_line1="address 1", address_line2="address 2", - state_territory="NY", + state_territory="NY" if not state_territory else state_territory, zipcode="10002", senior_official=so, requested_domain=domain, @@ -1085,6 +1087,10 @@ def completed_domain_request( # noqa investigator=investigator, federal_agency=federal_agency, ) + + if city: + domain_request_kwargs["city"] = city + if has_about_your_organization: domain_request_kwargs["about_your_organization"] = "e-Government" if has_anything_else: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 7cce0d2b2..dfe0904c5 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -32,7 +32,7 @@ import tablib from unittest.mock import patch, call, MagicMock, mock_open from epplibwrapper import commands, common -from .common import MockEppLib, less_console_noise, completed_domain_request, MockSESClient +from .common import MockEppLib, create_user, less_console_noise, completed_domain_request, MockSESClient from api.tests.common import less_console_noise_decorator @@ -1731,3 +1731,78 @@ class TestCreateFederalPortfolio(TestCase): self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) + + +class TestPopulateRequestedSuborg(MockEppLib): + """Tests for the populate_requested_suborg script""" + + @less_console_noise_decorator + def setUp(self): + """Creates test domain requests with various states""" + super().setUp() + + self.user = create_user() + # Create a portfolio + self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.user) + + # Create a domain request with all required fields + self.complete_request = completed_domain_request( + name="complete.gov", + organization_name="Complete Org", + city="Washington", + state_territory="DC", + portfolio=self.portfolio, + ) + + # Create a request that already has a suborganization + self.suborg = Suborganization.objects.create(name="Existing Suborg", portfolio=self.portfolio) + self.existing_suborg_request = completed_domain_request( + name="existing.gov", + organization_name="Existing Org", + city="New York", + state_territory="NY", + portfolio=self.portfolio, + sub_organization=self.suborg, + ) + + def tearDown(self): + """Cleans up test data""" + super().tearDown() + DomainRequest.objects.all().delete() + Suborganization.objects.all().delete() + Portfolio.objects.all().delete() + User.objects.all().delete() + + @less_console_noise_decorator + def run_populate_requested_suborg(self): + """Executes the populate_requested_suborg command""" + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", + return_value=True, + ): + call_command("populate_requested_suborg") + + @less_console_noise_decorator + def test_populate_requested_suborg_complete_request(self): + """Tests that complete requests are updated correctly""" + self.run_populate_requested_suborg() + + self.complete_request.refresh_from_db() + + # Verify fields were populated correctly + self.assertEqual(self.complete_request.requested_suborganization, "Complete Org") + self.assertEqual(self.complete_request.suborganization_city, "Washington") + self.assertEqual(self.complete_request.suborganization_state_territory, "DC") + + @less_console_noise_decorator + def test_populate_requested_suborg_existing_suborg(self): + """Tests that requests with existing suborgs are skipped""" + self.run_populate_requested_suborg() + + self.existing_suborg_request.refresh_from_db() + + # Verify the request wasn't modified + self.assertEqual(self.existing_suborg_request.sub_organization, self.suborg) + self.assertIsNone(self.existing_suborg_request.requested_suborganization) + self.assertIsNone(self.existing_suborg_request.suborganization_city) + self.assertIsNone(self.existing_suborg_request.suborganization_state_territory) From 822c8dde0f1273368cfab986144e96344c016574 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:40:51 -0700 Subject: [PATCH 21/77] Update test_management_scripts.py --- src/registrar/tests/test_management_scripts.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index dfe0904c5..de35fb02b 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1738,7 +1738,6 @@ class TestPopulateRequestedSuborg(MockEppLib): @less_console_noise_decorator def setUp(self): - """Creates test domain requests with various states""" super().setUp() self.user = create_user() @@ -1766,7 +1765,6 @@ class TestPopulateRequestedSuborg(MockEppLib): ) def tearDown(self): - """Cleans up test data""" super().tearDown() DomainRequest.objects.all().delete() Suborganization.objects.all().delete() From 6d13f26ffb18e972d5ebcaf582a2efb6cf5b33fd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:51:40 -0700 Subject: [PATCH 22/77] Add rule not to parse cases where the org name matches the portfolio name --- docs/operations/data_migration.md | 1 + .../commands/populate_requested_suborg.py | 5 +++++ .../tests/test_management_scripts.py | 22 +++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 93a5a4241..32de4d31a 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -926,6 +926,7 @@ These fields are: requested_suborganization, suborganization_city, suborganizati This is done by pulling from organization_name, city, and state_territory. The script will only parse records with a portfolio but no suborganization attached to it. +Additionally, it will not parse records wherein the organization name matches the portfolio org name. ### Running on sandboxes diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 8bb0a77b9..e33a93e42 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -2,6 +2,7 @@ import logging from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper from registrar.models import DomainRequest +from django.db.models import F logger = logging.getLogger(__name__) @@ -16,6 +17,10 @@ class Command(BaseCommand, PopulateScriptTemplate): fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) + def custom_filter(self, records): + """Exclude domain requests that have the same org name as the portfolio""" + return records.exclude(organization_name=F("portfolio__organization_name")) + def update_record(self, record: DomainRequest): """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" record.requested_suborganization = record.organization_name diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index de35fb02b..29b310fd7 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1764,6 +1764,16 @@ class TestPopulateRequestedSuborg(MockEppLib): sub_organization=self.suborg, ) + # Create a request where org name matches portfolio name + self.matching_name_request = completed_domain_request( + name="matching.gov", + organization_name="Test Portfolio", + city="Boston", + state_territory="MA", + portfolio=self.portfolio, + user=self.user, + ) + def tearDown(self): super().tearDown() DomainRequest.objects.all().delete() @@ -1804,3 +1814,15 @@ class TestPopulateRequestedSuborg(MockEppLib): self.assertIsNone(self.existing_suborg_request.requested_suborganization) self.assertIsNone(self.existing_suborg_request.suborganization_city) self.assertIsNone(self.existing_suborg_request.suborganization_state_territory) + + @less_console_noise_decorator + def test_populate_requested_suborg_matching_name(self): + """Tests that requests with matching org/portfolio names are skipped""" + self.run_populate_requested_suborg() + + self.matching_name_request.refresh_from_db() + + # Verify the request wasn't modified since org name matches portfolio name + self.assertIsNone(self.matching_name_request.requested_suborganization) + self.assertIsNone(self.matching_name_request.suborganization_city) + self.assertIsNone(self.matching_name_request.suborganization_state_territory) From 87b08e3fa0111bfa7521819b89f7c34200e8eeca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:00:52 -0700 Subject: [PATCH 23/77] Update populate_requested_suborg.py --- src/registrar/management/commands/populate_requested_suborg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index e33a93e42..e65ed3f53 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -19,7 +19,7 @@ class Command(BaseCommand, PopulateScriptTemplate): def custom_filter(self, records): """Exclude domain requests that have the same org name as the portfolio""" - return records.exclude(organization_name=F("portfolio__organization_name")) + return records.exclude(organization_name__iexact=F("portfolio__organization_name")) def update_record(self, record: DomainRequest): """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" From 8e7ffab8b2a34b3812c46fd6a254c1233d40bdd9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:10:40 -0700 Subject: [PATCH 24/77] Exclude approved --- .../management/commands/populate_requested_suborg.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index e65ed3f53..2fdb14ac2 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -18,8 +18,12 @@ class Command(BaseCommand, PopulateScriptTemplate): self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) def custom_filter(self, records): - """Exclude domain requests that have the same org name as the portfolio""" - return records.exclude(organization_name__iexact=F("portfolio__organization_name")) + """Exclude domain requests that have the same org name as the portfolio. + Also excludes approved.""" + return records.exclude( + organization_name__iexact=F("portfolio__organization_name"), + status=DomainRequest.DomainRequestStatus.APPROVED, + ) def update_record(self, record: DomainRequest): """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" From 4fceb62c434b41da8bee79c8f55560d3423bb471 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:17:33 -0700 Subject: [PATCH 25/77] Update populate_requested_suborg.py --- .../management/commands/populate_requested_suborg.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 2fdb14ac2..580d1cdbc 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -22,7 +22,12 @@ class Command(BaseCommand, PopulateScriptTemplate): Also excludes approved.""" return records.exclude( organization_name__iexact=F("portfolio__organization_name"), - status=DomainRequest.DomainRequestStatus.APPROVED, + status__in=[ + DomainRequest.DomainRequestStatus.APPROVED, + DomainRequest.DomainRequestStatus.REJECTED, + DomainRequest.DomainRequestStatus.INELIGIBLE, + DomainRequest.DomainRequestStatus.STARTED, + ], ) def update_record(self, record: DomainRequest): From e91c160a67301963140132b381dc4fdd6d0695c2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:51:43 -0700 Subject: [PATCH 26/77] bug fix --- .../commands/populate_requested_suborg.py | 34 ++++++++++++------- .../models/utility/generic_helper.py | 6 ++++ .../tests/test_management_scripts.py | 3 ++ 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 580d1cdbc..121eb497c 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -4,6 +4,8 @@ from registrar.management.commands.utility.terminal_helper import PopulateScript from registrar.models import DomainRequest from django.db.models import F +from registrar.models.utility.generic_helper import normalize_string + logger = logging.getLogger(__name__) @@ -17,19 +19,6 @@ class Command(BaseCommand, PopulateScriptTemplate): fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) - def custom_filter(self, records): - """Exclude domain requests that have the same org name as the portfolio. - Also excludes approved.""" - return records.exclude( - organization_name__iexact=F("portfolio__organization_name"), - status__in=[ - DomainRequest.DomainRequestStatus.APPROVED, - DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.DomainRequestStatus.INELIGIBLE, - DomainRequest.DomainRequestStatus.STARTED, - ], - ) - def update_record(self, record: DomainRequest): """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" record.requested_suborganization = record.organization_name @@ -40,3 +29,22 @@ class Command(BaseCommand, PopulateScriptTemplate): f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." ) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) + + def should_skip_record(self, record) -> bool: + """Skips updating the record if the portfolio name is the same as the org name, + or if we are trying to update a record in an invalid status.""" + portfolio_normalized = normalize_string(record.portfolio.organization_name) + org_normalized = normalize_string(record.organization_name) + if portfolio_normalized == org_normalized: + return True + + invalid_statuses = [ + DomainRequest.DomainRequestStatus.APPROVED, + DomainRequest.DomainRequestStatus.REJECTED, + DomainRequest.DomainRequestStatus.INELIGIBLE, + DomainRequest.DomainRequestStatus.STARTED, + ] + if record.status in invalid_statuses: + return True + + return False diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 5e425f5a3..9ee39911d 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,3 +343,9 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value + + +def normalize_string(string_to_normalize: str) -> str: + """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + new_string = " ".join(string_to_normalize.split()) + return new_string.lower() diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 29b310fd7..1c92728d2 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1751,6 +1751,7 @@ class TestPopulateRequestedSuborg(MockEppLib): city="Washington", state_territory="DC", portfolio=self.portfolio, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) # Create a request that already has a suborganization @@ -1762,6 +1763,7 @@ class TestPopulateRequestedSuborg(MockEppLib): state_territory="NY", portfolio=self.portfolio, sub_organization=self.suborg, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) # Create a request where org name matches portfolio name @@ -1772,6 +1774,7 @@ class TestPopulateRequestedSuborg(MockEppLib): state_territory="MA", portfolio=self.portfolio, user=self.user, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) def tearDown(self): From d68ccdc1b1aa2104a97128f22af0aad2e5d89a83 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:02:33 -0700 Subject: [PATCH 27/77] Finish script --- .../commands/populate_requested_suborg.py | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 121eb497c..0273aba28 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -2,8 +2,6 @@ import logging from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper from registrar.models import DomainRequest -from django.db.models import F - from registrar.models.utility.generic_helper import normalize_string logger = logging.getLogger(__name__) @@ -15,7 +13,12 @@ class Command(BaseCommand, PopulateScriptTemplate): def handle(self, **kwargs): """Loops through each DomainRequest object and populates its last_status_update and first_submitted_date values""" - filter_conditions = {"portfolio__isnull": False, "sub_organization__isnull": True} + filter_conditions = { + "portfolio__isnull": False, + "organization_name__isnull": False, + "sub_organization__isnull": True, + "portfolio__organization_name__isnull": False, + } fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) @@ -29,15 +32,10 @@ class Command(BaseCommand, PopulateScriptTemplate): f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." ) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) - + def should_skip_record(self, record) -> bool: """Skips updating the record if the portfolio name is the same as the org name, or if we are trying to update a record in an invalid status.""" - portfolio_normalized = normalize_string(record.portfolio.organization_name) - org_normalized = normalize_string(record.organization_name) - if portfolio_normalized == org_normalized: - return True - invalid_statuses = [ DomainRequest.DomainRequestStatus.APPROVED, DomainRequest.DomainRequestStatus.REJECTED, @@ -47,4 +45,9 @@ class Command(BaseCommand, PopulateScriptTemplate): if record.status in invalid_statuses: return True + portfolio_normalized = normalize_string(record.portfolio.organization_name) + org_normalized = normalize_string(record.organization_name) + if portfolio_normalized == org_normalized: + return True + return False From feb319f3c2839c3d9a444e5324987aff618c0b0a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:13:53 -0700 Subject: [PATCH 28/77] Update populate_requested_suborg.py --- src/registrar/management/commands/populate_requested_suborg.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 0273aba28..cf76c1fdf 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -37,7 +37,6 @@ class Command(BaseCommand, PopulateScriptTemplate): """Skips updating the record if the portfolio name is the same as the org name, or if we are trying to update a record in an invalid status.""" invalid_statuses = [ - DomainRequest.DomainRequestStatus.APPROVED, DomainRequest.DomainRequestStatus.REJECTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.STARTED, From 3dc445d17da90c09251faa03adaaa05960f84edc Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 6 Jan 2025 14:32:16 -0800 Subject: [PATCH 29/77] Update test and add in documentation --- docs/developer/registry-access.md | 47 ++++++++++++++++++++++++ src/registrar/tests/test_views_domain.py | 5 ++- src/registrar/views/domain.py | 4 +- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md index c7737d5bc..c52810c35 100644 --- a/docs/developer/registry-access.md +++ b/docs/developer/registry-access.md @@ -103,3 +103,50 @@ response = registry._client.transport.receive() ``` This is helpful for debugging situations where epplib is not correctly or fully parsing the XML returned from the registry. + +### Adding in a expiring soon domain +The below scenario is if you are NOT in org model mode (`organization_feature` waffle flag is off). + +1. Go to the `staging` sandbox and to `/admin` +2. Go to Domains and find a domain that is actually expired by sorting the Expiration Date column +3. Click into the domain to check the expiration date +4. Click into Manage Domain to double check the expiration date as well +5. Now hold onto that domain name, and save it for the command below + +6. In a terminal, run these commands: +``` +cf ssh getgov- +/tmp/lifecycle/shell +./manage.py shell +from registrar.models import Domain, DomainInvitation +from registrar.models import User +user = User.objects.filter(first_name="") +domain = Domain.objects.get_or_create(name="") +``` + +7. Go back to `/admin` and create Domain Information for that domain you just added in via the terminal +8. Go to Domain to find it +9. Click Manage Domain +10. Add yourself as domain manager +11. Go to the Registrar page and you should now see the expiring domain + +If you want to be in the org model mode, turn the `organization_feature` waffle flag on, and add that domain via Django Admin to a portfolio to be able to view it. + + + + + + + + + + + + + + + + + + + diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index fa4351de1..e6e42d563 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -576,7 +576,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(edit_page, "Review the details below and update any required information") @override_flag("domain_renewal", active=True) - def test_domain_renewal_form_security_contact_edit(self): + def test_domain_renewal_form_security_email_edit(self): with less_console_noise(): # Start on the Renewal page for the domain renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) @@ -584,6 +584,9 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): # Verify we see "Security email" on the renewal form self.assertContains(renewal_page, "Security email") + # Verify we see "strong recommend" blurb + self.assertContains(renewal_page, "We strongly recommend that you provide a security email.") + # Verify that the "Edit" button for Security email is there and links to correct URL edit_button_url = reverse("domain-security-email", kwargs={"pk": self.domain_with_ip.id}) self.assertContains(renewal_page, f'href="{edit_button_url}"') diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index ab68addb7..7ce0d7e1a 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -380,9 +380,9 @@ class DomainRenewalView(DomainBaseView): if "submit_button" in request.POST: try: domain.renew_domain() - messages.success(request, "This domain has been renewed for one year") + messages.success(request, "This domain has been renewed for one year.") except Exception as e: - messages.error(request, "*** This domain has not been renewed") + messages.error(request, "This domain has not been renewed for one year, error was %s" % e) return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) From 5f9a398bb77fd5217c4f0c019a8c4420fd8327e2 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Tue, 7 Jan 2025 09:43:48 -0500 Subject: [PATCH 30/77] some test --- src/registrar/tests/test_views_domain.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 02d7aa9ac..4eed20d56 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -530,7 +530,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(detail_page, "Renew to maintain access") @override_flag("domain_renewal", active=True) - def test_domain_renewal_sidebar_and_form(self): + def test_domain_renewal_form_and_sidebar(self): self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( Domain, "is_expired", self.custom_is_expired @@ -608,8 +608,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): # Simulate clicking on edit button edit_page = renewal_page.click(href=edit_button_url, index=1) self.assertEqual(edit_page.status_code, 200) - self.assertContains(edit_page, "Domain managers can update all information related to a domain") - + self.assertContains(edit_page, "Domain managers can update all information related to a domain" class TestDomainManagers(TestDomainOverview): @classmethod From ca9a40caaf89bb050990ab4d696a62a7ace7cbca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 08:31:17 -0700 Subject: [PATCH 31/77] Add logic for requested suborgs --- .../management/commands/create_federal_portfolio.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 9cf4d36ea..f417154c7 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -232,6 +232,18 @@ class Command(BaseCommand): domain_request.portfolio = portfolio if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) + else: + clean_organization_name = None + if isinstance(domain_request.organization_name, str): + clean_organization_name = domain_request.organization_name.strip() + + clean_city = None + if isinstance(domain_request.city, str): + clean_city = domain_request.city.strip() + + domain_request.requested_suborganization = clean_organization_name + domain_request.suborganization_city = clean_city + domain_request.suborganization_state_territory = domain_request.state_territory self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) From 2c5c9de6dca75356db2d9eb3904bb97e06438818 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 08:42:56 -0700 Subject: [PATCH 32/77] Remove second script, modify first --- docs/operations/data_migration.md | 33 ----- .../commands/populate_requested_suborg.py | 52 -------- .../models/utility/generic_helper.py | 6 - .../tests/test_management_scripts.py | 121 ++++-------------- 4 files changed, 24 insertions(+), 188 deletions(-) delete mode 100644 src/registrar/management/commands/populate_requested_suborg.py diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 32de4d31a..ada3efa55 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -919,36 +919,3 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi - Parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, you must specify at least one to run this script. - -## Populate requested suborg -This script populates the "requested suborganization" fields on DomainRequest. -These fields are: requested_suborganization, suborganization_city, suborganization_state_territory. - -This is done by pulling from organization_name, city, and state_territory. -The script will only parse records with a portfolio but no suborganization attached to it. -Additionally, it will not parse records wherein the organization name matches the portfolio org name. - -### Running on sandboxes - -#### Step 1: Login to CloudFoundry -```cf login -a api.fr.cloud.gov --sso``` - -#### Step 2: SSH into your environment -```cf ssh getgov-{space}``` - -Example: `cf ssh getgov-za` - -#### Step 3: Create a shell instance -```/tmp/lifecycle/shell``` - -#### Step 4: Upload your csv to the desired sandbox -[Follow these steps](#use-scp-to-transfer-data-to-sandboxes) to upload the federal_cio csv to a sandbox of your choice. - -#### Step 5: Running the script -To create a specific portfolio: -```./manage.py populate_requested_suborg``` - -### Running locally - -#### Step 1: Running the script -```docker-compose exec app ./manage.py populate_requested_suborg``` diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py deleted file mode 100644 index cf76c1fdf..000000000 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ /dev/null @@ -1,52 +0,0 @@ -import logging -from django.core.management import BaseCommand -from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper -from registrar.models import DomainRequest -from registrar.models.utility.generic_helper import normalize_string - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand, PopulateScriptTemplate): - help = "Loops through each domain request object and populates requested suborg info" - - def handle(self, **kwargs): - """Loops through each DomainRequest object and populates - its last_status_update and first_submitted_date values""" - filter_conditions = { - "portfolio__isnull": False, - "organization_name__isnull": False, - "sub_organization__isnull": True, - "portfolio__organization_name__isnull": False, - } - fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] - self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) - - def update_record(self, record: DomainRequest): - """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" - record.requested_suborganization = record.organization_name - record.suborganization_city = record.city - record.suborganization_state_territory = record.state_territory - message = ( - f"Updating {record} => requested_suborg: {record.organization_name}, " - f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." - ) - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) - - def should_skip_record(self, record) -> bool: - """Skips updating the record if the portfolio name is the same as the org name, - or if we are trying to update a record in an invalid status.""" - invalid_statuses = [ - DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.DomainRequestStatus.INELIGIBLE, - DomainRequest.DomainRequestStatus.STARTED, - ] - if record.status in invalid_statuses: - return True - - portfolio_normalized = normalize_string(record.portfolio.organization_name) - org_normalized = normalize_string(record.organization_name) - if portfolio_normalized == org_normalized: - return True - - return False diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 9ee39911d..5e425f5a3 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,9 +343,3 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value - - -def normalize_string(string_to_normalize: str) -> str: - """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" - new_string = " ".join(string_to_normalize.split()) - return new_string.lower() diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 1c92728d2..d391cb34d 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1588,6 +1588,30 @@ class TestCreateFederalPortfolio(TestCase): self.assertTrue(all([creator == User.get_default_user() for creator in creators])) self.assertTrue(all([note == "Auto-generated record" for note in notes])) + def test_script_adds_requested_suborganization_information(self): + """Tests that the script adds the requested suborg fields for domain requests""" + custom_suborg_request = completed_domain_request( + name="custom_org.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.executive_agency_2, + user=self.user, + organization_name="requested org name", + city="Austin", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS + ) + + self.assertIsNone(custom_suborg_request.requested_suborganization) + self.assertIsNone(custom_suborg_request.suborganization_city) + self.assertIsNone(custom_suborg_request.suborganization_state_territory) + + # Run the script and test it + self.run_create_federal_portfolio(branch="executive", parse_requests=True) + + self.assertEqual(custom_suborg_request.requested_suborganization, "requested org name") + self.assertEqual(custom_suborg_request.suborganization_city, "Austin") + self.assertEqual(custom_suborg_request.suborganization_state_territory, DomainRequest.StateTerritoryChoices.TEXAS) + def test_create_multiple_portfolios_for_branch_executive(self): """Tests creating all portfolios under a given branch""" federal_choice = DomainRequest.OrganizationChoices.FEDERAL @@ -1732,100 +1756,3 @@ class TestCreateFederalPortfolio(TestCase): self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) - -class TestPopulateRequestedSuborg(MockEppLib): - """Tests for the populate_requested_suborg script""" - - @less_console_noise_decorator - def setUp(self): - super().setUp() - - self.user = create_user() - # Create a portfolio - self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.user) - - # Create a domain request with all required fields - self.complete_request = completed_domain_request( - name="complete.gov", - organization_name="Complete Org", - city="Washington", - state_territory="DC", - portfolio=self.portfolio, - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - ) - - # Create a request that already has a suborganization - self.suborg = Suborganization.objects.create(name="Existing Suborg", portfolio=self.portfolio) - self.existing_suborg_request = completed_domain_request( - name="existing.gov", - organization_name="Existing Org", - city="New York", - state_territory="NY", - portfolio=self.portfolio, - sub_organization=self.suborg, - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - ) - - # Create a request where org name matches portfolio name - self.matching_name_request = completed_domain_request( - name="matching.gov", - organization_name="Test Portfolio", - city="Boston", - state_territory="MA", - portfolio=self.portfolio, - user=self.user, - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - ) - - def tearDown(self): - super().tearDown() - DomainRequest.objects.all().delete() - Suborganization.objects.all().delete() - Portfolio.objects.all().delete() - User.objects.all().delete() - - @less_console_noise_decorator - def run_populate_requested_suborg(self): - """Executes the populate_requested_suborg command""" - with patch( - "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", - return_value=True, - ): - call_command("populate_requested_suborg") - - @less_console_noise_decorator - def test_populate_requested_suborg_complete_request(self): - """Tests that complete requests are updated correctly""" - self.run_populate_requested_suborg() - - self.complete_request.refresh_from_db() - - # Verify fields were populated correctly - self.assertEqual(self.complete_request.requested_suborganization, "Complete Org") - self.assertEqual(self.complete_request.suborganization_city, "Washington") - self.assertEqual(self.complete_request.suborganization_state_territory, "DC") - - @less_console_noise_decorator - def test_populate_requested_suborg_existing_suborg(self): - """Tests that requests with existing suborgs are skipped""" - self.run_populate_requested_suborg() - - self.existing_suborg_request.refresh_from_db() - - # Verify the request wasn't modified - self.assertEqual(self.existing_suborg_request.sub_organization, self.suborg) - self.assertIsNone(self.existing_suborg_request.requested_suborganization) - self.assertIsNone(self.existing_suborg_request.suborganization_city) - self.assertIsNone(self.existing_suborg_request.suborganization_state_territory) - - @less_console_noise_decorator - def test_populate_requested_suborg_matching_name(self): - """Tests that requests with matching org/portfolio names are skipped""" - self.run_populate_requested_suborg() - - self.matching_name_request.refresh_from_db() - - # Verify the request wasn't modified since org name matches portfolio name - self.assertIsNone(self.matching_name_request.requested_suborganization) - self.assertIsNone(self.matching_name_request.suborganization_city) - self.assertIsNone(self.matching_name_request.suborganization_state_territory) From 7e274920fe6481610f44b2cdd056dce1680c0588 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 08:45:05 -0700 Subject: [PATCH 33/77] Remove errant spaces --- docs/operations/data_migration.md | 1 - src/registrar/tests/test_management_scripts.py | 1 - 2 files changed, 2 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index ada3efa55..0863aa0b7 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -918,4 +918,3 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi - Parameters #1-#2: Either `--agency_name` or `--branch` must be specified. Not both. - Parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, you must specify at least one to run this script. - diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index d391cb34d..d4b9589cd 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1755,4 +1755,3 @@ class TestCreateFederalPortfolio(TestCase): self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) - From d2804a64f5e5a3db469b280601d0a3f965e5ceed Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 08:59:44 -0700 Subject: [PATCH 34/77] Add bulk update --- .../management/commands/create_federal_portfolio.py | 11 ++++++++++- src/registrar/tests/test_management_scripts.py | 12 ++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index f417154c7..42f4f5687 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -246,7 +246,16 @@ class Command(BaseCommand): domain_request.suborganization_state_territory = domain_request.state_territory self.updated_portfolios.add(portfolio) - DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) + DomainRequest.objects.bulk_update( + domain_requests, + [ + "portfolio", + "sub_organization", + "requested_suborganization", + "suborganization_city", + "suborganization_state_territory", + ], + ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index d4b9589cd..4ca2a2d72 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1590,15 +1590,16 @@ class TestCreateFederalPortfolio(TestCase): def test_script_adds_requested_suborganization_information(self): """Tests that the script adds the requested suborg fields for domain requests""" + # Create a new domain request with some errant spacing custom_suborg_request = completed_domain_request( name="custom_org.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW, generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, federal_agency=self.executive_agency_2, user=self.user, - organization_name="requested org name", - city="Austin", - state_territory=DomainRequest.StateTerritoryChoices.TEXAS + organization_name=" requested org name ", + city="Austin ", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, ) self.assertIsNone(custom_suborg_request.requested_suborganization) @@ -1607,10 +1608,13 @@ class TestCreateFederalPortfolio(TestCase): # Run the script and test it self.run_create_federal_portfolio(branch="executive", parse_requests=True) + custom_suborg_request.refresh_from_db() self.assertEqual(custom_suborg_request.requested_suborganization, "requested org name") self.assertEqual(custom_suborg_request.suborganization_city, "Austin") - self.assertEqual(custom_suborg_request.suborganization_state_territory, DomainRequest.StateTerritoryChoices.TEXAS) + self.assertEqual( + custom_suborg_request.suborganization_state_territory, DomainRequest.StateTerritoryChoices.TEXAS + ) def test_create_multiple_portfolios_for_branch_executive(self): """Tests creating all portfolios under a given branch""" From d06df851e6ff0253b2b6db15aa28cca2f3905e97 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 09:11:35 -0700 Subject: [PATCH 35/77] Update test_management_scripts.py --- src/registrar/tests/test_management_scripts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 4ca2a2d72..5c3ab4ba3 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -32,7 +32,7 @@ import tablib from unittest.mock import patch, call, MagicMock, mock_open from epplibwrapper import commands, common -from .common import MockEppLib, create_user, less_console_noise, completed_domain_request, MockSESClient +from .common import MockEppLib, less_console_noise, completed_domain_request, MockSESClient from api.tests.common import less_console_noise_decorator From 3a356c6055f8c6eef1fca306e6572ada0f90a195 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 12:32:56 -0700 Subject: [PATCH 36/77] Normalize name and clear fed agency in some cases --- .../commands/create_federal_portfolio.py | 36 +++++++++++++--- src/registrar/models/domain_request.py | 19 +++++++- .../models/utility/generic_helper.py | 5 +++ .../tests/test_management_scripts.py | 43 +++++++++++++++++++ src/registrar/tests/test_models_requests.py | 32 ++++++++++++++ 5 files changed, 127 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 42f4f5687..83bb3dd0f 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -5,6 +5,7 @@ import logging from django.core.management import BaseCommand, CommandError from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User +from registrar.models.utility.generic_helper import normalize_string logger = logging.getLogger(__name__) @@ -51,6 +52,11 @@ class Command(BaseCommand): action=argparse.BooleanOptionalAction, help="Adds portfolio to both requests and domains", ) + parser.add_argument( + "--include_started_requests", + action=argparse.BooleanOptionalAction, + help="If parse_requests is enabled, we parse started", + ) def handle(self, **options): agency_name = options.get("agency_name") @@ -58,6 +64,7 @@ class Command(BaseCommand): parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") both = options.get("both") + include_started_requests = options.get("include_started_requests") if not both: if not parse_requests and not parse_domains: @@ -66,6 +73,9 @@ class Command(BaseCommand): if parse_requests or parse_domains: raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") + if include_started_requests and not parse_requests: + raise CommandError("You must pass --parse_requests when using --include_started_requests") + federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} agencies = FederalAgency.objects.filter(**federal_agency_filter) if not agencies or agencies.count() < 1: @@ -83,7 +93,9 @@ class Command(BaseCommand): TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + self.handle_populate_portfolio( + federal_agency, parse_domains, parse_requests, both, include_started_requests + ) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -99,7 +111,7 @@ class Command(BaseCommand): display_as_str=True, ) - def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): + def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both, include_started_requests): """Attempts to create a portfolio. If successful, this function will also create new suborganizations""" portfolio, created = self.create_portfolio(federal_agency) @@ -109,7 +121,7 @@ class Command(BaseCommand): self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency) + self.handle_portfolio_requests(portfolio, federal_agency, include_started_requests) def create_portfolio(self, federal_agency): """Creates a portfolio if it doesn't presently exist. @@ -182,7 +194,7 @@ class Command(BaseCommand): for name in org_names - set(existing_suborgs.values_list("name", flat=True)): # Stored in variables due to linter wanting type information here. portfolio_name: str = portfolio.organization_name if portfolio.organization_name is not None else "" - if name is not None and name.lower() == portfolio_name.lower(): + if name is not None and normalize_string(name) == normalize_string(portfolio_name): # You can use this to populate location information, when this occurs. # However, this isn't needed for now so we can skip it. message = ( @@ -191,7 +203,7 @@ class Command(BaseCommand): ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) # type: ignore + new_suborgs.append(Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio)) # type: ignore if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) @@ -201,16 +213,18 @@ class Command(BaseCommand): else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): + def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency, include_started_requests): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. """ invalid_states = [ - DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] + if not include_started_requests: + invalid_states.append(DomainRequest.DomainRequestStatus.STARTED) + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( status__in=invalid_states ) @@ -229,7 +243,14 @@ class Command(BaseCommand): # Get all suborg information and store it in a dict to avoid doing a db call suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_request in domain_requests: + # Set the portfolio domain_request.portfolio = portfolio + + # Conditionally clear federal agency if the org name is the same as the portfolio name. + if include_started_requests: + domain_request.sync_portfolio_and_federal_agency_for_started_requests() + + # Set suborg info if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) else: @@ -254,6 +275,7 @@ class Command(BaseCommand): "requested_suborganization", "suborganization_city", "suborganization_state_territory", + "federal_agency", ], ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 3d3aac769..617c96737 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -8,7 +8,7 @@ from django_fsm import FSMField, transition # type: ignore from django.utils import timezone from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, normalize_string from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices from auditlog.models import LogEntry @@ -729,6 +729,7 @@ class DomainRequest(TimeStampedModel): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() + self.sync_portfolio_and_federal_agency_for_started_requests() if self._cached_status != self.status: self.last_status_update = timezone.now().date() @@ -744,6 +745,22 @@ class DomainRequest(TimeStampedModel): # Update the cached values after saving self._cache_status_and_status_reasons() + def sync_portfolio_and_federal_agency_for_started_requests(self) -> bool: + """ + Prevents duplicate organization data by clearing the federal_agency field if it matches the portfolio. + Only runs on STARTED requests. + + Returns a bool indicating if the federal agency was changed or not. + """ + agency_name = getattr(self.federal_agency, "agency", None) + portfolio_name = getattr(self.portfolio, "organization_name", None) + if portfolio_name and agency_name and self.status == DomainRequest.DomainRequestStatus.STARTED: + if normalize_string(agency_name) == normalize_string(portfolio_name): + self.federal_agency = None + return True + + return False + def create_requested_suborganization(self): """Creates the requested suborganization. Adds the name, portfolio, city, and state_territory fields. diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 5e425f5a3..7aa4e62bc 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,3 +343,8 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value + +def normalize_string(string_to_normalize: str, lowercase=True) -> str: + """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + new_string = " ".join(string_to_normalize.split()) + return new_string.lower() if lowercase else new_string diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 5c3ab4ba3..418a22411 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1424,6 +1424,7 @@ class TestCreateFederalPortfolio(TestCase): # Create an agency wih no federal type (can only be created via specifiying it manually) self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") + self.federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane") # And create some with federal_type ones with creative names self.executive_agency_1 = FederalAgency.objects.create( @@ -1516,6 +1517,48 @@ class TestCreateFederalPortfolio(TestCase): ): call_command("create_federal_portfolio", **kwargs) + @less_console_noise_decorator + def test_handle_portfolio_requests_sync_federal_agency(self): + """Test that federal agency is cleared when org name matches portfolio name""" + + # Create a domain request with matching org name + matching_request = completed_domain_request( + name="matching.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency_2, + user=self.user, + ) + + # Create a request not in started (no change should occur) + matching_request_in_wrong_status = completed_domain_request( + name="kinda-matching.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + ) + + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True, include_started_requests=True) + self.run_create_federal_portfolio( + agency_name="Test Federal Agency", parse_requests=True, include_started_requests=True + ) + + # Refresh from db + matching_request.refresh_from_db() + matching_request_in_wrong_status.refresh_from_db() + + # Request with matching name should have federal_agency cleared + self.assertIsNone(matching_request.federal_agency) + self.assertIsNotNone(matching_request.portfolio) + self.assertEqual(matching_request.portfolio.organization_name, "Sugarcane") + + # Request with matching name but wrong state should keep its federal agency + self.assertEqual(matching_request_in_wrong_status.federal_agency, self.federal_agency) + self.assertIsNotNone(matching_request_in_wrong_status.portfolio) + self.assertEqual(matching_request_in_wrong_status.portfolio.organization_name, "Test Federal Agency") + + @less_console_noise_decorator def test_create_single_portfolio(self): """Test portfolio creation with suborg and senior official.""" self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index 983a12b3c..0c674d417 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -1074,6 +1074,38 @@ class TestDomainRequest(TestCase): self.assertEqual(domain_request2.generic_org_type, domain_request2.converted_generic_org_type) self.assertEqual(domain_request2.federal_agency, domain_request2.converted_federal_agency) + def test_sync_portfolio_and_federal_agency_for_started_requests(self): + """Tests the sync_portfolio_and_federal_agency_for_started_requests function""" + # Create a federal agency with a "bad" name + user = create_user() + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane ") + request = completed_domain_request( + name="matching.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=federal_agency_2, + user=user, + ) + self.assertIsNone(request.portfolio) + + # Nothing should happen on normal save + request.notes = "test change" + request.save() + self.assertEqual(request.federal_agency, federal_agency_2) + + # But when a portfolio exists, the federal agency should be cleared if its a duplicate + portfolio = Portfolio.objects.create(organization_name="sugarcane", creator=user) + request.portfolio = portfolio + request.save() + self.assertIsNone(request.federal_agency) + + # However -- this change should only occur if the names match (when normalized) + portfolio = Portfolio.objects.create(organization_name="some other name", creator=user) + request.portfolio = portfolio + request.federal_agency = federal_agency_2 + request.save() + self.assertEqual(request.federal_agency, federal_agency_2) + class TestDomainRequestSuborganization(TestCase): """Tests for the suborganization fields on domain requests""" From 2d163c1c1cac8f545b40c63c77760663fe40399a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 12:41:52 -0700 Subject: [PATCH 37/77] Lint --- .../commands/create_federal_portfolio.py | 17 ++++++++++------- src/registrar/models/utility/generic_helper.py | 1 + src/registrar/tests/test_management_scripts.py | 5 +++++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 83bb3dd0f..1674d9057 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -203,7 +203,9 @@ class Command(BaseCommand): ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append(Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio)) # type: ignore + new_suborgs.append( + Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio) + ) # type: ignore if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) @@ -246,25 +248,26 @@ class Command(BaseCommand): # Set the portfolio domain_request.portfolio = portfolio - # Conditionally clear federal agency if the org name is the same as the portfolio name. - if include_started_requests: - domain_request.sync_portfolio_and_federal_agency_for_started_requests() - # Set suborg info if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) else: clean_organization_name = None if isinstance(domain_request.organization_name, str): - clean_organization_name = domain_request.organization_name.strip() + clean_organization_name = normalize_string(domain_request.organization_name, lowercase=False) clean_city = None if isinstance(domain_request.city, str): - clean_city = domain_request.city.strip() + clean_city = normalize_string(domain_request.city, lowercase=False) domain_request.requested_suborganization = clean_organization_name domain_request.suborganization_city = clean_city domain_request.suborganization_state_territory = domain_request.state_territory + + # Conditionally clear federal agency if the org name is the same as the portfolio name. + if include_started_requests: + domain_request.sync_portfolio_and_federal_agency_for_started_requests() + self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update( diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 7aa4e62bc..7d384d577 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -344,6 +344,7 @@ def value_of_attribute(obj, attribute_name: str): value = value() return value + def normalize_string(string_to_normalize: str, lowercase=True) -> str: """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" new_string = " ".join(string_to_normalize.split()) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 418a22411..1c6cdd123 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1770,6 +1770,11 @@ class TestCreateFederalPortfolio(TestCase): ): self.run_create_federal_portfolio(agency_name="test") + with self.assertRaisesRegex( + CommandError, "You must pass --parse_requests when using --include_started_requests" + ): + self.run_create_federal_portfolio(agency_name="test", include_started_requests=True) + def test_command_error_agency_not_found(self): """Check error handling for non-existent agency.""" expected_message = ( From 2f5e0ec873f087e59a0329604a1b0f356f4bfb47 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 13:01:15 -0700 Subject: [PATCH 38/77] Code cleanup --- .../management/commands/create_federal_portfolio.py | 11 ++++------- src/registrar/tests/test_management_scripts.py | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 1674d9057..5ad350d02 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -184,7 +184,7 @@ class Command(BaseCommand): return # Check for existing suborgs on the current portfolio - existing_suborgs = Suborganization.objects.filter(name__in=org_names) + existing_suborgs = Suborganization.objects.filter(name__in=org_names, name__isnull=False) if existing_suborgs.exists(): message = f"Some suborganizations already exist for portfolio '{portfolio}'." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) @@ -192,9 +192,7 @@ class Command(BaseCommand): # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] for name in org_names - set(existing_suborgs.values_list("name", flat=True)): - # Stored in variables due to linter wanting type information here. - portfolio_name: str = portfolio.organization_name if portfolio.organization_name is not None else "" - if name is not None and normalize_string(name) == normalize_string(portfolio_name): + if normalize_string(name) == normalize_string(portfolio.organization_name): # You can use this to populate location information, when this occurs. # However, this isn't needed for now so we can skip it. message = ( @@ -249,9 +247,8 @@ class Command(BaseCommand): domain_request.portfolio = portfolio # Set suborg info - if domain_request.organization_name in suborgs: - domain_request.sub_organization = suborgs.get(domain_request.organization_name) - else: + domain_request.sub_organization = suborgs.get(domain_request.organization_name, None) + if domain_request.sub_organization is None: clean_organization_name = None if isinstance(domain_request.organization_name, str): clean_organization_name = normalize_string(domain_request.organization_name, lowercase=False) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 1c6cdd123..13746b624 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1773,7 +1773,7 @@ class TestCreateFederalPortfolio(TestCase): with self.assertRaisesRegex( CommandError, "You must pass --parse_requests when using --include_started_requests" ): - self.run_create_federal_portfolio(agency_name="test", include_started_requests=True) + self.run_create_federal_portfolio(agency_name="test", branch="executive", include_started_requests=True) def test_command_error_agency_not_found(self): """Check error handling for non-existent agency.""" From a8808e5356ccb5c5cf54b7f9f85616de6323a9ba Mon Sep 17 00:00:00 2001 From: asaki222 Date: Tue, 7 Jan 2025 16:11:33 -0500 Subject: [PATCH 39/77] ran app black --- src/registrar/tests/test_views_domain.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 25f79d7e0..ee8adf903 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -464,7 +464,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): def custom_is_expiring(self): return True - + def custom_renew_domain(self): self.domain_with_ip.expiration_date = self.todays_expiration_date() self.domain_with_ip.save() @@ -644,7 +644,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): str(messages[0]), "Check the box if you read and agree to the requirements for operating a .gov domain.", ) - + @override_flag("domain_renewal", active=True) def test_ack_checkbox_checked(self): @@ -652,19 +652,18 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): with patch.object(Domain, "renew_domain", self.custom_renew_domain): renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) - # Click the check, and submit + # Click the check, and submit response = self.client.post(renewal_url, data={"is_policy_acknowledged": "on", "submit_button": "next"}) - #Check that it redirects after a successfully submits - self.assertRedirects(response, reverse("domain", kwargs={"pk":self.domain_with_ip.id})) + # Check that it redirects after a successfully submits + self.assertRedirects(response, reverse("domain", kwargs={"pk": self.domain_with_ip.id})) - #Check for the updated expiration + # Check for the updated expiration formatted_new_expiration_date = self.todays_expiration_date().strftime("%b. %-d, %Y") - redirect_response = self.client.get(reverse("domain", kwargs={"pk":self.domain_with_ip.id}), follow=True) + redirect_response = self.client.get(reverse("domain", kwargs={"pk": self.domain_with_ip.id}), follow=True) self.assertContains(redirect_response, formatted_new_expiration_date) - class TestDomainManagers(TestDomainOverview): @classmethod def setUpClass(cls): From 9e49ca4eacc94fd77d6b689dcca693994034d5ca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:57:26 -0700 Subject: [PATCH 40/77] Simplify logic --- .../commands/create_federal_portfolio.py | 70 ++++++++++++------- src/registrar/models/domain_request.py | 17 ----- .../tests/test_management_scripts.py | 16 ++--- src/registrar/tests/test_models_requests.py | 32 --------- 4 files changed, 54 insertions(+), 81 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 5ad350d02..c3d3bdd42 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -52,11 +52,6 @@ class Command(BaseCommand): action=argparse.BooleanOptionalAction, help="Adds portfolio to both requests and domains", ) - parser.add_argument( - "--include_started_requests", - action=argparse.BooleanOptionalAction, - help="If parse_requests is enabled, we parse started", - ) def handle(self, **options): agency_name = options.get("agency_name") @@ -64,7 +59,6 @@ class Command(BaseCommand): parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") both = options.get("both") - include_started_requests = options.get("include_started_requests") if not both: if not parse_requests and not parse_domains: @@ -73,9 +67,6 @@ class Command(BaseCommand): if parse_requests or parse_domains: raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") - if include_started_requests and not parse_requests: - raise CommandError("You must pass --parse_requests when using --include_started_requests") - federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} agencies = FederalAgency.objects.filter(**federal_agency_filter) if not agencies or agencies.count() < 1: @@ -88,14 +79,15 @@ class Command(BaseCommand): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") + portfolio_set = set() for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - self.handle_populate_portfolio( - federal_agency, parse_domains, parse_requests, both, include_started_requests - ) + # We currently only grab the list of changed domain requests, but we may want to grab the domains too + portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + portfolio_set.add(portfolio) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -111,9 +103,39 @@ class Command(BaseCommand): display_as_str=True, ) - def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both, include_started_requests): + # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. + # We only do this for started domain requests. + if parse_requests: + message = "Removing duplicate portfolio and federal_agency values from domain requests..." + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + + domain_requests_to_update = DomainRequest.objects.filter( + portfolio__in=portfolio_set, + status=DomainRequest.DomainRequestStatus.STARTED, + federal_agency__agency__isnull=False, + portfolio__organization_name__isnull=False, + ) + updated_requests = [] + for req in domain_requests_to_update: + if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): + req.federal_agency = None + updated_requests.append(req) + DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) + + # Log the results + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + prompt_message=f"Updated {updated_requests} domain requests successfully.", + prompt_title="Do you want to see a list of all changed domain requests?", + ): + logger.info(f"Federal agency set to none on: {[str(request) for request in updated_requests]}") + + def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will - also create new suborganizations""" + also create new suborganizations. + + Returns the processed portfolio + """ portfolio, created = self.create_portfolio(federal_agency) if created: self.create_suborganizations(portfolio, federal_agency) @@ -121,7 +143,9 @@ class Command(BaseCommand): self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency, include_started_requests) + self.handle_portfolio_requests(portfolio, federal_agency) + + return portfolio def create_portfolio(self, federal_agency): """Creates a portfolio if it doesn't presently exist. @@ -213,17 +237,19 @@ class Command(BaseCommand): else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency, include_started_requests): + def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. + Returns a list of updated records. + + Returns a queryset of DomainRequest objects, or None if nothing changed. """ invalid_states = [ + DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - if not include_started_requests: - invalid_states.append(DomainRequest.DomainRequestStatus.STARTED) domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( status__in=invalid_states @@ -260,11 +286,6 @@ class Command(BaseCommand): domain_request.requested_suborganization = clean_organization_name domain_request.suborganization_city = clean_city domain_request.suborganization_state_territory = domain_request.state_territory - - # Conditionally clear federal agency if the org name is the same as the portfolio name. - if include_started_requests: - domain_request.sync_portfolio_and_federal_agency_for_started_requests() - self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update( @@ -275,7 +296,6 @@ class Command(BaseCommand): "requested_suborganization", "suborganization_city", "suborganization_state_territory", - "federal_agency", ], ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." @@ -285,6 +305,8 @@ class Command(BaseCommand): """ Associate portfolio with domains for a federal agency. Updates all relevant domain information records. + + Returns a queryset of DomainInformation objects, or None if nothing changed. """ domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True) if not domain_infos.exists(): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 617c96737..d1a2597f4 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -729,7 +729,6 @@ class DomainRequest(TimeStampedModel): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() - self.sync_portfolio_and_federal_agency_for_started_requests() if self._cached_status != self.status: self.last_status_update = timezone.now().date() @@ -745,22 +744,6 @@ class DomainRequest(TimeStampedModel): # Update the cached values after saving self._cache_status_and_status_reasons() - def sync_portfolio_and_federal_agency_for_started_requests(self) -> bool: - """ - Prevents duplicate organization data by clearing the federal_agency field if it matches the portfolio. - Only runs on STARTED requests. - - Returns a bool indicating if the federal agency was changed or not. - """ - agency_name = getattr(self.federal_agency, "agency", None) - portfolio_name = getattr(self.portfolio, "organization_name", None) - if portfolio_name and agency_name and self.status == DomainRequest.DomainRequestStatus.STARTED: - if normalize_string(agency_name) == normalize_string(portfolio_name): - self.federal_agency = None - return True - - return False - def create_requested_suborganization(self): """Creates the requested suborganization. Adds the name, portfolio, city, and state_territory fields. diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 13746b624..06ecf7a5f 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1520,7 +1520,11 @@ class TestCreateFederalPortfolio(TestCase): @less_console_noise_decorator def test_handle_portfolio_requests_sync_federal_agency(self): """Test that federal agency is cleared when org name matches portfolio name""" - + # Create a portfolio. This script skips over "started" + portfolio = Portfolio.objects.create( + organization_name="Sugarcane", + creator=self.user + ) # Create a domain request with matching org name matching_request = completed_domain_request( name="matching.gov", @@ -1528,6 +1532,7 @@ class TestCreateFederalPortfolio(TestCase): generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, federal_agency=self.federal_agency_2, user=self.user, + portfolio=portfolio ) # Create a request not in started (no change should occur) @@ -1539,9 +1544,9 @@ class TestCreateFederalPortfolio(TestCase): user=self.user, ) - self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True, include_started_requests=True) + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) self.run_create_federal_portfolio( - agency_name="Test Federal Agency", parse_requests=True, include_started_requests=True + agency_name="Test Federal Agency", parse_requests=True ) # Refresh from db @@ -1770,11 +1775,6 @@ class TestCreateFederalPortfolio(TestCase): ): self.run_create_federal_portfolio(agency_name="test") - with self.assertRaisesRegex( - CommandError, "You must pass --parse_requests when using --include_started_requests" - ): - self.run_create_federal_portfolio(agency_name="test", branch="executive", include_started_requests=True) - def test_command_error_agency_not_found(self): """Check error handling for non-existent agency.""" expected_message = ( diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index 0c674d417..983a12b3c 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -1074,38 +1074,6 @@ class TestDomainRequest(TestCase): self.assertEqual(domain_request2.generic_org_type, domain_request2.converted_generic_org_type) self.assertEqual(domain_request2.federal_agency, domain_request2.converted_federal_agency) - def test_sync_portfolio_and_federal_agency_for_started_requests(self): - """Tests the sync_portfolio_and_federal_agency_for_started_requests function""" - # Create a federal agency with a "bad" name - user = create_user() - federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane ") - request = completed_domain_request( - name="matching.gov", - status=DomainRequest.DomainRequestStatus.STARTED, - generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, - federal_agency=federal_agency_2, - user=user, - ) - self.assertIsNone(request.portfolio) - - # Nothing should happen on normal save - request.notes = "test change" - request.save() - self.assertEqual(request.federal_agency, federal_agency_2) - - # But when a portfolio exists, the federal agency should be cleared if its a duplicate - portfolio = Portfolio.objects.create(organization_name="sugarcane", creator=user) - request.portfolio = portfolio - request.save() - self.assertIsNone(request.federal_agency) - - # However -- this change should only occur if the names match (when normalized) - portfolio = Portfolio.objects.create(organization_name="some other name", creator=user) - request.portfolio = portfolio - request.federal_agency = federal_agency_2 - request.save() - self.assertEqual(request.federal_agency, federal_agency_2) - class TestDomainRequestSuborganization(TestCase): """Tests for the suborganization fields on domain requests""" From 8f3f7f14f2916511bd6ce531f855ef4153e0d858 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 15:12:25 -0700 Subject: [PATCH 41/77] Cleanup part 2 --- .../commands/create_federal_portfolio.py | 18 ++++++------------ src/registrar/models/domain_request.py | 2 +- src/registrar/models/utility/generic_helper.py | 4 ++++ src/registrar/tests/test_management_scripts.py | 11 +++-------- 4 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index c3d3bdd42..1bf4a3fca 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -275,17 +275,12 @@ class Command(BaseCommand): # Set suborg info domain_request.sub_organization = suborgs.get(domain_request.organization_name, None) if domain_request.sub_organization is None: - clean_organization_name = None - if isinstance(domain_request.organization_name, str): - clean_organization_name = normalize_string(domain_request.organization_name, lowercase=False) - - clean_city = None - if isinstance(domain_request.city, str): - clean_city = normalize_string(domain_request.city, lowercase=False) - - domain_request.requested_suborganization = clean_organization_name - domain_request.suborganization_city = clean_city + domain_request.requested_suborganization = normalize_string( + domain_request.organization_name, lowercase=False + ) + domain_request.suborganization_city = normalize_string(domain_request.city, lowercase=False) domain_request.suborganization_state_territory = domain_request.state_territory + self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update( @@ -322,8 +317,7 @@ class Command(BaseCommand): suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_info in domain_infos: domain_info.portfolio = portfolio - if domain_info.organization_name in suborgs: - domain_info.sub_organization = suborgs.get(domain_info.organization_name) + domain_info.sub_organization = suborgs.get(domain_info.organization_name, None) DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index d1a2597f4..3d3aac769 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -8,7 +8,7 @@ from django_fsm import FSMField, transition # type: ignore from django.utils import timezone from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, normalize_string +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices from auditlog.models import LogEntry diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 7d384d577..3a8da508e 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -347,5 +347,9 @@ def value_of_attribute(obj, attribute_name: str): def normalize_string(string_to_normalize: str, lowercase=True) -> str: """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + if not isinstance(string_to_normalize, str): + logger.error(f"normalize_string => {string_to_normalize} is not type str.") + return string_to_normalize + new_string = " ".join(string_to_normalize.split()) return new_string.lower() if lowercase else new_string diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 06ecf7a5f..882396f1e 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1521,10 +1521,7 @@ class TestCreateFederalPortfolio(TestCase): def test_handle_portfolio_requests_sync_federal_agency(self): """Test that federal agency is cleared when org name matches portfolio name""" # Create a portfolio. This script skips over "started" - portfolio = Portfolio.objects.create( - organization_name="Sugarcane", - creator=self.user - ) + portfolio = Portfolio.objects.create(organization_name="Sugarcane", creator=self.user) # Create a domain request with matching org name matching_request = completed_domain_request( name="matching.gov", @@ -1532,7 +1529,7 @@ class TestCreateFederalPortfolio(TestCase): generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, federal_agency=self.federal_agency_2, user=self.user, - portfolio=portfolio + portfolio=portfolio, ) # Create a request not in started (no change should occur) @@ -1545,9 +1542,7 @@ class TestCreateFederalPortfolio(TestCase): ) self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) - self.run_create_federal_portfolio( - agency_name="Test Federal Agency", parse_requests=True - ) + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) # Refresh from db matching_request.refresh_from_db() From d9cad13f953b581d525c7f2204ec9245ee3a0f2d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 15:55:27 -0700 Subject: [PATCH 42/77] Update create_federal_portfolio.py --- .../commands/create_federal_portfolio.py | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 1bf4a3fca..8ff32824b 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -106,29 +106,36 @@ class Command(BaseCommand): # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. if parse_requests: - message = "Removing duplicate portfolio and federal_agency values from domain requests..." - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + self.post_process_started_domain_requests(portfolio_set) - domain_requests_to_update = DomainRequest.objects.filter( - portfolio__in=portfolio_set, - status=DomainRequest.DomainRequestStatus.STARTED, - federal_agency__agency__isnull=False, - portfolio__organization_name__isnull=False, - ) - updated_requests = [] - for req in domain_requests_to_update: - if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): - req.federal_agency = None - updated_requests.append(req) - DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) + def post_process_started_domain_requests(self, portfolio_set): + """ + Removes duplicate organization data by clearing federal_agency when it matches the portfolio name. + Only processes domain requests in STARTED status. + """ + message = "Removing duplicate portfolio and federal_agency values from domain requests..." + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - # Log the results - if TerminalHelper.prompt_for_execution( - system_exit_on_terminate=False, - prompt_message=f"Updated {updated_requests} domain requests successfully.", - prompt_title="Do you want to see a list of all changed domain requests?", - ): - logger.info(f"Federal agency set to none on: {[str(request) for request in updated_requests]}") + domain_requests_to_update = DomainRequest.objects.filter( + portfolio__in=portfolio_set, + status=DomainRequest.DomainRequestStatus.STARTED, + federal_agency__agency__isnull=False, + portfolio__organization_name__isnull=False, + ) + updated_requests = [] + for req in domain_requests_to_update: + if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): + req.federal_agency = None + updated_requests.append(req) + DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) + + # Log the results + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + prompt_message=f"Updated {len(updated_requests)} domain requests successfully.", + prompt_title="Do you want to see a list of all changed domain requests?", + ): + logger.info(f"Federal agency set to none on: {[str(request) for request in updated_requests]}") def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will From b077f814094a98353b7e925a2b34d0ffd8c97376 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 7 Jan 2025 17:24:13 -0800 Subject: [PATCH 43/77] Address feedback - use form pattern and update error messaging --- docs/developer/registry-access.md | 21 +---- src/registrar/forms/__init__.py | 1 + src/registrar/forms/domain.py | 12 +++ src/registrar/models/domain.py | 5 +- src/registrar/templates/domain_renewal.html | 94 ++++++++++----------- src/registrar/tests/test_views_domain.py | 6 +- src/registrar/views/domain.py | 50 +++++------ 7 files changed, 93 insertions(+), 96 deletions(-) diff --git a/docs/developer/registry-access.md b/docs/developer/registry-access.md index c52810c35..50caa4823 100644 --- a/docs/developer/registry-access.md +++ b/docs/developer/registry-access.md @@ -130,23 +130,4 @@ domain = Domain.objects.get_or_create(name="") 10. Add yourself as domain manager 11. Go to the Registrar page and you should now see the expiring domain -If you want to be in the org model mode, turn the `organization_feature` waffle flag on, and add that domain via Django Admin to a portfolio to be able to view it. - - - - - - - - - - - - - - - - - - - +If you want to be in the org model mode, turn the `organization_feature` waffle flag on, and add that domain via Django Admin to a portfolio to be able to view it. \ No newline at end of file diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 121e2b3f7..13725f109 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -10,6 +10,7 @@ from .domain import ( DomainDsdataFormset, DomainDsdataForm, DomainSuborganizationForm, + DomainRenewalForm, ) from .portfolio import ( PortfolioOrgAddressForm, diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index b43d91a58..699efe63b 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -661,3 +661,15 @@ DomainDsdataFormset = formset_factory( extra=0, can_delete=True, ) + + +class DomainRenewalForm(forms.Form): + """Form making sure domain renewal ack is checked""" + + is_policy_acknowledged = forms.BooleanField( + required=True, + label="I have read and agree to the requirements for operating a .gov domain.", + error_messages={ + "required": "Check the box if you read and agree to the requirements for operating a .gov domain." + }, + ) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 59e04e7c3..217b88202 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -337,13 +337,14 @@ class Domain(TimeStampedModel, DomainHelper): self._cache["ex_date"] = registry.send(request, cleaned=True).res_data[0].ex_date self.expiration_date = self._cache["ex_date"] self.save() + except RegistryError as err: # if registry error occurs, log the error, and raise it as well - logger.error(f"registry error renewing domain: {err}") + logger.error(f"Registry error renewing domain '{self.name}': {err}") raise (err) except Exception as e: # exception raised during the save to registrar - logger.error(f"error updating expiration date in registrar: {e}") + logger.error(f"Error updating expiration date for domain '{self.name}' in registrar: {e}") raise (e) @Cache diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html index 07aeecd1a..bf529a04d 100644 --- a/src/registrar/templates/domain_renewal.html +++ b/src/registrar/templates/domain_renewal.html @@ -4,6 +4,18 @@ {% block domain_content %} {% block breadcrumb %} + + + {% if form.is_policy_acknowledged.errors %} +

      +
      + {% for error in form.is_policy_acknowledged.errors %} +

      {{ error }}

      + {% endfor %} +
      +
      + {% endif %} + {% if portfolio %}
      +
      {% url 'user-profile' as url %} {% include "includes/summary_item.html" with title='Your Contact Information' value=user edit_link=url editable=is_editable contact='true' %} @@ -66,62 +78,50 @@ {% include "includes/summary_item.html" with title='Domain managers' list=True users=True value=domain.permissions.all edit_link=url editable=is_editable %} {% endif %} - +
      -

      - Acknowledgement of .gov domain requirements -

      + Acknowledgement of .gov domain requirements       - - {% if messages %} -
        - {% for message in messages %} -

        {{ message }}

        - {% endfor %} - - {% endif %} - - -
        - {% csrf_token %} -
        -
        - - - - + . + * + +
        + + + + +
        {% endblock %} {# domain_content #} \ No newline at end of file diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index ee8adf903..0a4096fbf 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -454,7 +454,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.user.save() - def todays_expiration_date(self): + def expiration_date_one_year_out(self): todays_date = datetime.today() new_expiration_date = todays_date.replace(year=todays_date.year + 1) return new_expiration_date @@ -466,7 +466,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): return True def custom_renew_domain(self): - self.domain_with_ip.expiration_date = self.todays_expiration_date() + self.domain_with_ip.expiration_date = self.expiration_date_one_year_out() self.domain_with_ip.save() @override_flag("domain_renewal", active=True) @@ -659,7 +659,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertRedirects(response, reverse("domain", kwargs={"pk": self.domain_with_ip.id})) # Check for the updated expiration - formatted_new_expiration_date = self.todays_expiration_date().strftime("%b. %-d, %Y") + formatted_new_expiration_date = self.expiration_date_one_year_out().strftime("%b. %-d, %Y") redirect_response = self.client.get(reverse("domain", kwargs={"pk": self.domain_with_ip.id}), follow=True) self.assertContains(redirect_response, formatted_new_expiration_date) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index b849459f2..593fc9093 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -12,11 +12,11 @@ from django.contrib import messages from django.contrib.messages.views import SuccessMessageMixin from django.db import IntegrityError from django.http import HttpResponseRedirect -from django.shortcuts import redirect, render +from django.shortcuts import redirect, render, get_object_or_404 from django.urls import reverse from django.views.generic.edit import FormMixin from django.conf import settings -from registrar.forms.domain import DomainSuborganizationForm +from registrar.forms.domain import DomainSuborganizationForm, DomainRenewalForm from registrar.models import ( Domain, DomainRequest, @@ -364,30 +364,32 @@ class DomainRenewalView(DomainBaseView): self._update_session_with_domain() def post(self, request, pk): - domain = Domain.objects.filter(id=pk).first() + domain = get_object_or_404(Domain, id=pk) - # Check if the checkbox is checked - is_policy_acknowledged = request.POST.get("is_policy_acknowledged", None) - if is_policy_acknowledged != "on": - messages.error( - request, "Check the box if you read and agree to the requirements for operating a .gov domain." - ) - return render( - request, - "domain_renewal.html", - { - "domain": domain, - "form": request.POST, - }, - ) + form = DomainRenewalForm(request.POST) - if "submit_button" in request.POST: - try: - domain.renew_domain() - messages.success(request, "This domain has been renewed for one year.") - except Exception as e: - messages.error(request, "This domain has not been renewed for one year, error was %s" % e) - return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) + if form.is_valid(): + # check for key in the post request data + if "submit_button" in request.POST: + try: + domain.renew_domain() + messages.success(request, "This domain has been renewed for one year.") + except Exception as e: + messages.error( + request, + "This domain has not been renewed for one year, please email help@get.gov if this problem persists.", + ) + return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) + + # if not valid, render the template with error messages + return render( + request, + "domain_renewal.html", + { + "domain": domain, + "form": form, + }, + ) class DomainOrgNameAddressView(DomainFormBaseView): From e196e02d9284c55057ec1dc6ce4736c6b0f3e0ba Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 7 Jan 2025 17:28:35 -0800 Subject: [PATCH 44/77] Address linter errors --- src/registrar/views/domain.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 593fc9093..4b26caa5f 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -374,10 +374,11 @@ class DomainRenewalView(DomainBaseView): try: domain.renew_domain() messages.success(request, "This domain has been renewed for one year.") - except Exception as e: + except Exception: messages.error( request, - "This domain has not been renewed for one year, please email help@get.gov if this problem persists.", + "This domain has not been renewed for one year, " + "please email help@get.gov if this problem persists.", ) return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) From aad1ee976b5735633d93caa1a1b9724a46206206 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 7 Jan 2025 17:40:37 -0800 Subject: [PATCH 45/77] Fix checkbox unchecked test --- src/registrar/tests/test_views_domain.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 0a4096fbf..ef4d415df 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -631,19 +631,11 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): # Grab the renewal URL renewal_url = reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id}) - # Test clicking the checkbox + # Test that the checkbox is not checked response = self.client.post(renewal_url, data={"submit_button": "next"}) - # Verify the error message is displayed - # Retrieves messages obj (used in the post call) - messages = list(get_messages(response.wsgi_request)) - # Check we only get 1 error message - self.assertEqual(len(messages), 1) - # Check that the 1 error msg also is the right text - self.assertEqual( - str(messages[0]), - "Check the box if you read and agree to the requirements for operating a .gov domain.", - ) + error_message = "Check the box if you read and agree to the requirements for operating a .gov domain." + self.assertContains(response, error_message) @override_flag("domain_renewal", active=True) def test_ack_checkbox_checked(self): From 33e36590a4e0ff9b21836d22848618bc456b0533 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 7 Jan 2025 17:43:54 -0800 Subject: [PATCH 46/77] Remove unused import for linter --- src/registrar/tests/test_views_domain.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index ef4d415df..79240286a 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -11,7 +11,6 @@ from api.tests.common import less_console_noise_decorator from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .common import MockEppLib, MockSESClient, create_user # type: ignore from django_webtest import WebTest # type: ignore -from django.contrib.messages import get_messages import boto3_mocking # type: ignore from registrar.utility.errors import ( From 5ca74fdc5374b864db3aa6e32b6d2c5ebed3882c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:38:50 -0700 Subject: [PATCH 47/77] Finish unit tests --- .../commands/create_federal_portfolio.py | 32 ++++++++---- .../tests/test_management_scripts.py | 50 +++++++++++++++++-- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 8ff32824b..ddcb5a4a9 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -79,7 +79,7 @@ class Command(BaseCommand): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") - portfolio_set = set() + portfolios = [] for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) @@ -87,7 +87,7 @@ class Command(BaseCommand): # C901 'Command.handle' is too complex (12) # We currently only grab the list of changed domain requests, but we may want to grab the domains too portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) - portfolio_set.add(portfolio) + portfolios.append(portfolio) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -106,9 +106,14 @@ class Command(BaseCommand): # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. if parse_requests: - self.post_process_started_domain_requests(portfolio_set) + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + prompt_message="This action will update domain requests even if they aren't on a portfolio.", + prompt_title="Do you want to clear federal agency on started domain requests?", + ) + self.post_process_started_domain_requests(agencies, portfolios) - def post_process_started_domain_requests(self, portfolio_set): + def post_process_started_domain_requests(self, agencies, portfolios): """ Removes duplicate organization data by clearing federal_agency when it matches the portfolio name. Only processes domain requests in STARTED status. @@ -116,15 +121,24 @@ class Command(BaseCommand): message = "Removing duplicate portfolio and federal_agency values from domain requests..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + # For each request, clear the federal agency under these conditions: + # 1. A portfolio *already exists* with the same name as the federal agency. + # 2. Said portfolio (or portfolios) are only the ones specified at the start of the script. + # 3. The domain request is in status "started". + # Note: Both names are normalized so excess spaces are stripped and the string is lowercased. domain_requests_to_update = DomainRequest.objects.filter( - portfolio__in=portfolio_set, - status=DomainRequest.DomainRequestStatus.STARTED, + federal_agency__in=agencies, federal_agency__agency__isnull=False, - portfolio__organization_name__isnull=False, + status=DomainRequest.DomainRequestStatus.STARTED, + organization_name__isnull=False, ) + portfolio_set = {normalize_string(portfolio.organization_name) for portfolio in portfolios if portfolio} + + # Update the request, assuming the given agency name matches the portfolio name updated_requests = [] for req in domain_requests_to_update: - if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): + agency_name = normalize_string(req.federal_agency.agency) + if agency_name in portfolio_set: req.federal_agency = None updated_requests.append(req) DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) @@ -140,8 +154,6 @@ class Command(BaseCommand): def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will also create new suborganizations. - - Returns the processed portfolio """ portfolio, created = self.create_portfolio(federal_agency) if created: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 882396f1e..8ecb7cbea 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1424,7 +1424,6 @@ class TestCreateFederalPortfolio(TestCase): # Create an agency wih no federal type (can only be created via specifiying it manually) self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") - self.federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane") # And create some with federal_type ones with creative names self.executive_agency_1 = FederalAgency.objects.create( @@ -1518,8 +1517,13 @@ class TestCreateFederalPortfolio(TestCase): call_command("create_federal_portfolio", **kwargs) @less_console_noise_decorator - def test_handle_portfolio_requests_sync_federal_agency(self): - """Test that federal agency is cleared when org name matches portfolio name""" + def test_post_process_started_domain_requests_existing_portfolio(self): + """Ensures that federal agency is cleared when agency name matches portfolio name. + As the name implies, this implicitly tests the "post_process_started_domain_requests" function. + """ + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane", federal_type=BranchChoices.EXECUTIVE) + + # Test records with portfolios and no org names # Create a portfolio. This script skips over "started" portfolio = Portfolio.objects.create(organization_name="Sugarcane", creator=self.user) # Create a domain request with matching org name @@ -1527,7 +1531,7 @@ class TestCreateFederalPortfolio(TestCase): name="matching.gov", status=DomainRequest.DomainRequestStatus.STARTED, generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, - federal_agency=self.federal_agency_2, + federal_agency=federal_agency_2, user=self.user, portfolio=portfolio, ) @@ -1558,6 +1562,44 @@ class TestCreateFederalPortfolio(TestCase): self.assertIsNotNone(matching_request_in_wrong_status.portfolio) self.assertEqual(matching_request_in_wrong_status.portfolio.organization_name, "Test Federal Agency") + @less_console_noise_decorator + def test_post_process_started_domain_requests(self): + """Tests that federal agency is cleared when agency name + matches an existing portfolio's name, even if the domain request isn't + directly on that portfolio.""" + + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane", federal_type=BranchChoices.EXECUTIVE) + + # Create a request with matching federal_agency name but no direct portfolio association + matching_agency_request = completed_domain_request( + name="agency-match.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=federal_agency_2, + user=self.user, + ) + + # Create a control request that shouldn't match + non_matching_request = completed_domain_request( + name="no-match.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + ) + + # We expect the matching agency to have its fed agency cleared. + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) + matching_agency_request.refresh_from_db() + non_matching_request.refresh_from_db() + + # Request with matching agency name should have federal_agency cleared + self.assertIsNone(matching_agency_request.federal_agency) + + # Non-matching request should keep its federal_agency + self.assertIsNotNone(non_matching_request.federal_agency) + self.assertEqual(non_matching_request.federal_agency, self.federal_agency) + @less_console_noise_decorator def test_create_single_portfolio(self): """Test portfolio creation with suborg and senior official.""" From bc92270897cfc8967894e874f3cdddb39c8e78c6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:49:04 -0700 Subject: [PATCH 48/77] Update create_federal_portfolio.py --- .../commands/create_federal_portfolio.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index ddcb5a4a9..72548aa08 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -85,7 +85,6 @@ class Command(BaseCommand): TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - # We currently only grab the list of changed domain requests, but we may want to grab the domains too portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) portfolios.append(portfolio) except Exception as exec: @@ -105,11 +104,12 @@ class Command(BaseCommand): # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. - if parse_requests: + if parse_requests or both: TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, prompt_message="This action will update domain requests even if they aren't on a portfolio.", - prompt_title="Do you want to clear federal agency on started domain requests?", + prompt_title="Do you want to clear federal agency on (related) started domain requests?", + verify_message=None ) self.post_process_started_domain_requests(agencies, portfolios) @@ -141,15 +141,18 @@ class Command(BaseCommand): if agency_name in portfolio_set: req.federal_agency = None updated_requests.append(req) - DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) - # Log the results + # Execute the update and Log the results if TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - prompt_message=f"Updated {len(updated_requests)} domain requests successfully.", - prompt_title="Do you want to see a list of all changed domain requests?", + prompt_message=( + f"{len(domain_requests_to_update)} domain requests will be updated. " + f"These records will be changed: {[str(req) for req in updated_requests]}" + ), + prompt_title="Do wish to commit this update to the database?", ): - logger.info(f"Federal agency set to none on: {[str(request) for request in updated_requests]}") + DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will From b9dc92808440a5b124e6d146193302156920ee8c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:56:06 -0700 Subject: [PATCH 49/77] lint --- src/registrar/management/commands/create_federal_portfolio.py | 4 ++-- src/registrar/models/utility/generic_helper.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 72548aa08..d0fb14a9f 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -109,7 +109,7 @@ class Command(BaseCommand): system_exit_on_terminate=True, prompt_message="This action will update domain requests even if they aren't on a portfolio.", prompt_title="Do you want to clear federal agency on (related) started domain requests?", - verify_message=None + verify_message=None, ) self.post_process_started_domain_requests(agencies, portfolios) @@ -248,7 +248,7 @@ class Command(BaseCommand): TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: new_suborgs.append( - Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio) + Suborganization(name=name, portfolio=portfolio) ) # type: ignore if new_suborgs: diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 3a8da508e..e8992acc2 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -345,7 +345,7 @@ def value_of_attribute(obj, attribute_name: str): return value -def normalize_string(string_to_normalize: str, lowercase=True) -> str: +def normalize_string(string_to_normalize, lowercase=True): """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" if not isinstance(string_to_normalize, str): logger.error(f"normalize_string => {string_to_normalize} is not type str.") From 628c4c0d0e4015a248bcc2ee33d583bbf5bcdb69 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 8 Jan 2025 13:56:48 -0500 Subject: [PATCH 50/77] expired domain renewal form --- src/registrar/templates/domain_detail.html | 6 ++++++ src/registrar/templates/domain_sidebar.html | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index b168f7e82..becf46d5b 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -50,11 +50,17 @@ {% if domain.get_state_help_text %}
        {% if has_domain_renewal_flag and domain.is_expiring and is_domain_manager %} + This domain has expired, but it is still online. + {% url 'domain-renewal' pk=domain.id as url %} + Renew to maintain access. + {% elif has_domain_renewal_flag and domain.is_expired and is_domain_manager %} This domain will expire soon. {% url 'domain-renewal' pk=domain.id as url %} Renew to maintain access. {% elif has_domain_renewal_flag and domain.is_expiring and is_portfolio_user %} This domain will expire soon. Contact one of the listed domain managers to renew the domain. + {% elif has_domain_renewal_flag and domain.is_expired and is_portfolio_user %} + This domain has expired, but it is still online. Contact one of the listed domain managers to renew the domain. {% else %} {{ domain.get_state_help_text }} {% endif %} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 9d71ebf63..c9feb7200 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -80,7 +80,7 @@ {% include "includes/domain_sidenav_item.html" with item_text="Domain managers" %} {% endwith %} - {% if has_domain_renewal_flag and is_domain_manager and domain.is_expiring %} + {% if has_domain_renewal_flag and is_domain_manager and (domain.is_expiring or domain.is_expired) %} {% with url_name="domain-renewal" %} {% include "includes/domain_sidenav_item.html" with item_text="Renewal form" %} {% endwith %} From 6da172c0285ad0c568216311ecbe397b9103a81f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 12:25:17 -0700 Subject: [PATCH 51/77] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index d0fb14a9f..4f4002da4 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -108,7 +108,9 @@ class Command(BaseCommand): TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, prompt_message="This action will update domain requests even if they aren't on a portfolio.", - prompt_title="Do you want to clear federal agency on (related) started domain requests?", + prompt_title=( + "POST PROCESS STEP: Do you want to clear federal agency on (related) started domain requests?" + ), verify_message=None, ) self.post_process_started_domain_requests(agencies, portfolios) @@ -247,9 +249,7 @@ class Command(BaseCommand): ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append( - Suborganization(name=name, portfolio=portfolio) - ) # type: ignore + new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) # type: ignore if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) From e8fede74ac05885d3b83d90b5cff372f870a5c89 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 8 Jan 2025 15:25:06 -0500 Subject: [PATCH 52/77] updated code to fix parsing error --- src/registrar/templates/domain_sidebar.html | 10 +-- src/registrar/tests/test_views_domain.py | 77 +++++++++++++++------ 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index c9feb7200..336a9fe7c 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -80,10 +80,12 @@ {% include "includes/domain_sidenav_item.html" with item_text="Domain managers" %} {% endwith %} - {% if has_domain_renewal_flag and is_domain_manager and (domain.is_expiring or domain.is_expired) %} - {% with url_name="domain-renewal" %} - {% include "includes/domain_sidenav_item.html" with item_text="Renewal form" %} - {% endwith %} + {% if has_domain_renewal_flag and is_domain_manager%} + {% if domain.is_expiring or domain.is_expired %} + {% with url_name="domain-renewal" %} + {% include "includes/domain_sidenav_item.html" with item_text="Renewal form" %} + {% endwith %} + {% endif %} {% endif %} {% endif %} diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index ee8adf903..406e96ed4 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -440,15 +440,15 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): username="usertest", ) - self.expiringdomain, _ = Domain.objects.get_or_create( - name="expiringdomain.gov", + self.domaintorenew, _ = Domain.objects.get_or_create( + name="domainrenewal.gov", ) UserDomainRole.objects.get_or_create( - user=self.user, domain=self.expiringdomain, role=UserDomainRole.Roles.MANAGER + user=self.user, domain=self.domaintorenew, role=UserDomainRole.Roles.MANAGER ) - DomainInformation.objects.get_or_create(creator=self.user, domain=self.expiringdomain) + DomainInformation.objects.get_or_create(creator=self.user, domain=self.domaintorenew) self.portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org", creator=self.user) @@ -459,9 +459,12 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): new_expiration_date = todays_date.replace(year=todays_date.year + 1) return new_expiration_date - def custom_is_expired(self): + def custom_is_expired_false(self): return False + def custom_is_expired_true(self): + return True + def custom_is_expiring(self): return True @@ -473,11 +476,11 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): def test_expiring_domain_on_detail_page_as_domain_manager(self): self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( - Domain, "is_expired", self.custom_is_expired + Domain, "is_expired", self.custom_is_expired_false ): - self.assertEquals(self.expiringdomain.state, Domain.State.UNKNOWN) + self.assertEquals(self.domaintorenew.state, Domain.State.UNKNOWN) detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.expiringdomain.id}), + reverse("domain", kwargs={"pk": self.domaintorenew.id}), ) self.assertContains(detail_page, "Expiring soon") @@ -508,17 +511,17 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, ], ) - expiringdomain2, _ = Domain.objects.get_or_create(name="bogusdomain2.gov") + domaintorenew2, _ = Domain.objects.get_or_create(name="bogusdomain2.gov") DomainInformation.objects.get_or_create( - creator=non_dom_manage_user, domain=expiringdomain2, portfolio=self.portfolio + creator=non_dom_manage_user, domain=domaintorenew2, portfolio=self.portfolio ) non_dom_manage_user.refresh_from_db() self.client.force_login(non_dom_manage_user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( - Domain, "is_expired", self.custom_is_expired + Domain, "is_expired", self.custom_is_expired_false ): detail_page = self.client.get( - reverse("domain", kwargs={"pk": expiringdomain2.id}), + reverse("domain", kwargs={"pk": domaintorenew2.id}), ) self.assertContains(detail_page, "Contact one of the listed domain managers to renew the domain.") @@ -527,29 +530,29 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): def test_expiring_domain_on_detail_page_in_org_model_as_a_domain_manager(self): portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org2", creator=self.user) - expiringdomain3, _ = Domain.objects.get_or_create(name="bogusdomain3.gov") + domaintorenew3, _ = Domain.objects.get_or_create(name="bogusdomain3.gov") - UserDomainRole.objects.get_or_create(user=self.user, domain=expiringdomain3, role=UserDomainRole.Roles.MANAGER) - DomainInformation.objects.get_or_create(creator=self.user, domain=expiringdomain3, portfolio=portfolio) + UserDomainRole.objects.get_or_create(user=self.user, domain=domaintorenew3, role=UserDomainRole.Roles.MANAGER) + DomainInformation.objects.get_or_create(creator=self.user, domain=domaintorenew3, portfolio=portfolio) self.user.refresh_from_db() self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( - Domain, "is_expired", self.custom_is_expired + Domain, "is_expired", self.custom_is_expired_false ): detail_page = self.client.get( - reverse("domain", kwargs={"pk": expiringdomain3.id}), + reverse("domain", kwargs={"pk": domaintorenew3.id}), ) self.assertContains(detail_page, "Renew to maintain access") @override_flag("domain_renewal", active=True) - def test_domain_renewal_form_and_sidebar(self): + def test_domain_renewal_form_and_sidebar_expiring(self): self.client.force_login(self.user) with patch.object(Domain, "is_expiring", self.custom_is_expiring), patch.object( - Domain, "is_expired", self.custom_is_expired + Domain, "is_expiring", self.custom_is_expiring ): # Grab the detail page detail_page = self.client.get( - reverse("domain", kwargs={"pk": self.expiringdomain.id}), + reverse("domain", kwargs={"pk": self.domaintorenew.id}), ) # Make sure we see the link as a domain manager @@ -559,14 +562,44 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertContains(detail_page, "Renewal form") # Grab link to the renewal page - renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.expiringdomain.id}) + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domaintorenew.id}) self.assertContains(detail_page, f'href="{renewal_form_url}"') # Simulate clicking the link response = self.client.get(renewal_form_url) self.assertEqual(response.status_code, 200) - self.assertContains(response, f"Renew {self.expiringdomain.name}") + self.assertContains(response, f"Renew {self.domaintorenew.name}") + + @override_flag("domain_renewal", active=True) + def test_domain_renewal_form_and_sidebar_expired(self): + + self.client.force_login(self.user) + + with patch.object(Domain, "is_expired", self.custom_is_expired_true), patch.object( + Domain, "is_expired", self.custom_is_expired_true + ): + # Grab the detail page + detail_page = self.client.get( + reverse("domain", kwargs={"pk": self.domaintorenew.id}), + ) + + print("puglesss", self.domaintorenew.is_expired) + # Make sure we see the link as a domain manager + self.assertContains(detail_page, "Renew to maintain access") + + # Make sure we can see Renewal form on the sidebar since it's expired + self.assertContains(detail_page, "Renewal form") + + # Grab link to the renewal page + renewal_form_url = reverse("domain-renewal", kwargs={"pk": self.domaintorenew.id}) + self.assertContains(detail_page, f'href="{renewal_form_url}"') + + # Simulate clicking the link + response = self.client.get(renewal_form_url) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, f"Renew {self.domaintorenew.name}") @override_flag("domain_renewal", active=True) def test_domain_renewal_form_your_contact_info_edit(self): From c7af6645d4a8ffac6a3b33b307f97ed6871aded2 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 8 Jan 2025 15:31:59 -0500 Subject: [PATCH 53/77] ran app black . --- src/registrar/tests/test_views_domain.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 406e96ed4..575dc0faf 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -464,7 +464,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): def custom_is_expired_true(self): return True - + def custom_is_expiring(self): return True @@ -570,12 +570,12 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertEqual(response.status_code, 200) self.assertContains(response, f"Renew {self.domaintorenew.name}") - + @override_flag("domain_renewal", active=True) def test_domain_renewal_form_and_sidebar_expired(self): - + self.client.force_login(self.user) - + with patch.object(Domain, "is_expired", self.custom_is_expired_true), patch.object( Domain, "is_expired", self.custom_is_expired_true ): From acd20e89141f7a581fdb62281fc3d5c082d3b780 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 8 Jan 2025 21:16:01 -0800 Subject: [PATCH 54/77] Fix uncheck error --- src/registrar/templates/domain_renewal.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html index bf529a04d..7867bfce8 100644 --- a/src/registrar/templates/domain_renewal.html +++ b/src/registrar/templates/domain_renewal.html @@ -103,7 +103,9 @@ class="usa-checkbox__input" id="renewal-checkbox" type="checkbox" - name="{{ form.is_policy_acknowledged.name }}"> + name="{{ form.is_policy_acknowledged.name }}" + {% if form.is_policy_acknowledged.value %}checked{% endif %} + >
        {% url 'user-profile' as url %} - {% include "includes/summary_item.html" with title='Your Contact Information' value=request.user edit_link=url editable=is_editable contact='true' %} + {% include "includes/summary_item.html" with title='Your contact information' value=request.user edit_link=url editable=is_editable contact='true' %} {% if analyst_action != 'edit' or analyst_action_location != domain.pk %} {% if is_portfolio_user and not is_domain_manager %} From 25ba5b2a5172c4f7e4cd79eac25663ec0a80b4a6 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 10 Jan 2025 09:56:58 -0800 Subject: [PATCH 67/77] Fix test capitalization --- src/registrar/tests/test_views_domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 56bfe7306..d92da17dd 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -607,7 +607,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): renewal_page = self.app.get(reverse("domain-renewal", kwargs={"pk": self.domain_with_ip.id})) # Verify we see "Your contact information" on the renewal form - self.assertContains(renewal_page, "Your Contact Information") + self.assertContains(renewal_page, "Your contact information") # Verify that the "Edit" button for Your contact is there and links to correct URL edit_button_url = reverse("user-profile") From 19114c7e7214474e2afcb49a57140846e40640c6 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 10 Jan 2025 11:02:31 -0800 Subject: [PATCH 68/77] Fix checkbox issue --- src/registrar/templates/domain_renewal.html | 7 +++---- src/registrar/views/domain.py | 3 +++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/templates/domain_renewal.html b/src/registrar/templates/domain_renewal.html index f81389efb..02c9666e3 100644 --- a/src/registrar/templates/domain_renewal.html +++ b/src/registrar/templates/domain_renewal.html @@ -94,14 +94,13 @@
        {% csrf_token %}
        - {% if form.is_policy_acknowledged.errors %} -

        {{ form.is_policy_acknowledged.errors|join:" " }}

        - {% endif %} +
        - +