From 229c476d2eb5e0da023ab7a9e5f7128202907950 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 13 Nov 2024 18:59:28 -0500 Subject: [PATCH 1/3] add ADMIN tags to domain mgrs, conditional display based on portfolio --- src/registrar/templates/domain_users.html | 52 +++++++++------- src/registrar/views/domain.py | 76 +++++++++++++++++++++++ 2 files changed, 107 insertions(+), 21 deletions(-) diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index a2eb3e604..4f497dab0 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -6,10 +6,18 @@ {% block domain_content %}

Domain managers

+ {% comment %}Copy below differs depending on whether view is in portfolio mode.{% endcomment %} + {% if not portfolio %} +

+ Domain managers can update all information related to a domain within the + .gov registrar, including security email and DNS name servers. +

+ {% else %}

Domain managers can update all information related to a domain within the - .gov registrar, including security email and DNS name servers. + .gov registrar, including contact details, senior official, security email, and DNS name servers.

+ {% endif %} - {% if domain.permissions %} + {% if domain_manager_roles %}

Domain managers

@@ -28,17 +36,18 @@

Domain managers

- + {% if not portfolio %}{% endif %} - {% for permission in domain.permissions.all %} + {% for item in domain_manager_roles %} - - + {% if not portfolio %}{% endif %}
EmailRoleRoleAction
- {{ permission.user.email }} + + {{ item.permission.user.email }} + {% if item.has_admin_flag %}Admin{% endif %} {{ permission.role|title }}{{ item.permission.role|title }} {% if can_delete_users %} Domain managers Remove {# Display a custom message if the user is trying to delete themselves #} - {% if permission.user.email == current_user_email %} + {% if item.permission.user.email == current_user_email %}
Domain managers aria-describedby="You will be removed from this domain" data-force-action > -
+ {% with domain_name=domain.name|force_escape %} {% include 'includes/modal.html' with modal_heading="Are you sure you want to remove yourself as a domain manager?" modal_description="You will no longer be able to manage the domain "|add:domain_name|add:"."|safe modal_button=modal_button_self|safe %} {% endwith %} @@ -71,11 +80,11 @@

Domain managers

class="usa-modal" id="toggle-user-alert-{{ forloop.counter }}" aria-labelledby="Are you sure you want to continue?" - aria-describedby="{{ permission.user.email }} will be removed" + aria-describedby="{{ item.permission.user.email }} will be removed" data-force-action > - - {% with email=permission.user.email|default:permission.user|force_escape domain_name=domain.name|force_escape %} + + {% with email=item.permission.user.email|default:item.permission.user|force_escape domain_name=domain.name|force_escape %} {% include 'includes/modal.html' with modal_heading="Are you sure you want to remove " heading_value=email|add:"?" modal_description=""|add:email|add:" will no longer be able to manage the domain "|add:domain_name|add:"."|safe modal_button=modal_button|safe %} {% endwith %}
@@ -111,7 +120,7 @@

Domain managers

- {% if domain.invitations.exists %} + {% if invitations %}

Invitations

@@ -120,21 +129,22 @@

Invitations

- + {% if not portfolio %}{% endif %} - {% for invitation in domain.invitations.all %} + {% for invitation in invitations %} - - - + + {% if not portfolio %}{% endif %}
Email Date createdStatusStatusAction
- {{ invitation.email }} + + {{ invitation.domain_invitation.email }} + {% if invitation.has_admin_flag %}Admin{% endif %} {{ invitation.created_at|date }} {{ invitation.status|title }}{{ invitation.domain_invitation.created_at|date }} {{ invitation.domain_invitation.status|title }} - {% if invitation.status == invitation.DomainInvitationStatus.INVITED %} -
+ {% if invitation.domain_invitation.status == invitation.domain_invitation.DomainInvitationStatus.INVITED %} + {% csrf_token %}
{% endif %} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 6e85c9ffb..c378feeb9 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -28,6 +28,7 @@ UserPortfolioPermission, PublicContact, ) +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.utility.enums import DefaultEmail from registrar.utility.errors import ( GenericError, @@ -841,11 +842,86 @@ def get_context_data(self, **kwargs): # Add modal buttons to the context (such as for delete) context = self._add_modal_buttons_to_context(context) + # Get portfolio from session (if set) + portfolio = self.request.session.get("portfolio") + + # Add domain manager roles separately in order to also pass admin status + context = self._add_domain_manager_roles_to_context(context, portfolio) + + # Add domain invitations separately in order to also pass admin status + context = self._add_invitations_to_context(context, portfolio) + # Get the email of the current user context["current_user_email"] = self.request.user.email return context + def get(self, request, *args, **kwargs): + """Get method for DomainUsersView.""" + # Call the parent class's `get` method to get the response and context + response = super().get(request, *args, **kwargs) + + # Ensure context is available after the parent call + context = response.context_data if hasattr(response, "context_data") else {} + + # Check if context contains `domain_managers_roles` and its length is 1 + if context.get("domain_manager_roles") and len(context["domain_manager_roles"]) == 1: + # Add an info message + messages.info(request, "This domain has one manager. Adding more can prevent issues.") + + return response + + def _add_domain_manager_roles_to_context(self, context, portfolio): + """Add domain_manager_roles to context separately, as roles need admin indicator.""" + + # Prepare a list to store roles with an admin flag + domain_manager_roles = [] + + for permission in self.object.permissions.all(): + # Determine if the user has the ORGANIZATION_ADMIN role + has_admin_flag = any( + UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_permission.roles + and portfolio == portfolio_permission.portfolio + for portfolio_permission in permission.user.portfolio_permissions.all() + ) + + # Add the role along with the computed flag to the list + domain_manager_roles.append({"permission": permission, "has_admin_flag": has_admin_flag}) + + # Pass roles_with_flags to the context + context["domain_manager_roles"] = domain_manager_roles + + return context + + def _add_invitations_to_context(self, context, portfolio): + """Add invitations to context separately as invitations needs admin indicator.""" + + # Prepare a list to store invitations with an admin flag + invitations = [] + + for domain_invitation in self.object.invitations.all(): + # Check if there are any PortfolioInvitations linked to the same portfolio with the ORGANIZATION_ADMIN role + has_admin_flag = False + + # Query PortfolioInvitations linked to the same portfolio and check roles + portfolio_invitations = PortfolioInvitation.objects.filter( + portfolio=portfolio, email=domain_invitation.email + ) + + # If any of the PortfolioInvitations have the ORGANIZATION_ADMIN role, set the flag to True + for portfolio_invitation in portfolio_invitations: + if UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_invitation.roles: + has_admin_flag = True + break # Once we find one match, no need to check further + + # Add the role along with the computed flag to the list + invitations.append({"domain_invitation": domain_invitation, "has_admin_flag": has_admin_flag}) + + # Pass roles_with_flags to the context + context["invitations"] = invitations + + return context + def _add_booleans_to_context(self, context): # Determine if the current user can delete managers domain_pk = None From 224fe120af6abacc1c3d1b49789a73c8df933239 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 14 Nov 2024 06:10:01 -0500 Subject: [PATCH 2/3] added unit test --- src/registrar/tests/test_views_domain.py | 30 ++++++++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index a375493be..6f732bede 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -370,6 +370,17 @@ def setUpClass(cls): ] AllowedEmail.objects.bulk_create(allowed_emails) + def setUp(self): + super().setUp() + # Add portfolio in order to test portfolio view + self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Ice Cream") + # Add the portfolio to the domain_information object + self.domain_information.portfolio = self.portfolio + # Add portfolio perms to the user object + self.portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + user=self.user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + @classmethod def tearDownClass(cls): super().tearDownClass() @@ -383,13 +394,22 @@ def tearDown(self): def test_domain_managers(self): response = self.client.get(reverse("domain-users", kwargs={"pk": self.domain.id})) self.assertContains(response, "Domain managers") + self.assertContains(response, "Add a domain manager") + # assert that the non-portfolio view contains Role column and doesn't contain Admin + self.assertContains(response, "Role") + self.assertNotContains(response, "Admin") + self.assertContains(response, "This domain has one manager. Adding more can prevent issues.") @less_console_noise_decorator - def test_domain_managers_add_link(self): - """Button to get to user add page works.""" - management_page = self.app.get(reverse("domain-users", kwargs={"pk": self.domain.id})) - add_page = management_page.click("Add a domain manager") - self.assertContains(add_page, "Add a domain manager") + @override_flag("organization_feature", active=True) + def test_domain_managers_portfolio_view(self): + response = self.client.get(reverse("domain-users", kwargs={"pk": self.domain.id})) + self.assertContains(response, "Domain managers") + self.assertContains(response, "Add a domain manager") + # assert that the non-portfolio view contains Role column and doesn't contain Admin + self.assertNotContains(response, "Role") + self.assertContains(response, "Admin") + self.assertContains(response, "This domain has one manager. Adding more can prevent issues.") @less_console_noise_decorator def test_domain_user_add(self): From 9c671c0f71351beaaeba97c6ea8b61724f1b1fce Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 18 Nov 2024 12:27:05 -0500 Subject: [PATCH 3/3] fixed outstanding copy, and updated comment --- src/registrar/templates/domain_users.html | 5 +++-- src/registrar/tests/test_views_domain.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 4f497dab0..7fe97233f 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -24,8 +24,9 @@

Domain managers

  • After adding a domain manager, an email invitation will be sent to that user with instructions on how to set up an account.
  • All domain managers must keep their contact information updated and be responsive if contacted by the .gov team.
    • -
  • All domain managers will be notified when updates are made to this domain.
    • -
  • Domains must have at least one domain manager. You can’t remove yourself as a domain manager if you’re the only one assigned to this domain.
    • + {% if not portfolio %}
  • All domain managers will be notified when updates are made to this domain.
    • {% endif %} +
  • Domains must have at least one domain manager. You can’t remove yourself as a domain manager if you’re the only one assigned to this domain. + {% if portfolio %} Add another domain manager before you remove yourself from this domain.{% endif %}
    • {% if domain_manager_roles %} diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 6f732bede..1b7731222 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -406,7 +406,7 @@ def test_domain_managers_portfolio_view(self): response = self.client.get(reverse("domain-users", kwargs={"pk": self.domain.id})) self.assertContains(response, "Domain managers") self.assertContains(response, "Add a domain manager") - # assert that the non-portfolio view contains Role column and doesn't contain Admin + # assert that the portfolio view doesn't contain Role column and does contain Admin self.assertNotContains(response, "Role") self.assertContains(response, "Admin") self.assertContains(response, "This domain has one manager. Adding more can prevent issues.")