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 01/18] 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 90137c4af6..93c92f1ca2 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -74,6 +74,11 @@ 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 9362c7bbd4..88ec8e3f7e 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -4,7 +4,7 @@ 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 @@ def __init__(self, *args, **kwargs): 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 cfcdff3a87..cfbb30e911 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 0000000000..e22237ac4c --- /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 63ebbaa019..0903b2e587 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -3,7 +3,7 @@ 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 @@ def form_invalid(self, form): 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 02/18] 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 88ec8e3f7e..936953686c 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -70,27 +70,22 @@ def __init__(self, *args, **kwargs): 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 Meta: "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 e22237ac4c..9566736a01 100644 --- a/src/registrar/templates/portfolio_senior_official.html +++ b/src/registrar/templates/portfolio_senior_official.html @@ -23,9 +23,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 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 @@

    Senior Official

    {% 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 0903b2e587..3c851841ab 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 @@ def get(self, request, *args, **kwargs): 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 @@ def form_invalid(self, form): 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 03/18] 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 936953686c..d807b41848 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -106,6 +106,6 @@ 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/management/commands/load_senior_official_table.py b/src/registrar/management/commands/load_senior_official_table.py index cbfe479ea0..43f61d57a1 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 @@ def handle(self, federal_cio_csv_path, **kwargs): 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 @@ def handle(self, federal_cio_csv_path, **kwargs): # 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 @@ def handle(self, federal_cio_csv_path, **kwargs): 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 04/18] 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 d807b41848..92b68c86ad 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -98,14 +98,8 @@ class Meta: "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 9566736a01..12ddab908d 100644 --- a/src/registrar/templates/portfolio_senior_official.html +++ b/src/registrar/templates/portfolio_senior_official.html @@ -23,34 +23,19 @@

    Senior Official

    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 3c851841ab..8879f020c3 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -99,6 +99,7 @@ def get_success_url(self): 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 @@ def get(self, request, *args, **kwargs): 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 05/18] 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 92b68c86ad..ab94595885 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 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 06/18] 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 5d13e28e78..c911af1b9c 100644 --- a/src/registrar/templates/domain_senior_official.html +++ b/src/registrar/templates/domain_senior_official.html @@ -26,23 +26,13 @@

    Senior official

    {% 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 0000000000..73e7bfd774 --- /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 12ddab908d..976cf96399 100644 --- a/src/registrar/templates/portfolio_senior_official.html +++ b/src/registrar/templates/portfolio_senior_official.html @@ -28,15 +28,7 @@

    Senior Official

    {% 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 8879f020c3..a7c7d1356c 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 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 07/18] 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 ca4038d512..138183082e 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 02a0724d12..53f340aeeb 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 ab94595885..3a9e65b7f9 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 08/18] 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 138183082e..ae06a8d961 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,7 +22,7 @@ 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 53f340aeeb..4a1e954313 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 3a9e65b7f9..44d195dd6d 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 73e7bfd774..7a79087267 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 31ca776e21..efbc1296c6 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1192,14 +1192,14 @@ def assert_all_form_fields_have_expected_values(self, form, test_cases, test_for 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 @@ def test_domain_edit_senior_official_federal(self): 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 09/18] 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 f72130d949..d725c95ad6 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 @@ def tearDown(self): 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 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 10/18] 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 c911af1b9c..e51bf255c6 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 7a79087267..0000000000 --- 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 0000000000..c63493ae8c --- /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 976cf96399..5c47e1645f 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 b9042d617e..8a5321cc9c 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 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 11/18] 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 ae06a8d961..31cbdc38d5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,7 +22,7 @@ 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 @@ def changelist_view(self, request, extra_context=None): # 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 @@ def history_view(self, request, object_id, extra_context=None): ) ) - 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 @@ def formfield_for_manytomany(self, db_field, request, **kwargs): ) 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 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): # 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 @@ def changelist_view(self, request, extra_context=None): # 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 @@ def process_log_entry(self, log_entry): 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 @@ def has_change_permission(self, request, obj=None): 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 @@ def formfield_for_manytomany(self, db_field, request, **kwargs): 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 4a1e954313..a46a4d3e8d 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 44d195dd6d..67ccf2464b 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -87,5 +87,5 @@ class Meta: 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 c63493ae8c..b080127f45 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -44,4 +44,4 @@

    No senior official was found.

    {% 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 d725c95ad6..60764cf1ca 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -41,7 +41,7 @@ def tearDown(self): @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 @@ def test_portfolio_senior_official(self): 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 12/18] 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 a46a4d3e8d..c724ce3831 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -351,6 +351,12 @@ def __init__(self, disable_fields=False, *args, **kwargs): 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 67ccf2464b..cfd23c6307 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -89,3 +89,9 @@ 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/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index efbc1296c6..8c79c92aa2 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 @@ def test_domain_cannot_edit_senior_official_when_federal(self): 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 @@ def test_domain_edit_senior_official_tribal(self): 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 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 13/18] 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 cfd23c6307..14a45f6ae4 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -89,7 +89,7 @@ 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() 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 14/18] 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 31cbdc38d5..590ccbaacc 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 15/18] 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 e51bf255c6..7ed90c2ec7 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 b080127f45..ef816e2971 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -8,8 +8,10 @@

    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. + 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 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 16/18] 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 ef816e2971..67a66c3f85 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 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 17/18] 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 590ccbaacc..18c1052fcb 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -493,7 +493,7 @@ def changelist_view(self, request, extra_context=None): # 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 @@ def changelist_view(self, request, extra_context=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 @@ -2227,6 +2228,7 @@ def process_log_entry(self, log_entry): 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 18/18] 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 67a66c3f85..fda97b6a99 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -19,7 +19,7 @@

    Senior Official

    {% 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 %}