From 5183198d562fbbc42932525c302d6bca238d79c2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 30 Jul 2024 10:30:01 -0600 Subject: [PATCH 01/61] add view --- src/registrar/assets/js/get-gov.js | 33 +++++++++++++++ src/registrar/config/urls.py | 5 +++ src/registrar/forms/__init__.py | 1 + src/registrar/forms/domain.py | 40 +++++++++++++++++++ .../django/forms/widgets/combobox.html | 3 ++ src/registrar/templates/domain_detail.html | 4 +- src/registrar/templates/domain_sidebar.html | 5 --- .../templates/domain_suborganization.html | 28 +++++++++++++ src/registrar/views/__init__.py | 1 + src/registrar/views/domain.py | 40 ++++++++++++++++++- 10 files changed, 151 insertions(+), 9 deletions(-) create mode 100644 src/registrar/templates/django/forms/widgets/combobox.html create mode 100644 src/registrar/templates/domain_suborganization.html diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index a60a59673..0de790c76 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1978,3 +1978,36 @@ document.addEventListener('DOMContentLoaded', function() { showInputOnErrorFields(); })(); + + + +/** + * An IIFE that adds the default selection on comboboxes to the input field. + * This is because this action doesn't get fired by the time the page loads + * TODO: Will be refined in #2352 + */ +(function loadInitialValuesForComboBoxes() { + document.addEventListener('DOMContentLoaded', (event) => { + const comboBoxElements = document.querySelectorAll('.usa-combo-box'); + comboBoxElements.forEach(comboBox => { + const select = comboBox.querySelector('select'); + const input = comboBox.querySelector('input'); + + // Find the selected option + const selectedOption = select.querySelector('option[selected]'); + + // If there's a selected option, set its text as the input value. + // If the default name is "------", then this indicates that the field is blank. + // Don't populate in this case. + if (selectedOption) { + // Check to make sure the value isn't just a line of dashes. + // Caveat: we can't have any suborgs named "------". This is OK. + const isEmptyValue = /^-+$/.test(selectedOption.textContent); + if (!isEmptyValue) { + input.value = selectedOption.textContent; + comboBox.classList.add('usa-combo-box--pristine'); + } + } + }); + }); +})(); \ No newline at end of file diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 7f9db0e41..d9c70215b 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -193,6 +193,11 @@ urlpatterns = [ views.DomainOrgNameAddressView.as_view(), name="domain-org-name-address", ), + path( + "domain//suborganization", + views.DomainSubOrganizationView.as_view(), + name="domain-suborganization", + ), path( "domain//senior-official", views.DomainSeniorOfficialView.as_view(), diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 374b3102f..033e955ed 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -9,6 +9,7 @@ from .domain import ( DomainDnssecForm, DomainDsdataFormset, DomainDsdataForm, + DomainSuborganizationForm, ) from .portfolio import ( PortfolioOrgAddressForm, diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 02a0724d1..ea8fa23c6 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -6,6 +6,7 @@ from django.core.validators import MinValueValidator, MaxValueValidator, RegexVa from django.forms import formset_factory from registrar.models import DomainRequest from phonenumber_field.widgets import RegionalPhoneNumberWidget +from registrar.models.suborganization import Suborganization from registrar.models.utility.domain_helper import DomainHelper from registrar.utility.errors import ( NameserverError, @@ -153,6 +154,45 @@ class DomainNameserverForm(forms.Form): self.add_error("ip", str(e)) +class DomainSuborganizationForm(forms.ModelForm): + """Form for updating the suborganization""" + + sub_organization = forms.ModelChoiceField( + queryset=Suborganization.objects.none(), + required=True, + widget=forms.Select(), + ) + + class Meta: + model = DomainInformation + fields = [ + "sub_organization", + ] + + def __init__(self, *args, **kwargs): + # Get the incoming request object + self.request = kwargs.pop("request", None) + super().__init__(*args, **kwargs) + + portfolio = None + if self.instance and self.instance.portfolio: + # Get suborgs under the portfolio that this is associated with first + portfolio = self.instance.portfolio + elif self.request and self.request.user and self.request.user.portfolio: + # Question: If no portfolio is associated with this record, + # should we default to the user one? + # portfolio = self.request.user.portfolio + logger.warning(f"No portfolio was found for {self.instance}.") + + self.fields["sub_organization"].queryset = Suborganization.objects.filter(portfolio=portfolio) + + # Set custom form label + self.fields["sub_organization"].label = "Suborganization name" + + # Use the combobox rather than the regular select widget + self.fields["sub_organization"].widget.template_name = "django/forms/widgets/combobox.html" + + class BaseNameserverFormset(forms.BaseFormSet): def clean(self): """ diff --git a/src/registrar/templates/django/forms/widgets/combobox.html b/src/registrar/templates/django/forms/widgets/combobox.html new file mode 100644 index 000000000..c338953ed --- /dev/null +++ b/src/registrar/templates/django/forms/widgets/combobox.html @@ -0,0 +1,3 @@ +
+ {% include "django/forms/widgets/select.html" %} +
\ No newline at end of file diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index 711c3ac2a..4417b4847 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -54,10 +54,8 @@ {% endif %} {% if is_org_user %} - {% comment %} TODO - uncomment in #2352 and add to edit_link {% url 'domain-suborganization' pk=domain.id as url %} - {% endcomment %} - {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link="#" editable=domain.is_editable %} + {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=domain.is_editable %} {% else %} {% url 'domain-org-name-address' pk=domain.id as url %} {% include "includes/summary_item.html" with title='Organization name and mailing address' value=domain.domain_info address='true' edit_link=url editable=domain.is_editable %} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index a4b6d85cb..389df3baa 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -9,14 +9,9 @@ {% endwith %} {% if is_org_user %} - {% comment %} TODO - uncomment in #2352 {% with url_name="domain-suborganization" %} {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %} {% endwith %} - {% endcomment %} - {% with url="#" %} - {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %} - {% endwith %} {% else %} {% with url_name="domain-org-name-address" %} {% include "includes/domain_sidenav_item.html" with item_text="Organization name and mailing address" %} diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html new file mode 100644 index 000000000..729fd8407 --- /dev/null +++ b/src/registrar/templates/domain_suborganization.html @@ -0,0 +1,28 @@ +{% extends "domain_base.html" %} +{% load static field_helpers%} + +{% block title %}Suborganization{% endblock %} + +{% block domain_content %} + {# this is right after the messages block in the parent template #} + {% include "includes/form_errors.html" with form=form %} + +

Organization name and mailing address

+ +

+ The name of your suborganization will be publicly listed as the domain registrant. + This list of suborganizations has been populated the .gov program. + If you believe there is an error please contact help@get.gov. +

+ + {% if suborganization_is_editable %} + {% include "includes/required_fields.html" %} + {% endif %} + +
+ {% csrf_token %} + {% input_with_errors form.sub_organization %} + +
+ +{% endblock %} \ No newline at end of file diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index 37977f334..c224e72c0 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -3,6 +3,7 @@ from .domain import ( DomainView, DomainSeniorOfficialView, DomainOrgNameAddressView, + DomainSubOrganizationView, DomainDNSView, DomainNameserversView, DomainDNSSECView, diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index e0b37e770..566d8d162 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -15,7 +15,7 @@ from django.shortcuts import redirect 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.models import ( Domain, DomainRequest, @@ -222,6 +222,44 @@ class DomainOrgNameAddressView(DomainFormBaseView): return super().form_valid(form) +class DomainSubOrganizationView(DomainFormBaseView): + """Suborganization view""" + + model = Domain + template_name = "domain_suborganization.html" + context_object_name = "domain" + form_class = DomainSuborganizationForm + + def get_form_kwargs(self, *args, **kwargs): + """Add domain_info.organization_name instance to make a bound form.""" + form_kwargs = super().get_form_kwargs(*args, **kwargs) + form_kwargs["instance"] = self.object.domain_info + form_kwargs["request"] = self.request + return form_kwargs + + def get_success_url(self): + """Redirect to the overview page for the domain.""" + return reverse("domain-suborganization", kwargs={"pk": self.object.pk}) + + def form_valid(self, form): + """The form is valid, save the organization name and mailing address.""" + form.save() + + messages.success(self.request, "The suborganization name for this domain has been updated.") + + # superclass has the redirect + return super().form_valid(form) + + def get_context_data(self, **kwargs): + """Adds custom context.""" + context = super().get_context_data(**kwargs) + + # TODO: Switch to True #2352 + suborganization_is_editable = False + context["suborganization_is_editable"] = suborganization_is_editable + return context + + class DomainSeniorOfficialView(DomainFormBaseView): """Domain senior official editing view.""" From d58ea347aab10d7903c3b2da6c56a44317e96df2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 30 Jul 2024 12:39:34 -0600 Subject: [PATCH 02/61] Clean --- src/registrar/assets/js/get-gov.js | 1 - src/registrar/forms/domain.py | 2 +- src/registrar/templates/django/forms/widgets/combobox.html | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 0de790c76..075b28c0f 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1984,7 +1984,6 @@ document.addEventListener('DOMContentLoaded', function() { /** * An IIFE that adds the default selection on comboboxes to the input field. * This is because this action doesn't get fired by the time the page loads - * TODO: Will be refined in #2352 */ (function loadInitialValuesForComboBoxes() { document.addEventListener('DOMContentLoaded', (event) => { diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index ea8fa23c6..43c588d45 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -182,7 +182,7 @@ class DomainSuborganizationForm(forms.ModelForm): # Question: If no portfolio is associated with this record, # should we default to the user one? # portfolio = self.request.user.portfolio - logger.warning(f"No portfolio was found for {self.instance}.") + logger.warning(f"No portfolio was found for {self.instance} on user {self.request.user}.") self.fields["sub_organization"].queryset = Suborganization.objects.filter(portfolio=portfolio) diff --git a/src/registrar/templates/django/forms/widgets/combobox.html b/src/registrar/templates/django/forms/widgets/combobox.html index c338953ed..565d35415 100644 --- a/src/registrar/templates/django/forms/widgets/combobox.html +++ b/src/registrar/templates/django/forms/widgets/combobox.html @@ -1,3 +1,3 @@
{% include "django/forms/widgets/select.html" %} -
\ No newline at end of file + From 808c699027da7e461a546daf793e9ffc3bfb54d2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 10:15:28 -0600 Subject: [PATCH 03/61] Update domain_sidebar.html --- src/registrar/templates/domain_sidebar.html | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 7fa7d4d5e..274f17a25 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -11,7 +11,6 @@ {% if is_editable %} {% if portfolio %} - {% comment %} TODO - uncomment in #2352 {% with url_name="domain-suborganization" %} {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %} {% endwith %} From 22bdb1d3b0be625bfbe0a81d0ca0893d35f93739 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 11:40:18 -0600 Subject: [PATCH 04/61] More content --- src/registrar/context_processors.py | 2 ++ src/registrar/models/user.py | 3 +++ .../templates/domain_suborganization.html | 17 +++++++++-------- src/registrar/views/domain.py | 9 --------- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 861a4e701..b8edcb4d6 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -66,6 +66,7 @@ def portfolio_permissions(request): "has_base_portfolio_permission": False, "has_domains_portfolio_permission": False, "has_domain_requests_portfolio_permission": False, + "has_edit_org_portfolio_permission": False, "portfolio": None, "has_organization_feature_flag": False, } @@ -73,6 +74,7 @@ def portfolio_permissions(request): "has_base_portfolio_permission": request.user.has_base_portfolio_permission(), "has_domains_portfolio_permission": request.user.has_domains_portfolio_permission(), "has_domain_requests_portfolio_permission": request.user.has_domain_requests_portfolio_permission(), + "has_edit_org_portfolio_permission": request.user.has_edit_org_portfolio_permission(), "portfolio": request.user.portfolio, "has_organization_feature_flag": flag_is_active(request, "organization_feature"), } diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index b1c9473db..ba860d94b 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -275,6 +275,9 @@ class User(AbstractUser): def has_base_portfolio_permission(self): return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO) + def has_edit_org_portfolio_permission(self): + return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO) + def has_domains_portfolio_permission(self): return self._has_portfolio_permission( User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index 729fd8407..29cda3492 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -15,14 +15,15 @@ If you believe there is an error please contact help@get.gov.

- {% if suborganization_is_editable %} - {% include "includes/required_fields.html" %} + {% if has_edit_org_portfolio_permission %} + {% include "includes/required_fields.html" %} +
+ {% csrf_token %} + {% input_with_errors form.sub_organization %} + +
+ {% else %} +

Readonly content here

{% endif %} -
- {% csrf_token %} - {% input_with_errors form.sub_organization %} - -
- {% endblock %} \ No newline at end of file diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1fb92fbdb..5fc07e12a 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -261,15 +261,6 @@ class DomainSubOrganizationView(DomainFormBaseView): # superclass has the redirect return super().form_valid(form) - def get_context_data(self, **kwargs): - """Adds custom context.""" - context = super().get_context_data(**kwargs) - - # TODO: Switch to True #2352 - suborganization_is_editable = False - context["suborganization_is_editable"] = suborganization_is_editable - return context - class DomainSeniorOfficialView(DomainFormBaseView): """Domain senior official editing view.""" From 03e184e2616ecd9e9dcc55bed02a795d2dc87892 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 2 Aug 2024 14:38:45 -0600 Subject: [PATCH 05/61] Add readonly view --- src/registrar/models/user.py | 15 ++++++++++++--- src/registrar/models/utility/portfolio_helper.py | 5 +++++ src/registrar/templates/domain_detail.html | 5 +++-- src/registrar/templates/domain_sidebar.html | 9 ++++++--- .../templates/domain_suborganization.html | 8 +++++--- .../templates/includes/domains_table.html | 2 +- .../templates/includes/input_read_only.html | 3 +++ src/registrar/templatetags/custom_filters.py | 9 +++++++++ 8 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 6dc86df25..451a52213 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -73,12 +73,17 @@ class User(AbstractUser): UserPortfolioPermissionChoices.EDIT_REQUESTS, UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO, + # Domain: field specific permissions + UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, + UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, ], UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [ UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, UserPortfolioPermissionChoices.VIEW_MEMBER, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + # Domain: field specific permissions + UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, ], UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, @@ -255,9 +260,6 @@ class User(AbstractUser): def has_edit_org_portfolio_permission(self): return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_PORTFOLIO) - def has_edit_org_portfolio_permission(self): - return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_PORTFOLIO) - def has_domains_portfolio_permission(self): return self._has_portfolio_permission( UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS @@ -268,6 +270,13 @@ class User(AbstractUser): UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS ) or self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) + # Field specific permission checks + def has_view_suborganization(self): + return self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) + + def has_edit_suborganization(self): + return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) + @classmethod def needs_identity_verification(cls, email, uuid): """A method used by our oidc classes to test whether a user needs email/uuid verification diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 70977f312..2edca3422 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -26,3 +26,8 @@ class UserPortfolioPermissionChoices(models.TextChoices): VIEW_PORTFOLIO = "view_portfolio", "View organization" EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" + + # TODO - think of other solutions + # Domain: field specific permissions + VIEW_SUBORGANIZATION = "view_suborganization", "View suborganization" + EDIT_SUBORGANIZATION = "edit_suborganization", "Edit suborganization" diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index aa3f7efbb..af7fee363 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -1,5 +1,6 @@ {% extends "domain_base.html" %} {% load static url_helpers %} +{% load custom_filters %} {% block domain_content %} {{ block.super }} @@ -64,9 +65,9 @@ {% endif %} {% endif %} - {% if is_org_user %} + {% if portfolio and has_domains_portfolio_permission and request.user.has_view_suborganization %} {% url 'domain-suborganization' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable %} + {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:request.user.has_edit_suborganization %} {% else %} {% url 'domain-org-name-address' pk=domain.id as url %} {% include "includes/summary_item.html" with title='Organization name and mailing address' value=domain.domain_info address='true' edit_link=url editable=is_editable %} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 274f17a25..fad2d82a0 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -11,9 +11,12 @@ {% if is_editable %} {% if portfolio %} - {% with url_name="domain-suborganization" %} - {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %} - {% endwith %} + {% comment %} Only show this menu option if the user has the perms to do so {% endcomment %} + {% if has_domains_portfolio_permission and request.user.has_view_suborganization %} + {% with url_name="domain-suborganization" %} + {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %} + {% endwith %} + {% endif %} {% else %} {% with url_name="domain-org-name-address" %} {% include "includes/domain_sidenav_item.html" with item_text="Organization name and mailing address" %} diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index 29cda3492..6dbf2f0ff 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -7,7 +7,7 @@ {# this is right after the messages block in the parent template #} {% include "includes/form_errors.html" with form=form %} -

Organization name and mailing address

+

Suborganization

The name of your suborganization will be publicly listed as the domain registrant. @@ -15,7 +15,7 @@ If you believe there is an error please contact help@get.gov.

- {% if has_edit_org_portfolio_permission %} + {% if has_domains_portfolio_permission and request.user.has_edit_suborganization %} {% include "includes/required_fields.html" %}
{% csrf_token %} @@ -23,7 +23,9 @@
{% else %} -

Readonly content here

+ {% with description="The suborganization for this domain can only be updated by a organization administrator."%} + {% include "includes/input_read_only.html" with field=form.sub_organization label_description=description%} + {% endwith %} {% endif %} {% endblock %} \ No newline at end of file diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 64eddec41..3982e2d32 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -150,7 +150,7 @@ Domain name Expires Status - {% if has_domains_portfolio_permission %} + {% if has_domains_portfolio_permission and request.user.has_view_suborganization %} Suborganization {% endif %} {{ field.label }} +{% if label_description %} +

{{ label_description }}

+{% endif %}

{{ field.value }}

diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 8338eaf9d..5dcdecef6 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -150,3 +150,12 @@ def format_phone(value): @register.filter def in_path(url, path): return url in path + + +@register.filter(name='and') +def and_filter(value, arg): + """ + Implements logical AND operation in templates. + Usage: {{ value|and:arg }} + """ + return bool(value and arg) \ No newline at end of file From a1c63a809dda037f9eb3aff5eaf6bc637dec36c9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 08:50:31 -0600 Subject: [PATCH 06/61] Make field not required --- src/registrar/forms/domain.py | 2 +- ...rtfolio_additional_permissions_and_more.py | 66 +++++++++++++++++++ .../templates/domain_suborganization.html | 1 - 3 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 src/registrar/migrations/0116_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 43c588d45..9c99cfbe6 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -159,7 +159,7 @@ class DomainSuborganizationForm(forms.ModelForm): sub_organization = forms.ModelChoiceField( queryset=Suborganization.objects.none(), - required=True, + required=False, widget=forms.Select(), ) diff --git a/src/registrar/migrations/0116_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py b/src/registrar/migrations/0116_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py new file mode 100644 index 000000000..90623bff9 --- /dev/null +++ b/src/registrar/migrations/0116_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py @@ -0,0 +1,66 @@ +# Generated by Django 4.2.10 on 2024-08-05 14:33 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0115_portfolioinvitation"), + ] + + operations = [ + migrations.AlterField( + model_name="portfolioinvitation", + name="portfolio_additional_permissions", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("view_all_domains", "View all domains and domain reports"), + ("view_managed_domains", "View managed domains"), + ("view_member", "View members"), + ("edit_member", "Create and edit members"), + ("view_all_requests", "View all requests"), + ("view_created_requests", "View created requests"), + ("edit_requests", "Create and edit requests"), + ("view_portfolio", "View organization"), + ("edit_portfolio", "Edit organization"), + ("view_suborganization", "View suborganization"), + ("edit_suborganization", "Edit suborganization"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + migrations.AlterField( + model_name="user", + name="portfolio_additional_permissions", + field=django.contrib.postgres.fields.ArrayField( + base_field=models.CharField( + choices=[ + ("view_all_domains", "View all domains and domain reports"), + ("view_managed_domains", "View managed domains"), + ("view_member", "View members"), + ("edit_member", "Create and edit members"), + ("view_all_requests", "View all requests"), + ("view_created_requests", "View created requests"), + ("edit_requests", "Create and edit requests"), + ("view_portfolio", "View organization"), + ("edit_portfolio", "Edit organization"), + ("view_suborganization", "View suborganization"), + ("edit_suborganization", "Edit suborganization"), + ], + max_length=50, + ), + blank=True, + help_text="Select one or more additional permissions.", + null=True, + size=None, + ), + ), + ] diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index 6dbf2f0ff..57ec1d073 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -16,7 +16,6 @@

{% if has_domains_portfolio_permission and request.user.has_edit_suborganization %} - {% include "includes/required_fields.html" %}
{% csrf_token %} {% input_with_errors form.sub_organization %} From 6a0412d872bf643f7899876be4279009d099c854 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 09:15:55 -0600 Subject: [PATCH 07/61] Display name rather than id --- src/registrar/forms/domain.py | 4 ++++ src/registrar/templates/domain_suborganization.html | 2 +- src/registrar/templates/includes/input_read_only.html | 6 +++++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 9c99cfbe6..dd93f96f5 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -191,6 +191,10 @@ class DomainSuborganizationForm(forms.ModelForm): # Use the combobox rather than the regular select widget self.fields["sub_organization"].widget.template_name = "django/forms/widgets/combobox.html" + + def get_suborganization_name(self): + """Returns the suborganization name for the readonly view""" + return self.instance.sub_organization if self.instance else None class BaseNameserverFormset(forms.BaseFormSet): diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index 57ec1d073..cea889352 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -23,7 +23,7 @@
{% else %} {% with description="The suborganization for this domain can only be updated by a organization administrator."%} - {% include "includes/input_read_only.html" with field=form.sub_organization label_description=description%} + {% include "includes/input_read_only.html" with field=form.sub_organization value=instance.get_suborganization_name label_description=description%} {% endwith %} {% endif %} diff --git a/src/registrar/templates/includes/input_read_only.html b/src/registrar/templates/includes/input_read_only.html index 0e25a9a1f..e5161d3ba 100644 --- a/src/registrar/templates/includes/input_read_only.html +++ b/src/registrar/templates/includes/input_read_only.html @@ -7,4 +7,8 @@ Template include for read-only form fields {% if label_description %}

{{ label_description }}

{% endif %} -

{{ field.value }}

+{% comment %} +This allows us to customize the displayed value. +For instance, Select fields will display the id by default. +{% endcomment %} +

{{ value|default:field.value }}

From 74872cdd22b208c52abaf81aee2cd1e87931f71b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 11:25:18 -0600 Subject: [PATCH 08/61] Set initial value using the template rather than js --- src/registrar/assets/js/get-gov.js | 32 ------------- src/registrar/forms/domain.py | 8 ++++ .../django/forms/widgets/combobox.html | 6 ++- src/registrar/tests/test_views_domain.py | 46 +++++++++++++++++++ 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 1af8c1075..0712da0f7 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1985,35 +1985,3 @@ document.addEventListener('DOMContentLoaded', function() { showInputOnErrorFields(); })(); - - - -/** - * An IIFE that adds the default selection on comboboxes to the input field. - * This is because this action doesn't get fired by the time the page loads - */ -(function loadInitialValuesForComboBoxes() { - document.addEventListener('DOMContentLoaded', (event) => { - const comboBoxElements = document.querySelectorAll('.usa-combo-box'); - comboBoxElements.forEach(comboBox => { - const select = comboBox.querySelector('select'); - const input = comboBox.querySelector('input'); - - // Find the selected option - const selectedOption = select.querySelector('option[selected]'); - - // If there's a selected option, set its text as the input value. - // If the default name is "------", then this indicates that the field is blank. - // Don't populate in this case. - if (selectedOption) { - // Check to make sure the value isn't just a line of dashes. - // Caveat: we can't have any suborgs named "------". This is OK. - const isEmptyValue = /^-+$/.test(selectedOption.textContent); - if (!isEmptyValue) { - input.value = selectedOption.textContent; - comboBox.classList.add('usa-combo-box--pristine'); - } - } - }); - }); -})(); \ No newline at end of file diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index dd93f96f5..074c4add4 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -186,11 +186,19 @@ class DomainSuborganizationForm(forms.ModelForm): self.fields["sub_organization"].queryset = Suborganization.objects.filter(portfolio=portfolio) + # Set initial value + if self.instance and self.instance.sub_organization: + self.fields['sub_organization'].initial = self.instance.sub_organization + # Set custom form label self.fields["sub_organization"].label = "Suborganization name" # Use the combobox rather than the regular select widget self.fields["sub_organization"].widget.template_name = "django/forms/widgets/combobox.html" + + # Set data-default-value attribute + self.fields['sub_organization'].widget.attrs['data-default-value'] = self.instance.sub_organization.pk if self.instance and self.instance.sub_organization else '' + def get_suborganization_name(self): """Returns the suborganization name for the readonly view""" diff --git a/src/registrar/templates/django/forms/widgets/combobox.html b/src/registrar/templates/django/forms/widgets/combobox.html index 565d35415..c53fdc816 100644 --- a/src/registrar/templates/django/forms/widgets/combobox.html +++ b/src/registrar/templates/django/forms/widgets/combobox.html @@ -1,3 +1,7 @@ -
+
{% include "django/forms/widgets/select.html" %}
diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 16b6690c0..2bf88a8b8 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1577,6 +1577,52 @@ class TestDomainOrganization(TestDomainOverview): class TestDomainSuborganization(TestDomainOverview): """Tests the Suborganization page for portfolio users""" + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_edit_suborganization_field(self): + + # Create a portfolio and two suborgs + portfolio = Portfolio.objects.create(creator=self.user, organization_name="Ice Cream") + suborg = Suborganization.objects.create(portfolio=portfolio, name="Vanilla") + suborg_2 = Suborganization.objects.create(portfolio=portfolio, name="Chocolate") + + # Create an unrelated portfolio + unrelated_portfolio = Portfolio.objects.create(creator=self.user, organization_name="Fruit") + unrelated_suborg = Suborganization.objects.create(portfolio=portfolio, name="Apple") + + # Add the portfolio to the domain_information object + self.domain_information.portfolio = portfolio + + # Add a organization_name to test if the old value still displays + self.domain_information.organization_name = "Broccoli" + self.domain_information.save() + self.domain_information.refresh_from_db() + + # Add portfolio perms to the user object + self.user.portfolio = portfolio + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + self.user.save() + self.user.refresh_from_db() + + # Navigate to the suborganization page + page = self.app.get(reverse("domain-suborganization", kwargs={"pk": self.domain.id})) + + print(page) + # The page should contain the choices Vanilla and Chocolate + self.assertContains(page, "Vanilla") + self.assertContains("Chocolate") + self.assertNotContains("Apple") + + # Try changing the suborg + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + page.form["suborganization"] = suborg_2 + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + page.form.submit() + + self.assertContains(page, "The suborganization name for this domain has been updated.") + self.assertNotContains(page, "Vanilla") + self.assertContains("Chocolate") + @less_console_noise_decorator @override_flag("organization_feature", active=True) def test_has_suborganization_field_on_overview_with_flag(self): From f38cd72d4c222153a2867891befdb654ca1d0e83 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 11:49:35 -0600 Subject: [PATCH 09/61] Add unit test --- src/registrar/tests/test_views_domain.py | 32 ++++++++++++++++-------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 2bf88a8b8..f58ed4cd0 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1580,7 +1580,7 @@ class TestDomainSuborganization(TestDomainOverview): @less_console_noise_decorator @override_flag("organization_feature", active=True) def test_edit_suborganization_field(self): - + # Create a portfolio and two suborgs portfolio = Portfolio.objects.create(creator=self.user, organization_name="Ice Cream") suborg = Suborganization.objects.create(portfolio=portfolio, name="Vanilla") @@ -1588,10 +1588,11 @@ class TestDomainSuborganization(TestDomainOverview): # Create an unrelated portfolio unrelated_portfolio = Portfolio.objects.create(creator=self.user, organization_name="Fruit") - unrelated_suborg = Suborganization.objects.create(portfolio=portfolio, name="Apple") + unrelated_suborg = Suborganization.objects.create(portfolio=unrelated_portfolio, name="Apple") # Add the portfolio to the domain_information object self.domain_information.portfolio = portfolio + self.domain_information.sub_organization = suborg # Add a organization_name to test if the old value still displays self.domain_information.organization_name = "Broccoli" @@ -1604,24 +1605,35 @@ class TestDomainSuborganization(TestDomainOverview): self.user.save() self.user.refresh_from_db() + self.assertEqual(self.domain_information.sub_organization, suborg) + # Navigate to the suborganization page page = self.app.get(reverse("domain-suborganization", kwargs={"pk": self.domain.id})) - print(page) # The page should contain the choices Vanilla and Chocolate self.assertContains(page, "Vanilla") - self.assertContains("Chocolate") - self.assertNotContains("Apple") + self.assertContains(page, "Chocolate") + self.assertNotContains(page, unrelated_suborg.name) + + # Assert that the right option is selected. This component uses data-default-value. + self.assertContains(page, f'data-default-value="{suborg.id}"') # Try changing the suborg session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - page.form["suborganization"] = suborg_2 + page.form["sub_organization"] = suborg_2.id self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - page.form.submit() + page = page.form.submit().follow() - self.assertContains(page, "The suborganization name for this domain has been updated.") - self.assertNotContains(page, "Vanilla") - self.assertContains("Chocolate") + # The page should contain the choices Vanilla and Chocolate + self.assertContains(page, "Vanilla") + self.assertContains(page, "Chocolate") + self.assertNotContains(page, unrelated_suborg.name) + + # Assert that the right option is selected + self.assertContains(page, f'data-default-value="{suborg_2.id}"') + + self.domain_information.refresh_from_db() + self.assertEqual(self.domain_information.sub_organization, suborg_2) @less_console_noise_decorator @override_flag("organization_feature", active=True) From 7b8da895db2adf90beffff830738a156682d6fcb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 12:10:30 -0600 Subject: [PATCH 10/61] Cleanup --- src/registrar/forms/domain.py | 19 +++++-------------- .../templates/domain_suborganization.html | 3 +-- src/registrar/templatetags/custom_filters.py | 2 +- 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 074c4add4..ec29f4032 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -174,21 +174,12 @@ class DomainSuborganizationForm(forms.ModelForm): self.request = kwargs.pop("request", None) super().__init__(*args, **kwargs) - portfolio = None - if self.instance and self.instance.portfolio: - # Get suborgs under the portfolio that this is associated with first - portfolio = self.instance.portfolio - elif self.request and self.request.user and self.request.user.portfolio: - # Question: If no portfolio is associated with this record, - # should we default to the user one? - # portfolio = self.request.user.portfolio - logger.warning(f"No portfolio was found for {self.instance} on user {self.request.user}.") - + portfolio = self.instance.portfolio if self.instance else None self.fields["sub_organization"].queryset = Suborganization.objects.filter(portfolio=portfolio) # Set initial value if self.instance and self.instance.sub_organization: - self.fields['sub_organization'].initial = self.instance.sub_organization + self.fields["sub_organization"].initial = self.instance.sub_organization # Set custom form label self.fields["sub_organization"].label = "Suborganization name" @@ -197,12 +188,12 @@ class DomainSuborganizationForm(forms.ModelForm): self.fields["sub_organization"].widget.template_name = "django/forms/widgets/combobox.html" # Set data-default-value attribute - self.fields['sub_organization'].widget.attrs['data-default-value'] = self.instance.sub_organization.pk if self.instance and self.instance.sub_organization else '' + if self.instance and self.instance.sub_organization: + self.fields["sub_organization"].widget.attrs["data-default-value"] = self.instance.sub_organization.pk - def get_suborganization_name(self): """Returns the suborganization name for the readonly view""" - return self.instance.sub_organization if self.instance else None + return self.instance.sub_organization.name if self.instance else None class BaseNameserverFormset(forms.BaseFormSet): diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index cea889352..d1fdeac2f 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -26,5 +26,4 @@ {% include "includes/input_read_only.html" with field=form.sub_organization value=instance.get_suborganization_name label_description=description%} {% endwith %} {% endif %} - -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 5dcdecef6..7ad63bd15 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -158,4 +158,4 @@ def and_filter(value, arg): Implements logical AND operation in templates. Usage: {{ value|and:arg }} """ - return bool(value and arg) \ No newline at end of file + return bool(value and arg) From c109d383c09042b5a614d943739d775ce5f5b078 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 12:19:59 -0600 Subject: [PATCH 11/61] Add unit test for view --- src/registrar/tests/test_views_domain.py | 47 +++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index f58ed4cd0..1ec1f004a 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1580,7 +1580,7 @@ class TestDomainSuborganization(TestDomainOverview): @less_console_noise_decorator @override_flag("organization_feature", active=True) def test_edit_suborganization_field(self): - + """Ensure that org admins can edit the suborganization field""" # Create a portfolio and two suborgs portfolio = Portfolio.objects.create(creator=self.user, organization_name="Ice Cream") suborg = Suborganization.objects.create(portfolio=portfolio, name="Vanilla") @@ -1635,6 +1635,51 @@ class TestDomainSuborganization(TestDomainOverview): self.domain_information.refresh_from_db() self.assertEqual(self.domain_information.sub_organization, suborg_2) + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_view_suborganization_field(self): + """Only org admins can edit the suborg field, ensure that others cannot""" + + # Create a portfolio and two suborgs + portfolio = Portfolio.objects.create(creator=self.user, organization_name="Ice Cream") + suborg = Suborganization.objects.create(portfolio=portfolio, name="Vanilla") + suborg_2 = Suborganization.objects.create(portfolio=portfolio, name="Chocolate") + + # Create an unrelated portfolio + unrelated_portfolio = Portfolio.objects.create(creator=self.user, organization_name="Fruit") + unrelated_suborg = Suborganization.objects.create(portfolio=unrelated_portfolio, name="Apple") + + # Add the portfolio to the domain_information object + self.domain_information.portfolio = portfolio + self.domain_information.sub_organization = suborg + + # Add a organization_name to test if the old value still displays + self.domain_information.organization_name = "Broccoli" + self.domain_information.save() + self.domain_information.refresh_from_db() + + # Add portfolio perms to the user object + self.user.portfolio = portfolio + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] + self.user.save() + self.user.refresh_from_db() + + self.assertEqual(self.domain_information.sub_organization, suborg) + + # Navigate to the suborganization page + page = self.app.get(reverse("domain-suborganization", kwargs={"pk": self.domain.id})) + + # The page shouldn't contain these choices + self.assertNotContains(page, "Vanilla") + self.assertNotContains(page, "Chocolate") + self.assertNotContains(page, unrelated_suborg.name) + self.assertNotContains(page, "Save") + + self.assertContains( + page, + "The suborganization for this domain can only be updated by a organization administrator." + ) + @less_console_noise_decorator @override_flag("organization_feature", active=True) def test_has_suborganization_field_on_overview_with_flag(self): From 9c800e1f1d5a1a3b645438d17ea35754d923e39e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 12:23:49 -0600 Subject: [PATCH 12/61] Linting cleanup --- src/registrar/models/user.py | 2 +- src/registrar/templatetags/custom_filters.py | 2 +- src/registrar/tests/test_views_domain.py | 5 ++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 451a52213..e3e59409a 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -273,7 +273,7 @@ class User(AbstractUser): # Field specific permission checks def has_view_suborganization(self): return self._has_portfolio_permission(UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) - + def has_edit_suborganization(self): return self._has_portfolio_permission(UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 7ad63bd15..e90b3b17f 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -152,7 +152,7 @@ def in_path(url, path): return url in path -@register.filter(name='and') +@register.filter(name="and") def and_filter(value, arg): """ Implements logical AND operation in templates. diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 1ec1f004a..d6fe95371 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1643,7 +1643,7 @@ class TestDomainSuborganization(TestDomainOverview): # Create a portfolio and two suborgs portfolio = Portfolio.objects.create(creator=self.user, organization_name="Ice Cream") suborg = Suborganization.objects.create(portfolio=portfolio, name="Vanilla") - suborg_2 = Suborganization.objects.create(portfolio=portfolio, name="Chocolate") + Suborganization.objects.create(portfolio=portfolio, name="Chocolate") # Create an unrelated portfolio unrelated_portfolio = Portfolio.objects.create(creator=self.user, organization_name="Fruit") @@ -1676,8 +1676,7 @@ class TestDomainSuborganization(TestDomainOverview): self.assertNotContains(page, "Save") self.assertContains( - page, - "The suborganization for this domain can only be updated by a organization administrator." + page, "The suborganization for this domain can only be updated by a organization administrator." ) @less_console_noise_decorator From bd143af6f1de484c52c79aa14389b7eac9d8d669 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 12:29:45 -0600 Subject: [PATCH 13/61] Fix test --- 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 d6fe95371..de9cd5c0a 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1161,7 +1161,7 @@ class TestDomainSeniorOfficial(TestDomainOverview): # Add portfolio perms to the user object self.user.portfolio = portfolio - self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] self.user.save() self.user.refresh_from_db() From cde0d527ce7251e9073a66b053034d9657937ecb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 5 Aug 2024 12:59:44 -0600 Subject: [PATCH 14/61] PR cleanup --- src/registrar/context_processors.py | 2 -- src/registrar/forms/domain.py | 2 -- src/registrar/models/utility/portfolio_helper.py | 1 - src/registrar/templates/django/forms/widgets/combobox.html | 7 +++++++ src/registrar/views/domain.py | 5 +---- src/registrar/views/portfolios.py | 2 -- 6 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 36706edc9..ee5f8aee1 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -66,7 +66,6 @@ def portfolio_permissions(request): "has_base_portfolio_permission": False, "has_domains_portfolio_permission": False, "has_domain_requests_portfolio_permission": False, - "has_edit_org_portfolio_permission": False, "portfolio": None, "has_organization_feature_flag": False, } @@ -74,7 +73,6 @@ def portfolio_permissions(request): "has_base_portfolio_permission": request.user.has_base_portfolio_permission(), "has_domains_portfolio_permission": request.user.has_domains_portfolio_permission(), "has_domain_requests_portfolio_permission": request.user.has_domain_requests_portfolio_permission(), - "has_edit_org_portfolio_permission": request.user.has_edit_org_portfolio_permission(), "portfolio": request.user.portfolio, "has_organization_feature_flag": True, } diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index ec29f4032..ec370f268 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -170,8 +170,6 @@ class DomainSuborganizationForm(forms.ModelForm): ] def __init__(self, *args, **kwargs): - # Get the incoming request object - self.request = kwargs.pop("request", None) super().__init__(*args, **kwargs) portfolio = self.instance.portfolio if self.instance else None diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 2edca3422..86aaa5e16 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -27,7 +27,6 @@ class UserPortfolioPermissionChoices(models.TextChoices): VIEW_PORTFOLIO = "view_portfolio", "View organization" EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" - # TODO - think of other solutions # Domain: field specific permissions VIEW_SUBORGANIZATION = "view_suborganization", "View suborganization" EDIT_SUBORGANIZATION = "edit_suborganization", "Edit suborganization" diff --git a/src/registrar/templates/django/forms/widgets/combobox.html b/src/registrar/templates/django/forms/widgets/combobox.html index c53fdc816..107c2e14e 100644 --- a/src/registrar/templates/django/forms/widgets/combobox.html +++ b/src/registrar/templates/django/forms/widgets/combobox.html @@ -1,3 +1,10 @@ +{% comment %} +This is a custom widget for USWDS's comboboxes. +USWDS comboboxes are basically just selects with a "usa-combo-box" div wrapper. +We can further customize these by applying attributes to this parent element, +for now we just carry the attribute to both the parent element and the select. +{% endcomment %} +
Date: Wed, 7 Aug 2024 10:12:18 -0600 Subject: [PATCH 15/61] Reset search logic --- src/registrar/assets/js/get-gov.js | 62 +++++++++++++++++++ src/registrar/assets/sass/_theme/_base.scss | 6 ++ .../templates/domain_suborganization.html | 2 + 3 files changed, 70 insertions(+) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 0712da0f7..b44f13062 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1985,3 +1985,65 @@ document.addEventListener('DOMContentLoaded', function() { showInputOnErrorFields(); })(); + + +/** + * An IIFE that changes the default clear behavior on comboboxes to the input field. + * We want the search bar to act soley as a search bar. + */ +(function loadInitialValuesForComboBoxes() { + document.addEventListener('DOMContentLoaded', (event) => { + // The file location for the #undo svg + const undoIcon = document.querySelector("#uswds-undo-icon-url"); + const undoIconUrl = undoIcon ? undoIcon.getAttribute("data-undo-icon-url") : null; + if (undoIconUrl) { + handleAllComboBoxElements(undoIconUrl); + } + }); + + function handleAllComboBoxElements(undoIconUrl) { + const comboBoxElements = document.querySelectorAll(".usa-combo-box"); + comboBoxElements.forEach(comboBox => { + const input = comboBox.querySelector('input'); + if (!input || !undoIconUrl) { + console.warn("No input element found"); + return; + } + + let clearInputButton = comboBox.querySelector(".usa-combo-box__clear-input"); + if (!clearInputButton) { + console.warn("No clear element found"); + return; + } + + let resetSearchButton = clearInputButton.cloneNode(true); + resetSearchButton.classList.add('usa-combo-box__reset-search'); + resetSearchButton.style.display = 'none'; + // Change the icon to the "undo" icon. Due to the nature of how this element is styled, we have to do this as so. + resetSearchButton.style.backgroundImage = `url("${undoIconUrl}"), linear-gradient(transparent, transparent)`; + clearInputButton.insertAdjacentElement('afterend', resetSearchButton); + + // Show the reset search button when typing + input.addEventListener('input', () => { + resetSearchButton.style.display = 'inline-block'; + }); + + // Hide the reset search button when input loses focus + input.addEventListener('blur', () => { + resetSearchButton.style.display = 'none'; + }); + + handleMouseDownOnButton(resetSearchButton, input) + }); + } + + function handleMouseDownOnButton(button, inputToTarget) { + // Reset the input value when the reset search button is clicked + button.addEventListener('mousedown', (event) => { + // Simulate focus and blur to trigger the built in "resetSelection" and "hideList" functions + inputToTarget.focus(); + inputToTarget.blur(); + button.style.display = 'none'; + }); + } +})(); \ No newline at end of file diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index 9f8a0cbb6..27dac777d 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -192,3 +192,9 @@ abbr[title] { max-width: 50ch; } } + +.usa-combo-box__clear-input { + &.usa-combo-box__clear-input--reset { + + } +} \ No newline at end of file diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index d1fdeac2f..c2e3431ad 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -16,6 +16,8 @@

{% if has_domains_portfolio_permission and request.user.has_edit_suborganization %} + {% comment %} Store the undo icon for reference in js - since this is dynamic {% endcomment %} +
{% csrf_token %} {% input_with_errors form.sub_organization %} From 471e9e5a8fbf192ffe4108e0d289b678e326eba8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 08:15:11 -0600 Subject: [PATCH 16/61] Fix migrations - once again --- ...ioinvitation_portfolio_additional_permissions_and_more.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/registrar/migrations/{0116_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py => 0117_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py} (95%) diff --git a/src/registrar/migrations/0116_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py b/src/registrar/migrations/0117_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py similarity index 95% rename from src/registrar/migrations/0116_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py rename to src/registrar/migrations/0117_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py index 90623bff9..2418c7b95 100644 --- a/src/registrar/migrations/0116_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py +++ b/src/registrar/migrations/0117_alter_portfolioinvitation_portfolio_additional_permissions_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-08-05 14:33 +# Generated by Django 4.2.10 on 2024-08-08 14:14 import django.contrib.postgres.fields from django.db import migrations, models @@ -7,7 +7,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("registrar", "0115_portfolioinvitation"), + ("registrar", "0116_federalagency_initials_federalagency_is_fceb_and_more"), ] operations = [ From 48adabb4ad8cc4d7b3d6826d9ff47e751bd4430c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 11:53:51 -0600 Subject: [PATCH 17/61] Update src/registrar/assets/sass/_theme/_base.scss --- src/registrar/assets/sass/_theme/_base.scss | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index 27dac777d..90ea79909 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -191,10 +191,4 @@ abbr[title] { .ellipsis--desktop-50 { max-width: 50ch; } -} - -.usa-combo-box__clear-input { - &.usa-combo-box__clear-input--reset { - - } } \ No newline at end of file From 21abccc8e52e0e25e506f9cb213431f4d6bde84d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 11:54:10 -0600 Subject: [PATCH 18/61] Update src/registrar/assets/sass/_theme/_base.scss --- src/registrar/assets/sass/_theme/_base.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index 90ea79909..9f8a0cbb6 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -191,4 +191,4 @@ abbr[title] { .ellipsis--desktop-50 { max-width: 50ch; } -} \ No newline at end of file +} From 730677b85e06f11eb188ee817272b01a8faa322f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 11:54:23 -0600 Subject: [PATCH 19/61] Update src/registrar/assets/js/get-gov.js --- src/registrar/assets/js/get-gov.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index b44f13062..bbea5d496 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -2046,4 +2046,4 @@ document.addEventListener('DOMContentLoaded', function() { button.style.display = 'none'; }); } -})(); \ No newline at end of file +})(); From 1dc93f7d0f838f0964067c9e346d69732e077c04 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 15:17:18 -0600 Subject: [PATCH 20/61] Basic form view --- src/registrar/config/urls.py | 5 ++ src/registrar/forms/portfolio.py | 46 ++++++++++++- .../portfolio_organization_sidebar.html | 4 +- .../templates/portfolio_senior_official.html | 56 ++++++++++++++++ src/registrar/views/portfolios.py | 64 ++++++++++++++++++- 5 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 src/registrar/templates/portfolio_senior_official.html diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 90137c4af..93c92f1ca 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -74,6 +74,11 @@ urlpatterns = [ views.PortfolioOrganizationView.as_view(), name="organization", ), + path( + "senior-official/", + views.PortfolioSeniorOfficialView.as_view(), + name="senior-official", + ), path( "admin/logout/", RedirectView.as_view(pattern_name="logout", permanent=False), diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 9362c7bbd..88ec8e3f7 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -4,7 +4,7 @@ import logging from django import forms from django.core.validators import RegexValidator -from ..models import DomainInformation, Portfolio +from ..models import DomainInformation, Portfolio, SeniorOfficial logger = logging.getLogger(__name__) @@ -67,3 +67,47 @@ class PortfolioOrgAddressForm(forms.ModelForm): self.fields[field_name].required = True self.fields["state_territory"].widget.attrs.pop("maxlength", None) self.fields["zipcode"].widget.attrs.pop("maxlength", None) + + +class PortfolioSeniorOfficialForm(forms.ModelForm): + """Form for updating the portfolio senior official.""" + + JOIN = "senior_official" + + class Meta: + model = SeniorOfficial + # TODO - add full name + fields = [ + "first_name", + "last_name", + "title", + "email", + ] + + # error_messages = { + # "address_line1": {"required": "Enter the street address of your organization."}, + # "city": {"required": "Enter the city where your organization is located."}, + # "state_territory": { + # "required": "Select the state, territory, or military post where your organization is located." + # }, + # } + widgets = { + # We need to set the required attributed for State/territory + # because for this fields we are creating an individual + # instance of the Select. For the other fields we use the for loop to set + # the class's required attribute to true. + "first_name": forms.TextInput, + "last_name": forms.TextInput, + "title": forms.TextInput, + "email": forms.TextInput, + } + + # the database fields have blank=True so ModelForm doesn't create + # required fields by default. Use this list in __init__ to mark each + # of these fields as required + required = ["first_name", "last_name", "title", "email"] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + for field_name in self.required: + self.fields[field_name].required = True diff --git a/src/registrar/templates/portfolio_organization_sidebar.html b/src/registrar/templates/portfolio_organization_sidebar.html index cfcdff3a8..cfbb30e91 100644 --- a/src/registrar/templates/portfolio_organization_sidebar.html +++ b/src/registrar/templates/portfolio_organization_sidebar.html @@ -13,7 +13,9 @@
  • - Senior official diff --git a/src/registrar/templates/portfolio_senior_official.html b/src/registrar/templates/portfolio_senior_official.html new file mode 100644 index 000000000..e22237ac4 --- /dev/null +++ b/src/registrar/templates/portfolio_senior_official.html @@ -0,0 +1,56 @@ +{% extends 'portfolio_base.html' %} +{% load static field_helpers%} + +{% block title %}Senior Official | {{ portfolio.name }} | {% endblock %} + +{% load static %} + +{% block portfolio_content %} +
    +
    +

    + Portfolio name: {{ portfolio }} +

    + + {% include 'portfolio_organization_sidebar.html' %} +
    + +
    + +

    Senior Official

    + +

    Your senior official is a person within your organization who can authorize domain requests.

    + +

    The senior official for your organization can’t be updated here. To suggest an update, email help@get.gov

    + + {% if has_edit_org_portfolio_permission %} + {% include "includes/form_errors.html" with form=form %} + {% include "includes/required_fields.html" %} + + {% csrf_token %} + {% input_with_errors form.first_name %} + {% input_with_errors form.last_name %} + {% input_with_errors form.title %} + {% input_with_errors form.email %} + + + {% else %} +

    Full name

    +

    + {{ portfolio.senior_official.get_formatted_name }} +

    + {% if form.city.value is not None %} + {% include "includes/input_read_only.html" with field=form.title %} + {% endif %} + {% if form.state_territory.value is not None %} + {% include "includes/input_read_only.html" with field=form.email %} + {% endif %} + {% endif %} + +
    +
    +{% endblock %} diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 63ebbaa01..0903b2e58 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -3,7 +3,7 @@ from django.http import Http404 from django.shortcuts import render from django.urls import reverse from django.contrib import messages -from registrar.forms.portfolio import PortfolioOrgAddressForm +from registrar.forms.portfolio import PortfolioOrgAddressForm, PortfolioSeniorOfficialForm from registrar.models.portfolio import Portfolio from registrar.views.utility.permission_views import ( PortfolioDomainRequestsPermissionView, @@ -94,3 +94,65 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): def get_success_url(self): """Redirect to the overview page for the portfolio.""" return reverse("organization") + + +class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): + """ + View to handle displaying and updating the portfolio's senior official details. + """ + + model = Portfolio + template_name = "portfolio_senior_official.html" + form_class = PortfolioSeniorOfficialForm + context_object_name = "portfolio" + + def get_context_data(self, **kwargs): + """Add additional context data to the template.""" + context = super().get_context_data(**kwargs) + context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission() + return context + + def get_object(self, queryset=None): + """Get the portfolio object based on the request user.""" + portfolio = self.request.user.portfolio + if portfolio is None: + raise Http404("No organization found for this user") + return portfolio + + def get_form_kwargs(self): + """Include the instance in the form kwargs.""" + kwargs = super().get_form_kwargs() + kwargs["instance"] = self.get_object().senior_official + return kwargs + + def get(self, request, *args, **kwargs): + """Handle GET requests to display the form.""" + self.object = self.get_object() + form = self.get_form() + return self.render_to_response(self.get_context_data(form=form)) + + def post(self, request, *args, **kwargs): + """Handle POST requests to process form submission.""" + self.object = self.get_object() + form = self.get_form() + if form.is_valid(): + return self.form_valid(form) + else: + return self.form_invalid(form) + + def form_valid(self, form): + """Handle the case when the form is valid.""" + senior_official = form.save() + portfolio = self.get_object() + portfolio.senior_official = senior_official + portfolio.save() + messages.success(self.request, "The Senior Official for this portfolio has been updated.") + return super().form_valid(form) + + def form_invalid(self, form): + """Handle the case when the form is invalid.""" + return self.render_to_response(self.get_context_data(form=form)) + + def get_success_url(self): + """Redirect to the overview page for the portfolio.""" + return reverse("senior-official") \ No newline at end of file From a9a93df5cdc2d2138f5883d1baff15ab71f0164d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:00:44 -0600 Subject: [PATCH 21/61] Readonly logic --- src/registrar/forms/portfolio.py | 20 +++++++------- .../templates/portfolio_senior_official.html | 27 ++++++++++--------- src/registrar/views/portfolios.py | 8 +++--- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 88ec8e3f7..936953686 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -70,27 +70,22 @@ class PortfolioOrgAddressForm(forms.ModelForm): class PortfolioSeniorOfficialForm(forms.ModelForm): - """Form for updating the portfolio senior official.""" + """ + Form for updating the portfolio senior official. + This form is readonly for now. + """ JOIN = "senior_official" + full_name = forms.CharField(label="Full name", required=False) class Meta: model = SeniorOfficial - # TODO - add full name fields = [ "first_name", "last_name", "title", "email", ] - - # error_messages = { - # "address_line1": {"required": "Enter the street address of your organization."}, - # "city": {"required": "Enter the city where your organization is located."}, - # "state_territory": { - # "required": "Select the state, territory, or military post where your organization is located." - # }, - # } widgets = { # We need to set the required attributed for State/territory # because for this fields we are creating an individual @@ -100,14 +95,17 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): "last_name": forms.TextInput, "title": forms.TextInput, "email": forms.TextInput, + "full_name": forms.TextInput(attrs={"readonly": "readonly"}) } # the database fields have blank=True so ModelForm doesn't create # required fields by default. Use this list in __init__ to mark each # of these fields as required required = ["first_name", "last_name", "title", "email"] - def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) for field_name in self.required: self.fields[field_name].required = True + + if self.instance: + self.fields["full_name"].initial = self.instance.get_formatted_name() diff --git a/src/registrar/templates/portfolio_senior_official.html b/src/registrar/templates/portfolio_senior_official.html index e22237ac4..9566736a0 100644 --- a/src/registrar/templates/portfolio_senior_official.html +++ b/src/registrar/templates/portfolio_senior_official.html @@ -23,9 +23,7 @@

    Your senior official is a person within your organization who can authorize domain requests.

    -

    The senior official for your organization can’t be updated here. To suggest an update, email help@get.gov

    - - {% if has_edit_org_portfolio_permission %} + {% if santa_exists %} {% include "includes/form_errors.html" with form=form %} {% include "includes/required_fields.html" %}
    @@ -39,15 +37,20 @@
    {% else %} -

    Full name

    -

    - {{ portfolio.senior_official.get_formatted_name }} -

    - {% if form.city.value is not None %} - {% include "includes/input_read_only.html" with field=form.title %} - {% endif %} - {% if form.state_territory.value is not None %} - {% include "includes/input_read_only.html" with field=form.email %} +

    The senior official for your organization can’t be updated here. To suggest an update, email help@get.gov

    + + {% if not senior_official %} +

    No senior official was found for this portfolio.

    + {% else %} + {% if form.full_name.value is not None %} + {% include "includes/input_read_only.html" with field=form.full_name %} + {% endif %} + {% if form.title.value is not None %} + {% include "includes/input_read_only.html" with field=form.title %} + {% endif %} + {% if form.email.value is not None %} + {% include "includes/input_read_only.html" with field=form.email %} + {% endif %} {% endif %} {% endif %} diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 0903b2e58..3c851841a 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -48,7 +48,7 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): def get_context_data(self, **kwargs): """Add additional context data to the template.""" context = super().get_context_data(**kwargs) - context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission() + # context["has_edit_org_portfolio_permission"] return context def get_object(self, queryset=None): @@ -109,7 +109,7 @@ class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): def get_context_data(self, **kwargs): """Add additional context data to the template.""" context = super().get_context_data(**kwargs) - context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission() + context["senior_official"] = self.get_object().senior_official return context def get_object(self, queryset=None): @@ -131,6 +131,8 @@ class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): form = self.get_form() return self.render_to_response(self.get_context_data(form=form)) + # These functions are included for future compatibility, but for now + # we do not offer an edit mode for senior officials. def post(self, request, *args, **kwargs): """Handle POST requests to process form submission.""" self.object = self.get_object() @@ -155,4 +157,4 @@ class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): def get_success_url(self): """Redirect to the overview page for the portfolio.""" - return reverse("senior-official") \ No newline at end of file + return reverse("senior-official") From 3837359977df8425b16742bacde154f2a52a0e14 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:09:17 -0600 Subject: [PATCH 22/61] Fix bug with load senior official table --- src/registrar/forms/portfolio.py | 2 +- .../commands/load_senior_official_table.py | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 936953686..d807b4184 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -106,6 +106,6 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): super().__init__(*args, **kwargs) for field_name in self.required: self.fields[field_name].required = True - + if self.instance: self.fields["full_name"].initial = self.instance.get_formatted_name() diff --git a/src/registrar/management/commands/load_senior_official_table.py b/src/registrar/management/commands/load_senior_official_table.py index cbfe479ea..43f61d57a 100644 --- a/src/registrar/management/commands/load_senior_official_table.py +++ b/src/registrar/management/commands/load_senior_official_table.py @@ -35,7 +35,6 @@ class Command(BaseCommand): Note: - If the row is missing SO data - it will not be added. - - Given we can add the row, any blank first_name will be replaced with "-". """, # noqa: W291 prompt_title="Do you wish to load records into the SeniorOfficial table?", ) @@ -64,7 +63,11 @@ class Command(BaseCommand): # Clean the returned data for key, value in so_kwargs.items(): if isinstance(value, str): - so_kwargs[key] = value.strip() + clean_string = value.strip() + if clean_string: + so_kwargs[key] = clean_string + else: + so_kwargs[key] = None # Handle the federal_agency record seperately (db call) agency_name = row.get("Agency").strip() if row.get("Agency") else None @@ -95,17 +98,11 @@ class Command(BaseCommand): def create_senior_official(self, so_kwargs): """Creates a senior official object from kwargs but does not add it to the DB""" - # WORKAROUND: Placeholder value for first name, - # as not having these makes it impossible to access through DJA. - old_first_name = so_kwargs["first_name"] - if not so_kwargs["first_name"]: - so_kwargs["first_name"] = "-" - # Create a new SeniorOfficial object new_so = SeniorOfficial(**so_kwargs) # Store a variable for the console logger - if all([old_first_name, new_so.last_name]): + if all([new_so.first_name, new_so.last_name]): record_display = new_so else: record_display = so_kwargs From 0218da50d8a449ad624753f10c17fe04ac818e7b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:22:41 -0600 Subject: [PATCH 23/61] Cleanup unused content --- src/registrar/forms/portfolio.py | 6 --- .../templates/portfolio_senior_official.html | 39 ++++++------------- src/registrar/views/portfolios.py | 29 +------------- 3 files changed, 13 insertions(+), 61 deletions(-) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index d807b4184..92b68c86a 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -98,14 +98,8 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): "full_name": forms.TextInput(attrs={"readonly": "readonly"}) } - # the database fields have blank=True so ModelForm doesn't create - # required fields by default. Use this list in __init__ to mark each - # of these fields as required - required = ["first_name", "last_name", "title", "email"] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - for field_name in self.required: - self.fields[field_name].required = True if self.instance: self.fields["full_name"].initial = self.instance.get_formatted_name() diff --git a/src/registrar/templates/portfolio_senior_official.html b/src/registrar/templates/portfolio_senior_official.html index 9566736a0..12ddab908 100644 --- a/src/registrar/templates/portfolio_senior_official.html +++ b/src/registrar/templates/portfolio_senior_official.html @@ -23,34 +23,19 @@

    Your senior official is a person within your organization who can authorize domain requests.

    - {% if santa_exists %} - {% include "includes/form_errors.html" with form=form %} - {% include "includes/required_fields.html" %} -
    - {% csrf_token %} - {% input_with_errors form.first_name %} - {% input_with_errors form.last_name %} - {% input_with_errors form.title %} - {% input_with_errors form.email %} - -
    +

    The senior official for your organization can’t be updated here. To suggest an update, email help@get.gov

    + + {% if not senior_official %} +

    No senior official was found for this portfolio.

    {% else %} -

    The senior official for your organization can’t be updated here. To suggest an update, email help@get.gov

    - - {% if not senior_official %} -

    No senior official was found for this portfolio.

    - {% else %} - {% if form.full_name.value is not None %} - {% include "includes/input_read_only.html" with field=form.full_name %} - {% endif %} - {% if form.title.value is not None %} - {% include "includes/input_read_only.html" with field=form.title %} - {% endif %} - {% if form.email.value is not None %} - {% include "includes/input_read_only.html" with field=form.email %} - {% endif %} + {% if form.full_name.value is not None %} + {% include "includes/input_read_only.html" with field=form.full_name %} + {% endif %} + {% if form.title.value is not None %} + {% include "includes/input_read_only.html" with field=form.title %} + {% endif %} + {% if form.email.value is not None %} + {% include "includes/input_read_only.html" with field=form.email %} {% endif %} {% endif %} diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 3c851841a..8879f020c 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -99,6 +99,7 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): """ View to handle displaying and updating the portfolio's senior official details. + For now, this view is readonly. """ model = Portfolio @@ -130,31 +131,3 @@ class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): self.object = self.get_object() form = self.get_form() return self.render_to_response(self.get_context_data(form=form)) - - # These functions are included for future compatibility, but for now - # we do not offer an edit mode for senior officials. - def post(self, request, *args, **kwargs): - """Handle POST requests to process form submission.""" - self.object = self.get_object() - form = self.get_form() - if form.is_valid(): - return self.form_valid(form) - else: - return self.form_invalid(form) - - def form_valid(self, form): - """Handle the case when the form is valid.""" - senior_official = form.save() - portfolio = self.get_object() - portfolio.senior_official = senior_official - portfolio.save() - messages.success(self.request, "The Senior Official for this portfolio has been updated.") - return super().form_valid(form) - - def form_invalid(self, form): - """Handle the case when the form is invalid.""" - return self.render_to_response(self.get_context_data(form=form)) - - def get_success_url(self): - """Redirect to the overview page for the portfolio.""" - return reverse("senior-official") From 956e27cc77894f097fd673a942032b917e2f7b34 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 10:24:45 -0600 Subject: [PATCH 24/61] Update portfolio.py --- src/registrar/forms/portfolio.py | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 92b68c86a..ab9459588 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -76,30 +76,15 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): """ JOIN = "senior_official" - full_name = forms.CharField(label="Full name", required=False) - + full_name = forms.CharField(label="Full name") class Meta: model = SeniorOfficial fields = [ - "first_name", - "last_name", "title", "email", ] - widgets = { - # We need to set the required attributed for State/territory - # because for this fields we are creating an individual - # instance of the Select. For the other fields we use the for loop to set - # the class's required attribute to true. - "first_name": forms.TextInput, - "last_name": forms.TextInput, - "title": forms.TextInput, - "email": forms.TextInput, - "full_name": forms.TextInput(attrs={"readonly": "readonly"}) - } def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - if self.instance: self.fields["full_name"].initial = self.instance.get_formatted_name() From 5b541b8c039bee02e84341eee35136bf34cf184c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 11:44:59 -0600 Subject: [PATCH 25/61] Custom clear button logic --- src/registrar/assets/js/get-gov.js | 90 +++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index bbea5d496..0bebb4d9b 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1992,6 +1992,9 @@ document.addEventListener('DOMContentLoaded', function() { * We want the search bar to act soley as a search bar. */ (function loadInitialValuesForComboBoxes() { + var overrideDefaultClearButton = true; + var isTyping = false; + document.addEventListener('DOMContentLoaded', (event) => { // The file location for the #undo svg const undoIcon = document.querySelector("#uswds-undo-icon-url"); @@ -2005,10 +2008,13 @@ document.addEventListener('DOMContentLoaded', function() { const comboBoxElements = document.querySelectorAll(".usa-combo-box"); comboBoxElements.forEach(comboBox => { const input = comboBox.querySelector('input'); - if (!input || !undoIconUrl) { - console.warn("No input element found"); + const select = comboBox.querySelector("select"); + if (!input || !undoIconUrl || !select) { + console.warn("No combobox element found"); return; } + // Set the initial value of the combobox + let initialValue = select.getAttribute("data-default-value"); let clearInputButton = comboBox.querySelector(".usa-combo-box__clear-input"); if (!clearInputButton) { @@ -2016,34 +2022,76 @@ document.addEventListener('DOMContentLoaded', function() { return; } - let resetSearchButton = clearInputButton.cloneNode(true); - resetSearchButton.classList.add('usa-combo-box__reset-search'); - resetSearchButton.style.display = 'none'; - // Change the icon to the "undo" icon. Due to the nature of how this element is styled, we have to do this as so. - resetSearchButton.style.backgroundImage = `url("${undoIconUrl}"), linear-gradient(transparent, transparent)`; - clearInputButton.insertAdjacentElement('afterend', resetSearchButton); + // Override the default clear button behavior such that it no longer clears the input, + // it just resets to the data-initial-value. - // Show the reset search button when typing + // Due to the nature of how uswds works, this is slightly hacky. + + // Use a MutationObserver to watch for changes in the dropdown list + const dropdownList = document.querySelector(`#${input.id}--list`); + const observer = new MutationObserver(function(mutations) { + mutations.forEach(function(mutation) { + if (mutation.type === "childList") { + addBlankOption(clearInputButton, dropdownList, initialValue); + } + }); + }); + + // Configure the observer to watch for changes in the dropdown list + const config = { childList: true, subtree: true }; + observer.observe(dropdownList, config); + + // Add input event listener to detect typing input.addEventListener('input', () => { - resetSearchButton.style.display = 'inline-block'; + isTyping = true; }); - // Hide the reset search button when input loses focus + // Add blur event listener to reset typing state input.addEventListener('blur', () => { - resetSearchButton.style.display = 'none'; + isTyping = false; }); - handleMouseDownOnButton(resetSearchButton, input) + // Change the default input behaviour - have it reset to the data default instead + clearInputButton.addEventListener("click", (e) => { + if (overrideDefaultClearButton && initialValue) { + e.preventDefault(); + e.stopPropagation(); + input.click(); + // Find the dropdown option with the desired value + const dropdownOptions = document.querySelectorAll(".usa-combo-box__list-option"); + if (dropdownOptions) { + dropdownOptions.forEach(option => { + if (option.getAttribute("data-value") === initialValue) { + // Simulate a click event on the dropdown option + option.click(); + } + }); + } + } + }); }); } - function handleMouseDownOnButton(button, inputToTarget) { - // Reset the input value when the reset search button is clicked - button.addEventListener('mousedown', (event) => { - // Simulate focus and blur to trigger the built in "resetSelection" and "hideList" functions - inputToTarget.focus(); - inputToTarget.blur(); - button.style.display = 'none'; - }); + function addBlankOption(clearInputButton, dropdownList, initialValue) { + if (dropdownList && !dropdownList.querySelector('[data-value=""]') && !isTyping) { + const blankOption = document.createElement("li"); + blankOption.setAttribute("role", "option"); + blankOption.setAttribute("data-value", ""); + blankOption.classList.add("usa-combo-box__list-option"); + if (!initialValue){ + blankOption.classList.add("usa-combo-box__list-option--selected") + } + blankOption.textContent = "---------"; + + dropdownList.insertBefore(blankOption, dropdownList.firstChild); + blankOption.addEventListener("click", (e) => { + e.preventDefault(); + e.stopPropagation(); + overrideDefaultClearButton = false; + // Trigger the default clear behavior + clearInputButton.click(); + overrideDefaultClearButton = true; + }); + } } })(); From e017315638e790c1dbf725d8af8c98a3b13025a4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 13:11:48 -0600 Subject: [PATCH 26/61] Modify existing --- .../templates/domain_senior_official.html | 24 ++++++------------- .../includes/readonly_senior_official.html | 11 +++++++++ .../templates/portfolio_senior_official.html | 10 +------- src/registrar/views/portfolios.py | 2 +- 4 files changed, 20 insertions(+), 27 deletions(-) create mode 100644 src/registrar/templates/includes/readonly_senior_official.html diff --git a/src/registrar/templates/domain_senior_official.html b/src/registrar/templates/domain_senior_official.html index 5d13e28e7..c911af1b9 100644 --- a/src/registrar/templates/domain_senior_official.html +++ b/src/registrar/templates/domain_senior_official.html @@ -26,23 +26,13 @@ {% csrf_token %} {% if generic_org_type == "federal" or generic_org_type == "tribal" %} - {# If all fields are disabled, add SR content #} -
    {{ form.first_name.value }}
    -
    {{ form.last_name.value }}
    -
    {{ form.title.value }}
    -
    {{ form.email.value }}
    - {% endif %} - - {% input_with_errors form.first_name %} - - {% input_with_errors form.last_name %} - - {% input_with_errors form.title %} - - {% input_with_errors form.email %} - - {% if generic_org_type != "federal" and generic_org_type != "tribal" %} - + {% include "includes/readonly_senior_official.html" with form=form senior_official=senior_officialn%} + {% else %} + {% input_with_errors form.first_name %} + {% input_with_errors form.last_name %} + {% input_with_errors form.title %} + {% input_with_errors form.email %} + {% endif %} {% endblock %} {# domain_content #} diff --git a/src/registrar/templates/includes/readonly_senior_official.html b/src/registrar/templates/includes/readonly_senior_official.html new file mode 100644 index 000000000..73e7bfd77 --- /dev/null +++ b/src/registrar/templates/includes/readonly_senior_official.html @@ -0,0 +1,11 @@ +{% if form.full_name.value is not None %} + {% include "includes/input_read_only.html" with field=form.full_name %} +{% endif %} + +{% if form.title.value is not None %} + {% include "includes/input_read_only.html" with field=form.title %} +{% endif %} + +{% if form.email.value is not None %} + {% include "includes/input_read_only.html" with field=form.email %} +{% endif %} \ No newline at end of file diff --git a/src/registrar/templates/portfolio_senior_official.html b/src/registrar/templates/portfolio_senior_official.html index 12ddab908..976cf9639 100644 --- a/src/registrar/templates/portfolio_senior_official.html +++ b/src/registrar/templates/portfolio_senior_official.html @@ -28,15 +28,7 @@ {% if not senior_official %}

    No senior official was found for this portfolio.

    {% else %} - {% if form.full_name.value is not None %} - {% include "includes/input_read_only.html" with field=form.full_name %} - {% endif %} - {% if form.title.value is not None %} - {% include "includes/input_read_only.html" with field=form.title %} - {% endif %} - {% if form.email.value is not None %} - {% include "includes/input_read_only.html" with field=form.email %} - {% endif %} + {% include "includes/readonly_senior_official.html" with form=form %} {% endif %}
    • diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 8879f020c..a7c7d1356 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -48,7 +48,7 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): def get_context_data(self, **kwargs): """Add additional context data to the template.""" context = super().get_context_data(**kwargs) - # context["has_edit_org_portfolio_permission"] + context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission() return context def get_object(self, queryset=None): From cdcd90bb89706776399384e6430b2e5f7bc89a73 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Fri, 9 Aug 2024 14:37:49 -0500 Subject: [PATCH 27/61] add dnssec summary item to overview page --- src/registrar/templates/domain_detail.html | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index 0b6f47481..ad187aa4e 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -64,6 +64,13 @@ {% endif %} {% endif %} + {% url 'domain-dns-dnssec' pk=domain.id as url %} + {% if domain.dnssecdata is not None %} + {% include "includes/summary_item.html" with title='DNSSEC' value='Enabled' edit_link=url editable=is_editable %} + {% else %} + {% include "includes/summary_item.html" with title='DNSSEC' value='Disabled' edit_link=url editable=is_editable %} + {% endif %} + {% if portfolio %} {% comment %} TODO - uncomment in #2352 and add to edit_link {% url 'domain-suborganization' pk=domain.id as url %} From 2863e29d96892dd076ffa572ada9eae8162821d0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 13:40:16 -0600 Subject: [PATCH 28/61] Fix bug with senior_official in django admin, add readonly --- src/registrar/admin.py | 2 -- src/registrar/forms/domain.py | 5 ++++- src/registrar/forms/portfolio.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ca4038d51..138183082 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -504,8 +504,6 @@ class AdminSortFields: # == Contact == # "other_contacts": (Contact, _name_sort), "submitter": (Contact, _name_sort), - # == Senior Official == # - "senior_official": (SeniorOfficial, _name_sort), # == User == # "creator": (User, _name_sort), "user": (User, _name_sort), diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 02a0724d1..53f340aee 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -321,9 +321,12 @@ class SeniorOfficialContactForm(ContactForm): """Form for updating senior official contacts.""" JOIN = "senior_official" - + full_name = forms.CharField(label="Full name", required=False) def __init__(self, disable_fields=False, *args, **kwargs): super().__init__(*args, **kwargs) + + if self.instance: + self.fields["full_name"].initial = self.instance.get_formatted_name() # Overriding bc phone not required in this form self.fields["phone"] = forms.IntegerField(required=False) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index ab9459588..3a9e65b7f 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -76,7 +76,7 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): """ JOIN = "senior_official" - full_name = forms.CharField(label="Full name") + full_name = forms.CharField(label="Full name", required=False) class Meta: model = SeniorOfficial fields = [ From 4be1e3e29e2140ebecfd3c08885de00ad7a95d7e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 13:53:54 -0600 Subject: [PATCH 29/61] Linting and fix existing unit test --- src/registrar/admin.py | 2 +- src/registrar/forms/domain.py | 3 +- src/registrar/forms/portfolio.py | 1 + .../includes/readonly_senior_official.html | 2 +- src/registrar/tests/test_views_domain.py | 45 +++---------------- 5 files changed, 11 insertions(+), 42 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 138183082..ae06a8d96 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,7 +22,7 @@ from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial +from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 53f340aee..4a1e95431 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -322,9 +322,10 @@ class SeniorOfficialContactForm(ContactForm): JOIN = "senior_official" full_name = forms.CharField(label="Full name", required=False) + def __init__(self, disable_fields=False, *args, **kwargs): super().__init__(*args, **kwargs) - + if self.instance: self.fields["full_name"].initial = self.instance.get_formatted_name() diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 3a9e65b7f..44d195dd6 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -77,6 +77,7 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): JOIN = "senior_official" full_name = forms.CharField(label="Full name", required=False) + class Meta: model = SeniorOfficial fields = [ diff --git a/src/registrar/templates/includes/readonly_senior_official.html b/src/registrar/templates/includes/readonly_senior_official.html index 73e7bfd77..7a7908726 100644 --- a/src/registrar/templates/includes/readonly_senior_official.html +++ b/src/registrar/templates/includes/readonly_senior_official.html @@ -8,4 +8,4 @@ {% if form.email.value is not None %} {% include "includes/input_read_only.html" with field=form.email %} -{% endif %} \ No newline at end of file +{% endif %} diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 31ca776e2..efbc1296c 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1192,14 +1192,14 @@ class TestDomainSeniorOfficial(TestDomainOverview): self.assertTrue("disabled" in form[field_name].attrs) @less_console_noise_decorator - def test_domain_edit_senior_official_federal(self): + def test_domain_cannot_edit_senior_official_when_federal(self): """Tests that no edit can occur when the underlying domain is federal""" # Set the org type to federal self.domain_information.generic_org_type = DomainInformation.OrganizationChoices.FEDERAL self.domain_information.save() - # Add an SO. We can do this at the model level, just not the form level. + # Add an SO self.domain_information.senior_official = Contact( first_name="Apple", last_name="Tester", title="CIO", email="nobody@igorville.gov" ) @@ -1210,43 +1210,10 @@ class TestDomainSeniorOfficial(TestDomainOverview): session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # Test if the form is populating data correctly - so_form = so_page.forms[0] - - test_cases = [ - ("first_name", "Apple"), - ("last_name", "Tester"), - ("title", "CIO"), - ("email", "nobody@igorville.gov"), - ] - self.assert_all_form_fields_have_expected_values(so_form, test_cases, test_for_disabled=True) - - # Attempt to change data on each field. Because this domain is federal, - # this should not succeed. - so_form["first_name"] = "Orange" - so_form["last_name"] = "Smoothie" - so_form["title"] = "Cat" - so_form["email"] = "somebody@igorville.gov" - - submission = so_form.submit() - - # A 302 indicates this page underwent a redirect. - self.assertEqual(submission.status_code, 302) - - followed_submission = submission.follow() - - # Test the returned form for data accuracy. These values should be unchanged. - new_form = followed_submission.forms[0] - self.assert_all_form_fields_have_expected_values(new_form, test_cases, test_for_disabled=True) - - # refresh domain information. Test these values in the DB. - self.domain_information.refresh_from_db() - - # All values should be unchanged. These are defined manually for code clarity. - self.assertEqual("Apple", self.domain_information.senior_official.first_name) - self.assertEqual("Tester", self.domain_information.senior_official.last_name) - self.assertEqual("CIO", self.domain_information.senior_official.title) - self.assertEqual("nobody@igorville.gov", self.domain_information.senior_official.email) + self.assertContains(so_page, "Apple Tester") + self.assertContains(so_page, "CIO") + self.assertContains(so_page, "nobody@igorville.gov") + self.assertNotContains(so_page, "Save") @less_console_noise_decorator def test_domain_edit_senior_official_tribal(self): From 3005158b82fc563f3e94dc7e94e1351772971241 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 14:05:11 -0600 Subject: [PATCH 30/61] Add unit test --- src/registrar/tests/test_views_portfolio.py | 31 ++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index f72130d94..d725c95ad 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -1,7 +1,7 @@ from django.urls import reverse from api.tests.common import less_console_noise_decorator from registrar.config import settings -from registrar.models.portfolio import Portfolio +from registrar.models import Portfolio, SeniorOfficial from django_webtest import WebTest # type: ignore from registrar.models import ( DomainRequest, @@ -38,6 +38,35 @@ class TestPortfolio(WebTest): User.objects.all().delete() super().tearDown() + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_portfolio_senior_official(self): + """Tests the senior official page on portfolio""" + self.app.set_user(self.user.username) + + so = SeniorOfficial.objects.create( + first_name="Saturn", last_name="Enceladus", title="Planet/Moon", email="spacedivision@igorville.com" + ) + + self.portfolio.senior_official = so + self.portfolio.save() + self.portfolio.refresh_from_db() + + self.user.portfolio = self.portfolio + self.user.portfolio_additional_permissions = [UserPortfolioPermissionChoices.VIEW_PORTFOLIO] + self.user.save() + self.user.refresh_from_db() + + so_portfolio_page = self.app.get(reverse("senior-official")) + # Assert that we're on the right page + self.assertContains(so_portfolio_page, "Senior official") + self.assertContains(so_portfolio_page, "Saturn Enceladus") + self.assertContains(so_portfolio_page, "Planet/Moon") + self.assertContains(so_portfolio_page, "spacedivision@igorville.com") + + self.portfolio.delete() + so.delete() + @less_console_noise_decorator def test_middleware_does_not_redirect_if_no_permission(self): """Test that user with no portfolio permission is not redirected when attempting to access home""" From 38e1c79e751d0d8266c3531b41383d8fce6379d0 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Fri, 9 Aug 2024 15:10:47 -0500 Subject: [PATCH 31/61] Additional required changes --- src/registrar/templates/domain_base.html | 2 +- src/registrar/templates/domain_detail.html | 8 ++++---- src/registrar/templates/includes/summary_item.html | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/templates/domain_base.html b/src/registrar/templates/domain_base.html index 9a869ef42..b99b12740 100644 --- a/src/registrar/templates/domain_base.html +++ b/src/registrar/templates/domain_base.html @@ -54,7 +54,7 @@ {% block domain_content %} -

    {{ domain.name }}

    +

    Domain Overview

    {% endblock %} {# domain_content #} {% endif %} diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index ad187aa4e..312c35f40 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -4,7 +4,7 @@ {% block domain_content %} {{ block.super }}
    - +

    {{ domain.name }}

    DNS name servers +

    DNS name servers

    No DNS name servers have been added yet. Before your domain can be used we’ll need information about your domain name servers.

    Add DNS name servers {% else %} {% include "includes/summary_item.html" with title='DNS name servers' domains='true' value='' edit_link=url editable=is_editable %} {% endif %} {% endif %} - + {% url 'domain-dns-dnssec' pk=domain.id as url %} {% if domain.dnssecdata is not None %} {% include "includes/summary_item.html" with title='DNSSEC' value='Enabled' edit_link=url editable=is_editable %} {% else %} - {% include "includes/summary_item.html" with title='DNSSEC' value='Disabled' edit_link=url editable=is_editable %} + {% include "includes/summary_item.html" with title='DNSSEC' value='Not Enabled' edit_link=url editable=is_editable %} {% endif %} {% if portfolio %} diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 6ec69b770..ff42c2eaf 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -7,7 +7,7 @@ {% if heading_level %} <{{ heading_level }} {% else %} -

    {{ sub_header_text }}

    From 5e7a7d94d59831bfbeecc5dd00271cccc65183b3 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Fri, 9 Aug 2024 15:20:32 -0500 Subject: [PATCH 32/61] change subheader text to h4 --- src/registrar/templates/includes/summary_item.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index ff42c2eaf..f69bbaf96 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -22,7 +22,7 @@ {% endif %} {% if sub_header_text %} -

    {{ sub_header_text }}

    +

    {{ sub_header_text }}

    {% endif %} {% if address %} {% include "includes/organization_address.html" with organization=value %} From 727362014161e50e96bff0637f540e7efd3c4293 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 9 Aug 2024 14:28:27 -0600 Subject: [PATCH 33/61] Use same template for both views --- .../templates/domain_senior_official.html | 32 ++----------- .../includes/readonly_senior_official.html | 11 ----- .../templates/includes/senior_official.html | 47 +++++++++++++++++++ .../templates/portfolio_senior_official.html | 14 +----- src/registrar/views/portfolios.py | 6 --- 5 files changed, 51 insertions(+), 59 deletions(-) delete mode 100644 src/registrar/templates/includes/readonly_senior_official.html create mode 100644 src/registrar/templates/includes/senior_official.html diff --git a/src/registrar/templates/domain_senior_official.html b/src/registrar/templates/domain_senior_official.html index c911af1b9..e51bf255c 100644 --- a/src/registrar/templates/domain_senior_official.html +++ b/src/registrar/templates/domain_senior_official.html @@ -4,35 +4,9 @@ {% block title %}Senior official | {{ domain.name }} | {% endblock %} {% block domain_content %} - {# this is right after the messages block in the parent template #} - {% include "includes/form_errors.html" with form=form %} - -

    Senior official

    - -

    Your senior official is a person within your organization who can - authorize domain requests. This person must be in a role of significant, executive responsibility within the organization. Read more about who can serve as a senior official.

    - {% if generic_org_type == "federal" or generic_org_type == "tribal" %} -

    - The senior official for your organization can’t be updated here. - To suggest an update, email help@get.gov. -

    + {% include "includes/senior_official.html" with can_edit=False %} {% else %} - {% include "includes/required_fields.html" %} + {% include "includes/senior_official.html" with can_edit=True %} {% endif %} - - -
    - {% csrf_token %} - - {% if generic_org_type == "federal" or generic_org_type == "tribal" %} - {% include "includes/readonly_senior_official.html" with form=form senior_official=senior_officialn%} - {% else %} - {% input_with_errors form.first_name %} - {% input_with_errors form.last_name %} - {% input_with_errors form.title %} - {% input_with_errors form.email %} - - {% endif %} -
    -{% endblock %} {# domain_content #} +{% endblock %} diff --git a/src/registrar/templates/includes/readonly_senior_official.html b/src/registrar/templates/includes/readonly_senior_official.html deleted file mode 100644 index 7a7908726..000000000 --- a/src/registrar/templates/includes/readonly_senior_official.html +++ /dev/null @@ -1,11 +0,0 @@ -{% if form.full_name.value is not None %} - {% include "includes/input_read_only.html" with field=form.full_name %} -{% endif %} - -{% if form.title.value is not None %} - {% include "includes/input_read_only.html" with field=form.title %} -{% endif %} - -{% if form.email.value is not None %} - {% include "includes/input_read_only.html" with field=form.email %} -{% endif %} diff --git a/src/registrar/templates/includes/senior_official.html b/src/registrar/templates/includes/senior_official.html new file mode 100644 index 000000000..c63493ae8 --- /dev/null +++ b/src/registrar/templates/includes/senior_official.html @@ -0,0 +1,47 @@ +{% load static field_helpers url_helpers %} + +{% if can_edit %} + {% include "includes/form_errors.html" with form=form %} +{% endif %} + +

    Senior Official

    + +

    + Your senior official is a person within your organization who can authorize domain requests. + This person must be in a role of significant, executive responsibility within the organization. + Read more about who can serve as a senior official. +

    + +{% if can_edit %} + {% include "includes/required_fields.html" %} +{% else %} +

    + The senior official for your organization can’t be updated here. + To suggest an update, email help@get.gov +

    +{% endif %} + +{% if can_edit %} +
    + {% csrf_token %} + {% input_with_errors form.first_name %} + {% input_with_errors form.last_name %} + {% input_with_errors form.title %} + {% input_with_errors form.email %} + +
    +{% elif not form.full_name.value and not form.title.value and not form.email.value %} +

    No senior official was found.

    +{% else %} + {% if form.full_name.value is not None %} + {% include "includes/input_read_only.html" with field=form.full_name %} + {% endif %} + + {% if form.title.value is not None %} + {% include "includes/input_read_only.html" with field=form.title %} + {% endif %} + + {% if form.email.value is not None %} + {% include "includes/input_read_only.html" with field=form.email %} + {% endif %} +{% endif %} \ No newline at end of file diff --git a/src/registrar/templates/portfolio_senior_official.html b/src/registrar/templates/portfolio_senior_official.html index 976cf9639..5c47e1645 100644 --- a/src/registrar/templates/portfolio_senior_official.html +++ b/src/registrar/templates/portfolio_senior_official.html @@ -18,19 +18,7 @@
    - -

    Senior Official

    - -

    Your senior official is a person within your organization who can authorize domain requests.

    - -

    The senior official for your organization can’t be updated here. To suggest an update, email help@get.gov

    - - {% if not senior_official %} -

    No senior official was found for this portfolio.

    - {% else %} - {% include "includes/readonly_senior_official.html" with form=form %} - {% endif %} - + {% include "includes/senior_official.html" with can_edit=False %}
    {% endblock %} diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index b9042d617..8a5321cc9 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -110,12 +110,6 @@ class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): form_class = PortfolioSeniorOfficialForm context_object_name = "portfolio" - def get_context_data(self, **kwargs): - """Add additional context data to the template.""" - context = super().get_context_data(**kwargs) - context["senior_official"] = self.get_object().senior_official - return context - def get_object(self, queryset=None): """Get the portfolio object based on the request user.""" portfolio = self.request.user.portfolio From de50972ff1e29c835ee81cfe3f93100a5edd7baa Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:40:31 -0400 Subject: [PATCH 34/61] Update domain_invitation.txt --- src/registrar/templates/emails/domain_invitation.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/domain_invitation.txt b/src/registrar/templates/emails/domain_invitation.txt index e426ae6ef..068040205 100644 --- a/src/registrar/templates/emails/domain_invitation.txt +++ b/src/registrar/templates/emails/domain_invitation.txt @@ -38,5 +38,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) {% endautoescape %} From d24c7f64604c03493aa76204da11de743efdf4f2 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:42:11 -0400 Subject: [PATCH 35/61] Update transition_domain_invitation.txt --- src/registrar/templates/emails/transition_domain_invitation.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/transition_domain_invitation.txt b/src/registrar/templates/emails/transition_domain_invitation.txt index bdce7d107..b6773d9e9 100644 --- a/src/registrar/templates/emails/transition_domain_invitation.txt +++ b/src/registrar/templates/emails/transition_domain_invitation.txt @@ -60,5 +60,5 @@ The .gov team Domain management Get.gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) {% endautoescape %} From 5cd0900fe1e2dea415bb79b0e340c14270351706 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:42:32 -0400 Subject: [PATCH 36/61] Update questionable_senior_official.txt --- .../action_needed_reasons/questionable_senior_official.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official.txt b/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official.txt index 94e15b78c..e20e4cb60 100644 --- a/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official.txt +++ b/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official.txt @@ -32,5 +32,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) -{% endautoescape %} \ No newline at end of file +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +{% endautoescape %} From 411b1b655739d1226d78278fc49b5dc717e366e4 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:44:58 -0400 Subject: [PATCH 37/61] Update status_change_rejected.txt --- src/registrar/templates/emails/status_change_rejected.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/status_change_rejected.txt b/src/registrar/templates/emails/status_change_rejected.txt index 12693deb9..2fcbb1d83 100644 --- a/src/registrar/templates/emails/status_change_rejected.txt +++ b/src/registrar/templates/emails/status_change_rejected.txt @@ -77,5 +77,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) {% endautoescape %} From 5ce926f84badebeb3af5efe562ee42a215b1ec2c Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:45:56 -0400 Subject: [PATCH 38/61] Update eligibility_unclear.txt --- .../emails/action_needed_reasons/eligibility_unclear.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear.txt b/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear.txt index 280321045..d3a986183 100644 --- a/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear.txt +++ b/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear.txt @@ -31,5 +31,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) -{% endautoescape %} \ No newline at end of file +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +{% endautoescape %} From 77499cdc384fe6cc3292ec901e6aa798da5f577d Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:47:53 -0400 Subject: [PATCH 39/61] Update already_has_domains.txt --- .../emails/action_needed_reasons/already_has_domains.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/emails/action_needed_reasons/already_has_domains.txt b/src/registrar/templates/emails/action_needed_reasons/already_has_domains.txt index 264fe265b..b1b3b0a1c 100644 --- a/src/registrar/templates/emails/action_needed_reasons/already_has_domains.txt +++ b/src/registrar/templates/emails/action_needed_reasons/already_has_domains.txt @@ -47,5 +47,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) -{% endautoescape %} \ No newline at end of file +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +{% endautoescape %} From f8c20946d0b5b6f86da5db1e16f3f22754f99eab Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:48:41 -0400 Subject: [PATCH 40/61] Update submission_confirmation.txt --- src/registrar/templates/emails/submission_confirmation.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/submission_confirmation.txt b/src/registrar/templates/emails/submission_confirmation.txt index 2dd4387a4..740e6f393 100644 --- a/src/registrar/templates/emails/submission_confirmation.txt +++ b/src/registrar/templates/emails/submission_confirmation.txt @@ -38,5 +38,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) {% endautoescape %} From c1c89f1c57b37b28982073c0361318f3b79d9881 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:49:03 -0400 Subject: [PATCH 41/61] Update bad_name.txt --- .../templates/emails/action_needed_reasons/bad_name.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/emails/action_needed_reasons/bad_name.txt b/src/registrar/templates/emails/action_needed_reasons/bad_name.txt index 95967639c..7d088aa4e 100644 --- a/src/registrar/templates/emails/action_needed_reasons/bad_name.txt +++ b/src/registrar/templates/emails/action_needed_reasons/bad_name.txt @@ -30,5 +30,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) -{% endautoescape %} \ No newline at end of file +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +{% endautoescape %} From 6c73175c802e382b63f71ac5af1ee761b06390a4 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:49:22 -0400 Subject: [PATCH 42/61] Update status_change_approved.txt --- src/registrar/templates/emails/status_change_approved.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/status_change_approved.txt b/src/registrar/templates/emails/status_change_approved.txt index bbef8c81a..70f813599 100644 --- a/src/registrar/templates/emails/status_change_approved.txt +++ b/src/registrar/templates/emails/status_change_approved.txt @@ -49,5 +49,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) {% endautoescape %} From 7463e356f948fe7aa72813b467224787b408e4f1 Mon Sep 17 00:00:00 2001 From: Cameron Dixon Date: Mon, 12 Aug 2024 11:54:13 -0400 Subject: [PATCH 43/61] Update domain_request_withdrawn.txt --- src/registrar/templates/emails/domain_request_withdrawn.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/domain_request_withdrawn.txt b/src/registrar/templates/emails/domain_request_withdrawn.txt index 0c061c53c..6efa92d64 100644 --- a/src/registrar/templates/emails/domain_request_withdrawn.txt +++ b/src/registrar/templates/emails/domain_request_withdrawn.txt @@ -26,5 +26,5 @@ The .gov team Contact us: Learn about .gov -The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) {% endautoescape %} From ad674e646b7e31d8538f28e46c105e6f10a131ea Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 09:59:10 -0600 Subject: [PATCH 44/61] Fix pre-existing bug with sortfields The AdminSortFields helper is incorrectly sorting the senior_official contact object. Added a workaround as this ultimately needs a refactor --- src/registrar/admin.py | 42 ++++++++++++++++--- src/registrar/forms/domain.py | 2 +- src/registrar/forms/portfolio.py | 2 +- .../templates/includes/senior_official.html | 2 +- src/registrar/tests/test_views_portfolio.py | 3 +- 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ae06a8d96..31cbdc38d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,7 +22,7 @@ from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website +from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR @@ -493,6 +493,8 @@ class CustomLogEntryAdmin(LogEntryAdmin): # return super().change_view(request, object_id, form_url, extra_context=extra_context) +# TODO - this should be refactored. This is shared among every class that inherits this, +# and it breaks the senior_official field because it exists both as model "Contact" and "SeniorOfficial". class AdminSortFields: _name_sort = ["first_name", "last_name", "email"] @@ -504,6 +506,8 @@ class AdminSortFields: # == Contact == # "other_contacts": (Contact, _name_sort), "submitter": (Contact, _name_sort), + # == Senior Official == # + "senior_official": (SeniorOfficial, _name_sort), # == User == # "creator": (User, _name_sort), "user": (User, _name_sort), @@ -553,15 +557,16 @@ class AuditedAdmin(admin.ModelAdmin): ) ) - def formfield_for_manytomany(self, db_field, request, **kwargs): + def formfield_for_manytomany(self, db_field, request, use_admin_sort_fields=True, **kwargs): """customize the behavior of formfields with manytomany relationships. the customized behavior includes sorting of objects in lists as well as customizing helper text""" # Define a queryset. Note that in the super of this, # a new queryset will only be generated if one does not exist. # Thus, the order in which we define queryset matters. + queryset = AdminSortFields.get_queryset(db_field) - if queryset: + if queryset and use_admin_sort_fields: kwargs["queryset"] = queryset formfield = super().formfield_for_manytomany(db_field, request, **kwargs) @@ -572,7 +577,7 @@ class AuditedAdmin(admin.ModelAdmin): ) return formfield - def formfield_for_foreignkey(self, db_field, request, **kwargs): + def formfield_for_foreignkey(self, db_field, request, use_admin_sort_fields=True, **kwargs): """Customize the behavior of formfields with foreign key relationships. This will customize the behavior of selects. Customized behavior includes sorting of objects in list.""" @@ -580,7 +585,7 @@ class AuditedAdmin(admin.ModelAdmin): # a new queryset will only be generated if one does not exist. # Thus, the order in which we define queryset matters. queryset = AdminSortFields.get_queryset(db_field) - if queryset: + if queryset and use_admin_sort_fields: kwargs["queryset"] = queryset return super().formfield_for_foreignkey(db_field, request, **kwargs) @@ -1542,6 +1547,16 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Get the filtered values return super().changelist_view(request, extra_context=extra_context) + def formfield_for_foreignkey(self, db_field, request, **kwargs): + """Customize the behavior of formfields with foreign key relationships. This will customize + the behavior of selects. Customized behavior includes sorting of objects in list.""" + # Remove this check on senior_official if this underlying model changes from + # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields. + # Removing this will cause the list on django admin to return SeniorOffical + # objects rather than Contact objects. + use_sort = db_field.name != "senior_official" + return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs) + class DomainRequestResource(FsmModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the @@ -2209,6 +2224,16 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return None + def formfield_for_foreignkey(self, db_field, request, **kwargs): + """Customize the behavior of formfields with foreign key relationships. This will customize + the behavior of selects. Customized behavior includes sorting of objects in list.""" + # Remove this check on senior_official if this underlying model changes from + # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields. + # Removing this will cause the list on django admin to return SeniorOffical + # objects rather than Contact objects. + use_sort = db_field.name != "senior_official" + return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs) + class TransitionDomainAdmin(ListHeaderAdmin): """Custom transition domain admin class.""" @@ -2258,6 +2283,7 @@ class DomainInformationInline(admin.StackedInline): def formfield_for_manytomany(self, db_field, request, **kwargs): """customize the behavior of formfields with manytomany relationships. the customized behavior includes sorting of objects in lists as well as customizing helper text""" + queryset = AdminSortFields.get_queryset(db_field) if queryset: kwargs["queryset"] = queryset @@ -2272,8 +2298,12 @@ class DomainInformationInline(admin.StackedInline): def formfield_for_foreignkey(self, db_field, request, **kwargs): """Customize the behavior of formfields with foreign key relationships. This will customize the behavior of selects. Customized behavior includes sorting of objects in list.""" + # Remove this check on senior_official if this underlying model changes from + # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields. + # Removing this will cause the list on django admin to return SeniorOffical + # objects rather than Contact objects. queryset = AdminSortFields.get_queryset(db_field) - if queryset: + if queryset and db_field.name != "senior_official": kwargs["queryset"] = queryset return super().formfield_for_foreignkey(db_field, request, **kwargs) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 4a1e95431..a46a4d3e8 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -326,7 +326,7 @@ class SeniorOfficialContactForm(ContactForm): def __init__(self, disable_fields=False, *args, **kwargs): super().__init__(*args, **kwargs) - if self.instance: + if self.instance and self.instance.id: self.fields["full_name"].initial = self.instance.get_formatted_name() # Overriding bc phone not required in this form diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 44d195dd6..67ccf2464 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -87,5 +87,5 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - if self.instance: + if self.instance and self.instance.id: self.fields["full_name"].initial = self.instance.get_formatted_name() diff --git a/src/registrar/templates/includes/senior_official.html b/src/registrar/templates/includes/senior_official.html index c63493ae8..b080127f4 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -44,4 +44,4 @@ {% if form.email.value is not None %} {% include "includes/input_read_only.html" with field=form.email %} {% endif %} -{% endif %} \ No newline at end of file +{% endif %} diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index d725c95ad..60764cf1c 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -41,7 +41,7 @@ class TestPortfolio(WebTest): @less_console_noise_decorator @override_flag("organization_feature", active=True) def test_portfolio_senior_official(self): - """Tests the senior official page on portfolio""" + """Tests that the senior official page on portfolio contains the content we expect""" self.app.set_user(self.user.username) so = SeniorOfficial.objects.create( @@ -63,6 +63,7 @@ class TestPortfolio(WebTest): self.assertContains(so_portfolio_page, "Saturn Enceladus") self.assertContains(so_portfolio_page, "Planet/Moon") self.assertContains(so_portfolio_page, "spacedivision@igorville.com") + self.assertNotContains(so_portfolio_page, "Save") self.portfolio.delete() so.delete() From 6c4e10cab14a9896ffbdda55dfddddabdd06ba07 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 10:10:38 -0600 Subject: [PATCH 45/61] Fix unit tests --- src/registrar/forms/domain.py | 6 +++ src/registrar/forms/portfolio.py | 6 +++ src/registrar/tests/test_views_domain.py | 51 +++--------------------- 3 files changed, 18 insertions(+), 45 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index a46a4d3e8..c724ce383 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -351,6 +351,12 @@ class SeniorOfficialContactForm(ContactForm): if disable_fields: DomainHelper.mass_disable_fields(fields=self.fields, disable_required=True, disable_maxlength=True) + def clean(self): + """Clean override to remove unused fields""" + cleaned_data = super().clean() + cleaned_data.pop("full_name", None) + return cleaned_data + def save(self, commit=True): """ Override the save() method of the BaseModelForm. diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 67ccf2464..cfd23c630 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -89,3 +89,9 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): super().__init__(*args, **kwargs) if self.instance and self.instance.id: self.fields["full_name"].initial = self.instance.get_formatted_name() + + def clean(self): + """Clean override to remove unused fields""" + cleaned_data = super().clean() + cleaned_data.pop("full_name", None) + return cleaned_data diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index efbc1296c..8c79c92aa 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1128,7 +1128,7 @@ class TestDomainSeniorOfficial(TestDomainOverview): def test_domain_senior_official(self): """Can load domain's senior official page.""" page = self.client.get(reverse("domain-senior-official", kwargs={"pk": self.domain.id})) - self.assertContains(page, "Senior official", count=14) + self.assertContains(page, "Senior official", count=3) @less_console_noise_decorator def test_domain_senior_official_content(self): @@ -1207,16 +1207,13 @@ class TestDomainSeniorOfficial(TestDomainOverview): self.domain_information.save() so_page = self.app.get(reverse("domain-senior-official", kwargs={"pk": self.domain.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - self.assertContains(so_page, "Apple Tester") self.assertContains(so_page, "CIO") self.assertContains(so_page, "nobody@igorville.gov") self.assertNotContains(so_page, "Save") @less_console_noise_decorator - def test_domain_edit_senior_official_tribal(self): + def test_domain_cannot_edit_senior_official_tribal(self): """Tests that no edit can occur when the underlying domain is tribal""" # Set the org type to federal @@ -1231,46 +1228,10 @@ class TestDomainSeniorOfficial(TestDomainOverview): self.domain_information.save() so_page = self.app.get(reverse("domain-senior-official", kwargs={"pk": self.domain.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - - # Test if the form is populating data correctly - so_form = so_page.forms[0] - - test_cases = [ - ("first_name", "Apple"), - ("last_name", "Tester"), - ("title", "CIO"), - ("email", "nobody@igorville.gov"), - ] - self.assert_all_form_fields_have_expected_values(so_form, test_cases, test_for_disabled=True) - - # Attempt to change data on each field. Because this domain is federal, - # this should not succeed. - so_form["first_name"] = "Orange" - so_form["last_name"] = "Smoothie" - so_form["title"] = "Cat" - so_form["email"] = "somebody@igorville.gov" - - submission = so_form.submit() - - # A 302 indicates this page underwent a redirect. - self.assertEqual(submission.status_code, 302) - - followed_submission = submission.follow() - - # Test the returned form for data accuracy. These values should be unchanged. - new_form = followed_submission.forms[0] - self.assert_all_form_fields_have_expected_values(new_form, test_cases, test_for_disabled=True) - - # refresh domain information. Test these values in the DB. - self.domain_information.refresh_from_db() - - # All values should be unchanged. These are defined manually for code clarity. - self.assertEqual("Apple", self.domain_information.senior_official.first_name) - self.assertEqual("Tester", self.domain_information.senior_official.last_name) - self.assertEqual("CIO", self.domain_information.senior_official.title) - self.assertEqual("nobody@igorville.gov", self.domain_information.senior_official.email) + self.assertContains(so_page, "Apple Tester") + self.assertContains(so_page, "CIO") + self.assertContains(so_page, "nobody@igorville.gov") + self.assertNotContains(so_page, "Save") @less_console_noise_decorator def test_domain_edit_senior_official_creates_new(self): From 60c4f54e63c3ab703bc02dbcb7a33c520084479d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 10:24:36 -0600 Subject: [PATCH 46/61] Fix suborg issue --- src/registrar/forms/domain.py | 4 ---- src/registrar/templates/domain_suborganization.html | 2 +- src/registrar/views/domain.py | 7 +++++++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index ec370f268..55b6619d9 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -189,10 +189,6 @@ class DomainSuborganizationForm(forms.ModelForm): if self.instance and self.instance.sub_organization: self.fields["sub_organization"].widget.attrs["data-default-value"] = self.instance.sub_organization.pk - def get_suborganization_name(self): - """Returns the suborganization name for the readonly view""" - return self.instance.sub_organization.name if self.instance else None - class BaseNameserverFormset(forms.BaseFormSet): def clean(self): diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index c2e3431ad..a3b2231a7 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -25,7 +25,7 @@ {% else %} {% with description="The suborganization for this domain can only be updated by a organization administrator."%} - {% include "includes/input_read_only.html" with field=form.sub_organization value=instance.get_suborganization_name label_description=description%} + {% include "includes/input_read_only.html" with field=form.sub_organization value=suborganization_name label_description=description%} {% endwith %} {% endif %} {% endblock %} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 949d580bd..9b5f2e1c7 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -250,6 +250,13 @@ class DomainSubOrganizationView(DomainFormBaseView): context_object_name = "domain" form_class = DomainSuborganizationForm + def get_context_data(self, **kwargs): + """Adds custom context.""" + context = super().get_context_data(**kwargs) + if self.object and self.object.domain_info and self.object.domain_info.sub_organization: + context["suborganization_name"] = self.object.domain_info.sub_organization.name + return context + def get_form_kwargs(self, *args, **kwargs): """Add domain_info.organization_name instance to make a bound form.""" form_kwargs = super().get_form_kwargs(*args, **kwargs) From b59a6243916c5105489a6f5185d59985d5dcd873 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 10:42:52 -0600 Subject: [PATCH 47/61] Code cleanup --- src/registrar/assets/js/get-gov.js | 22 +++++++------------ .../templates/domain_suborganization.html | 2 -- src/registrar/views/domain.py | 10 +++++++++ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 0bebb4d9b..1d9caafde 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1996,26 +1996,20 @@ document.addEventListener('DOMContentLoaded', function() { var isTyping = false; document.addEventListener('DOMContentLoaded', (event) => { - // The file location for the #undo svg - const undoIcon = document.querySelector("#uswds-undo-icon-url"); - const undoIconUrl = undoIcon ? undoIcon.getAttribute("data-undo-icon-url") : null; - if (undoIconUrl) { - handleAllComboBoxElements(undoIconUrl); - } + handleAllComboBoxElements(); }); - function handleAllComboBoxElements(undoIconUrl) { + function handleAllComboBoxElements() { const comboBoxElements = document.querySelectorAll(".usa-combo-box"); comboBoxElements.forEach(comboBox => { - const input = comboBox.querySelector('input'); + const input = comboBox.querySelector("input"); const select = comboBox.querySelector("select"); - if (!input || !undoIconUrl || !select) { + if (!input || !select) { console.warn("No combobox element found"); return; } // Set the initial value of the combobox let initialValue = select.getAttribute("data-default-value"); - let clearInputButton = comboBox.querySelector(".usa-combo-box__clear-input"); if (!clearInputButton) { console.warn("No clear element found"); @@ -2041,13 +2035,13 @@ document.addEventListener('DOMContentLoaded', function() { const config = { childList: true, subtree: true }; observer.observe(dropdownList, config); - // Add input event listener to detect typing - input.addEventListener('input', () => { + // Input event listener to detect typing + input.addEventListener("input", () => { isTyping = true; }); - // Add blur event listener to reset typing state - input.addEventListener('blur', () => { + // Blur event listener to reset typing state + input.addEventListener("blur", () => { isTyping = false; }); diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index a3b2231a7..42bb766a3 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -16,8 +16,6 @@

    {% if has_domains_portfolio_permission and request.user.has_edit_suborganization %} - {% comment %} Store the undo icon for reference in js - since this is dynamic {% endcomment %} -
    {% csrf_token %} {% input_with_errors form.sub_organization %} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 9b5f2e1c7..72f2fd27e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -250,6 +250,16 @@ class DomainSubOrganizationView(DomainFormBaseView): context_object_name = "domain" form_class = DomainSuborganizationForm + def has_permission(self): + """Override for the has_permission class to exclude non-portfolio users""" + + # non-org users shouldn't have access to this page + is_org_user = self.request.user.is_org_user(self.request) + if self.request.user.portfolio and is_org_user: + return super().has_permission() + else: + return False + def get_context_data(self, **kwargs): """Adds custom context.""" context = super().get_context_data(**kwargs) From 9523e7966e138610f9aa9783eee6eac40e77dcc2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 10:54:35 -0600 Subject: [PATCH 48/61] Don't show button if there is nothing to reset --- src/registrar/assets/js/get-gov.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 1d9caafde..d238823b3 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -2045,6 +2045,13 @@ document.addEventListener('DOMContentLoaded', function() { isTyping = false; }); + // Hide the reset button when there is nothing to reset. + // Do this once on init, then everytime a change occurs. + updateClearButtonVisibility(select, initialValue, clearInputButton) + select.addEventListener("change", () => { + updateClearButtonVisibility(select, initialValue, clearInputButton) + }); + // Change the default input behaviour - have it reset to the data default instead clearInputButton.addEventListener("click", (e) => { if (overrideDefaultClearButton && initialValue) { @@ -2066,6 +2073,14 @@ document.addEventListener('DOMContentLoaded', function() { }); } + function updateClearButtonVisibility(select, initialValue, clearInputButton) { + if (select.value === initialValue) { + hideElement(clearInputButton); + }else { + showElement(clearInputButton) + } + } + function addBlankOption(clearInputButton, dropdownList, initialValue) { if (dropdownList && !dropdownList.querySelector('[data-value=""]') && !isTyping) { const blankOption = document.createElement("li"); From 5f507c0bc8fa3f6ae86b32fd366d872f53f05074 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 10:57:56 -0600 Subject: [PATCH 49/61] Update test_views_domain.py --- src/registrar/tests/test_views_domain.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 2b83926c9..63a1651d6 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1618,8 +1618,10 @@ class TestDomainSuborganization(TestDomainOverview): # Navigate to the suborganization page page = self.app.get(reverse("domain-suborganization", kwargs={"pk": self.domain.id})) + # The page should display the readonly option + self.assertContains(page, "Vanilla") + # The page shouldn't contain these choices - self.assertNotContains(page, "Vanilla") self.assertNotContains(page, "Chocolate") self.assertNotContains(page, unrelated_suborg.name) self.assertNotContains(page, "Save") From c2af76b6dcd34996ea3ed03d8b7bb41966f5e94e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 11:21:16 -0600 Subject: [PATCH 50/61] lint --- src/registrar/forms/portfolio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index cfd23c630..14a45f6ae 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -89,7 +89,7 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): super().__init__(*args, **kwargs) if self.instance and self.instance.id: self.fields["full_name"].initial = self.instance.get_formatted_name() - + def clean(self): """Clean override to remove unused fields""" cleaned_data = super().clean() From 6b9ec52921361e7ac264069b207ad3de202ff507 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 12:19:50 -0600 Subject: [PATCH 51/61] Update admin.py --- src/registrar/admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 31cbdc38d..590ccbaac 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2876,6 +2876,7 @@ class PortfolioAdmin(ListHeaderAdmin): autocomplete_fields = [ "creator", "federal_agency", + "senior_official", ] def change_view(self, request, object_id, form_url="", extra_context=None): From 3691a9ee58751761070f5cd44c7ebf054cd93bc2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 12:51:08 -0600 Subject: [PATCH 52/61] Text toggle --- src/registrar/templates/domain_senior_official.html | 4 ++-- src/registrar/templates/includes/senior_official.html | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/domain_senior_official.html b/src/registrar/templates/domain_senior_official.html index e51bf255c..7ed90c2ec 100644 --- a/src/registrar/templates/domain_senior_official.html +++ b/src/registrar/templates/domain_senior_official.html @@ -5,8 +5,8 @@ {% block domain_content %} {% if generic_org_type == "federal" or generic_org_type == "tribal" %} - {% include "includes/senior_official.html" with can_edit=False %} + {% include "includes/senior_official.html" with can_edit=False include_read_more_text=True %} {% else %} - {% include "includes/senior_official.html" with can_edit=True %} + {% include "includes/senior_official.html" with can_edit=True include_read_more_text=True %} {% endif %} {% endblock %} diff --git a/src/registrar/templates/includes/senior_official.html b/src/registrar/templates/includes/senior_official.html index b080127f4..ef816e297 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -8,8 +8,10 @@

    Your senior official is a person within your organization who can authorize domain requests. - This person must be in a role of significant, executive responsibility within the organization. + This person must be in a role of significant, executive responsibility within the organization. + {% if include_read_more_text %} Read more about who can serve as a senior official. + {% endif %}

    {% if can_edit %} From bea53019a5f66940e9ccf650bdd55ab85fc2b3f8 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 12 Aug 2024 15:28:02 -0400 Subject: [PATCH 53/61] added clean method which performs validation on user model --- src/registrar/models/user.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 0221c2d50..73d5f6a1d 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -3,6 +3,7 @@ import logging from django.contrib.auth.models import AbstractUser from django.db import models from django.db.models import Q +from django.forms import ValidationError from registrar.models.domain_information import DomainInformation from registrar.models.user_domain_role import UserDomainRole @@ -229,6 +230,15 @@ class User(AbstractUser): def has_contact_info(self): return bool(self.title or self.email or self.phone) + def clean(self): + super().clean() + + if self.portfolio is None and self._get_portfolio_permissions(): + raise ValidationError("When portfolio roles or additional permissions are assigned, portfolio is required.") + + if self.portfolio is not None and not self._get_portfolio_permissions(): + raise ValidationError("When portfolio is assigned, portfolio roles or additional permissions are required.") + def _get_portfolio_permissions(self): """ Retrieve the permissions for the user's portfolio roles. From 334b6eac3816c9b2bff0b15029ac3b5d221be065 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 12 Aug 2024 15:29:15 -0400 Subject: [PATCH 54/61] commented --- src/registrar/models/user.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 73d5f6a1d..81d3b9b61 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -231,6 +231,7 @@ class User(AbstractUser): return bool(self.title or self.email or self.phone) def clean(self): + """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() if self.portfolio is None and self._get_portfolio_permissions(): From a2a6b9e68f83bd51dc304a417731d30fab4c818e Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 12 Aug 2024 15:36:14 -0400 Subject: [PATCH 55/61] fix zap for adding suborganization url --- src/zap.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/src/zap.conf b/src/zap.conf index d6d22995c..c97897aeb 100644 --- a/src/zap.conf +++ b/src/zap.conf @@ -71,6 +71,7 @@ 10038 OUTOFSCOPE http://app:8080/domain_requests/ 10038 OUTOFSCOPE http://app:8080/domains/ 10038 OUTOFSCOPE http://app:8080/organization/ +10038 OUTOFSCOPE http://app:8080/suborganization/ # This URL always returns 404, so include it as well. 10038 OUTOFSCOPE http://app:8080/todo # OIDC isn't configured in the test environment and DEBUG=True so this gives a 500 without CSP headers From 77cf108323d4c7848109adad735e277d6758543f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 12 Aug 2024 15:46:52 -0400 Subject: [PATCH 56/61] test cases added --- src/registrar/tests/test_models.py | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 8c69517e9..cfb66eb2a 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1,3 +1,4 @@ +from django.forms import ValidationError from django.test import TestCase from django.db.utils import IntegrityError from django.db import transaction @@ -1348,6 +1349,7 @@ class TestUser(TestCase): self.user.phone = None self.assertFalse(self.user.has_contact_info()) + @less_console_noise_decorator def test_has_portfolio_permission(self): """ 0. Returns False when user does not have a permission @@ -1401,6 +1403,40 @@ class TestUser(TestCase): Portfolio.objects.all().delete() + @less_console_noise_decorator + def test_user_with_portfolio_but_no_roles(self): + # Create an instance of User with a portfolio but no roles or additional permissions + portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") + + self.user.portfolio = portfolio + self.user.portfolio_roles = [] + + # Test if the ValidationError is raised with the correct message + with self.assertRaises(ValidationError) as cm: + self.user.clean() + + self.assertEqual( + cm.exception.message, "When portfolio is assigned, portfolio roles or additional permissions are required." + ) + self.user.refresh_from_db() + Portfolio.objects.all().delete() + + @less_console_noise_decorator + def test_user_with_portfolio_roles_but_no_portfolio(self): + # Create an instance of User with a portfolio role but no portfolio + self.user.portfolio = None + self.user.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + + # Test if the ValidationError is raised with the correct message + with self.assertRaises(ValidationError) as cm: + self.user.clean() + + self.assertEqual( + cm.exception.message, "When portfolio roles or additional permissions are assigned, portfolio is required." + ) + self.user.refresh_from_db() + Portfolio.objects.all().delete() + class TestContact(TestCase): @less_console_noise_decorator From 8ff5ba8ba0093eccec9a57536cfae3591ca8972a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 14:29:12 -0600 Subject: [PATCH 57/61] Update senior_official.html --- src/registrar/templates/includes/senior_official.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/includes/senior_official.html b/src/registrar/templates/includes/senior_official.html index ef816e297..67a66c3f8 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -7,9 +7,9 @@

    Senior Official

    - Your senior official is a person within your organization who can authorize domain requests. + Your senior official is a person within your organization who can authorize domain requests. + {% if include_read_more_text %} This person must be in a role of significant, executive responsibility within the organization. - {% if include_read_more_text %} Read more about who can serve as a senior official. {% endif %}

    From 2c50c35ed485952187f389c2464832880a59ccb3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 12 Aug 2024 17:22:33 -0400 Subject: [PATCH 58/61] fixed tests --- src/registrar/tests/test_models.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index cfb66eb2a..5167aac99 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1418,7 +1418,6 @@ class TestUser(TestCase): self.assertEqual( cm.exception.message, "When portfolio is assigned, portfolio roles or additional permissions are required." ) - self.user.refresh_from_db() Portfolio.objects.all().delete() @less_console_noise_decorator @@ -1434,8 +1433,6 @@ class TestUser(TestCase): self.assertEqual( cm.exception.message, "When portfolio roles or additional permissions are assigned, portfolio is required." ) - self.user.refresh_from_db() - Portfolio.objects.all().delete() class TestContact(TestCase): From 59166fa2e6556d1ed5ac1667c66ffc78aaa6ebbd Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 13 Aug 2024 09:36:07 -0500 Subject: [PATCH 59/61] remove header specificity on register-form-review-header styling --- src/registrar/assets/sass/_theme/_register-form.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_register-form.scss b/src/registrar/assets/sass/_theme/_register-form.scss index 7c93f0a10..41d2980e3 100644 --- a/src/registrar/assets/sass/_theme/_register-form.scss +++ b/src/registrar/assets/sass/_theme/_register-form.scss @@ -25,7 +25,7 @@ } } -h3.register-form-review-header { +.register-form-review-header { color: color('primary-dark'); margin-top: units(2); margin-bottom: 0; From 6b89ca43e43ee60b1b272d5a8c0056e7ed48794b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 13 Aug 2024 09:21:00 -0600 Subject: [PATCH 60/61] Add comments --- src/registrar/admin.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 590ccbaac..18c1052fc 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -493,7 +493,7 @@ class CustomLogEntryAdmin(LogEntryAdmin): # return super().change_view(request, object_id, form_url, extra_context=extra_context) -# TODO - this should be refactored. This is shared among every class that inherits this, +# TODO #2571 - this should be refactored. This is shared among every class that inherits this, # and it breaks the senior_official field because it exists both as model "Contact" and "SeniorOfficial". class AdminSortFields: _name_sort = ["first_name", "last_name", "email"] @@ -1550,6 +1550,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): def formfield_for_foreignkey(self, db_field, request, **kwargs): """Customize the behavior of formfields with foreign key relationships. This will customize the behavior of selects. Customized behavior includes sorting of objects in list.""" + # TODO #2571 # Remove this check on senior_official if this underlying model changes from # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields. # Removing this will cause the list on django admin to return SeniorOffical @@ -2227,6 +2228,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): def formfield_for_foreignkey(self, db_field, request, **kwargs): """Customize the behavior of formfields with foreign key relationships. This will customize the behavior of selects. Customized behavior includes sorting of objects in list.""" + # TODO #2571 # Remove this check on senior_official if this underlying model changes from # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields. # Removing this will cause the list on django admin to return SeniorOffical From 0efa02006fdcb1358378e393904e0f4eb518da84 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 13 Aug 2024 09:29:18 -0600 Subject: [PATCH 61/61] Add period --- src/registrar/templates/includes/senior_official.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/includes/senior_official.html b/src/registrar/templates/includes/senior_official.html index 67a66c3f8..fda97b6a9 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -19,7 +19,7 @@ {% else %}

    The senior official for your organization can’t be updated here. - To suggest an update, email help@get.gov + To suggest an update, email help@get.gov.

    {% endif %}