diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 2d750874e..607313f2d 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -493,6 +493,8 @@ class CustomLogEntryAdmin(LogEntryAdmin):
# return super().change_view(request, object_id, form_url, extra_context=extra_context)
+# 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"]
@@ -555,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)
@@ -574,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."""
@@ -582,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)
@@ -1544,6 +1547,17 @@ 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."""
+ # 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
+ # 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
@@ -2211,6 +2225,17 @@ 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."""
+ # 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
+ # 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."""
@@ -2260,6 +2285,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
@@ -2274,8 +2300,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)
@@ -2948,6 +2978,7 @@ class PortfolioAdmin(ListHeaderAdmin):
autocomplete_fields = [
"creator",
"federal_agency",
+ "senior_official",
]
# Q for reviewers: What should this be called?
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;
diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py
index 63026f931..3d13cf729 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/domain.py b/src/registrar/forms/domain.py
index 55b6619d9..02299395f 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -358,10 +358,14 @@ 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 and self.instance.id:
+ 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)
@@ -384,6 +388,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 9362c7bbd..14a45f6ae 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,31 @@ 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.
+ This form is readonly for now.
+ """
+
+ JOIN = "senior_official"
+ full_name = forms.CharField(label="Full name", required=False)
+
+ class Meta:
+ model = SeniorOfficial
+ fields = [
+ "title",
+ "email",
+ ]
+
+ def __init__(self, *args, **kwargs):
+ 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/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
diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py
index 0f75c0499..a532fe5fb 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,16 @@ class User(AbstractUser):
def has_contact_info(self):
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():
+ 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.
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 2e1d31d74..19f196e40 100644
--- a/src/registrar/templates/domain_detail.html
+++ b/src/registrar/templates/domain_detail.html
@@ -5,7 +5,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='Not Enabled' edit_link=url editable=is_editable %}
+ {% endif %}
{% if portfolio and has_domains_portfolio_permission and request.user.has_view_suborganization %}
{% url 'domain-suborganization' pk=domain.id as url %}
diff --git a/src/registrar/templates/domain_senior_official.html b/src/registrar/templates/domain_senior_official.html
index 5d13e28e7..7ed90c2ec 100644
--- a/src/registrar/templates/domain_senior_official.html
+++ b/src/registrar/templates/domain_senior_official.html
@@ -4,45 +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 include_read_more_text=True %}
{% else %}
- {% include "includes/required_fields.html" %}
+ {% include "includes/senior_official.html" with can_edit=True include_read_more_text=True %}
{% endif %}
-
-
-
-{% endblock %} {# domain_content #}
+{% endblock %}
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 %}
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 %}
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 %}
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 %}
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 %}
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 %}
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 %}
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 %}
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 %}
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 %}
diff --git a/src/registrar/templates/includes/senior_official.html b/src/registrar/templates/includes/senior_official.html
new file mode 100644
index 000000000..fda97b6a9
--- /dev/null
+++ b/src/registrar/templates/includes/senior_official.html
@@ -0,0 +1,49 @@
+{% 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.
+ {% if include_read_more_text %}
+ This person must be in a role of significant, executive responsibility within the organization.
+ Read more about who can serve as a senior official.
+ {% endif %}
+
+
+{% 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 %}
+
+{% 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 %}
diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html
index 6ec69b770..f69bbaf96 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 %}
-
-
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..5c47e1645
--- /dev/null
+++ b/src/registrar/templates/portfolio_senior_official.html
@@ -0,0 +1,24 @@
+{% 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' %}
+
+
+
+ {% include "includes/senior_official.html" with can_edit=False %}
+
+
+{% endblock %}
diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py
index 8c69517e9..5167aac99 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,37 @@ 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."
+ )
+ 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."
+ )
+
class TestContact(TestCase):
@less_console_noise_decorator
diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py
index 63a1651d6..3a90543a2 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):
@@ -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"
)
@@ -1207,49 +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)
-
- # 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):
+ 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
@@ -1264,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):
diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index f72130d94..60764cf1c 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,36 @@ 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 that the senior official page on portfolio contains the content we expect"""
+ 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.assertNotContains(so_portfolio_page, "Save")
+
+ 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"""
diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py
index 1956cf544..8a5321cc9 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,
@@ -97,3 +97,34 @@ 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.
+ For now, this view is readonly.
+ """
+
+ model = Portfolio
+ template_name = "portfolio_senior_official.html"
+ form_class = PortfolioSeniorOfficialForm
+ context_object_name = "portfolio"
+
+ 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))
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