Merge pull request #3078 from cisagov/bob/2728-domain-manager-page-up…

…dates

#2728: domain manager page updates

src/registrar/templates/domain_users.html

@@ -6,39 +6,49 @@
 {% block domain_content %}
   <h1>Domain managers</h1>
 
+  {% comment %}Copy below differs depending on whether view is in portfolio mode.{% endcomment %}
+  {% if not portfolio %}
+  <p>
+    Domain managers can update all information related to a domain within the
+    .gov registrar, including security email and DNS name servers.
+  </p>
+  {% else %}
   <p>
   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.
   </p>
+  {% endif %}
 
   <ul class="usa-list">
     <li>There is no limit to the number of domain managers you can add.</li>
     <li>After adding a domain manager, an email invitation will be sent to that user with
       instructions on how to set up an account.</li>
     <li>All domain managers must keep their contact information updated and be responsive if contacted by the .gov team.</li>
-    <li>All domain managers will be notified when updates are made to this domain.</li>
-    <li>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.</li>
+    {% if not portfolio %}<li>All domain managers will be notified when updates are made to this domain.</li>{% endif %}
+    <li>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 %}</li>
   </ul>
 
-  {% if domain.permissions %}
+  {% if domain_manager_roles %}
   <section class="section-outlined">
   <table class="usa-table usa-table--borderless usa-table--stacked dotgov-table--stacked dotgov-table">
     <h2 class> Domain managers </h2>
     <caption class="sr-only">Domain managers</caption>
     <thead>
       <tr>
         <th data-sortable scope="col" role="columnheader">Email</th>
-        <th class="grid-col-2" data-sortable scope="col" role="columnheader">Role</th>
+        {% if not portfolio %}<th class="grid-col-2" data-sortable scope="col" role="columnheader">Role</th>{% endif %}
         <th class="grid-col-1" scope="col" role="columnheader"><span class="sr-only">Action</span></th>
       </tr>
     </thead>
     <tbody>
-      {% for permission in domain.permissions.all %}
+      {% for item in domain_manager_roles %}
       <tr>
-        <th scope="row" role="rowheader" data-sort-value="{{ permission.user.email }}" data-label="Email">
-          {{ permission.user.email }}
+        <th scope="row" role="rowheader" data-sort-value="{{ item.permission.user.email }}" data-label="Email">
+          {{ item.permission.user.email }}
+          {% if item.has_admin_flag %}<span class="usa-tag margin-left-1 bg-primary">Admin</span>{% endif %}
         </th>
-        <td data-label="Role">{{ permission.role|title }}</td>
+        {% if not portfolio %}<td data-label="Role">{{ item.permission.role|title }}</td>{% endif %}
         <td>
         {% if can_delete_users %}
           <a
@@ -52,15 +62,15 @@ <h2 class> Domain managers </h2>
             Remove
           </a>
           {# 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 %}
             <div
             class="usa-modal"
             id="toggle-user-alert-{{ forloop.counter }}"
             aria-labelledby="Are you sure you want to continue?"
             aria-describedby="You will be removed from this domain"
             data-force-action
             >
-              <form method="POST" action="{% url "domain-user-delete" pk=domain.id user_pk=permission.user.id %}">
+              <form method="POST" action="{% url "domain-user-delete" pk=domain.id user_pk=item.permission.user.id %}">
                 {% 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 <strong>"|add:domain_name|add:"</strong>."|safe modal_button=modal_button_self|safe %}
                 {% endwith %}
@@ -71,11 +81,11 @@ <h2 class> Domain managers </h2>
               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
             >
-              <form method="POST" action="{% url "domain-user-delete" pk=domain.id user_pk=permission.user.id %}">
-                {% with email=permission.user.email|default:permission.user|force_escape domain_name=domain.name|force_escape %}
+              <form method="POST" action="{% url "domain-user-delete" pk=domain.id user_pk=item.permission.user.id %}">
+                {% 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="<strong>"|add:email|add:"</strong> will no longer be able to manage the domain <strong>"|add:domain_name|add:"</strong>."|safe modal_button=modal_button|safe %}
                 {% endwith %}
               </form>
@@ -111,7 +121,7 @@ <h2 class> Domain managers </h2>
   </a>
   </section>
 
-  {% if domain.invitations.exists %}
+  {% if invitations %}
   <section class="section-outlined">
   <h2>Invitations</h2>
   <table class="usa-table usa-table--borderless usa-table--stacked dotgov-table--stacked dotgov-table">
@@ -120,21 +130,22 @@ <h2>Invitations</h2>
         <tr>
           <th data-sortable scope="col" role="columnheader">Email</th>
           <th data-sortable scope="col" role="columnheader">Date created</th>
-          <th class="grid-col-2" data-sortable scope="col" role="columnheader">Status</th>
+          {% if not portfolio %}<th class="grid-col-2" data-sortable scope="col" role="columnheader">Status</th>{% endif %}
           <th class="grid-col-1" scope="col" role="columnheader"><span class="sr-only">Action</span></th>
         </tr>
       </thead>
       <tbody>
-        {% for invitation in domain.invitations.all %}
+        {% for invitation in invitations %}
         <tr>
-          <th scope="row" role="rowheader" data-sort-value="{{ invitation.user.email }}" data-label="Email">
-            {{ invitation.email }}
+          <th scope="row" role="rowheader" data-sort-value="{{ invitation.domain_invitation.user.email }}" data-label="Email">
+            {{ invitation.domain_invitation.email }}
+            {% if invitation.has_admin_flag %}<span class="usa-tag margin-left-1 bg-primary">Admin</span>{% endif %}
           </th>
-          <td data-sort-value="{{ invitation.created_at|date:"U" }}" data-label="Date created">{{ invitation.created_at|date }} </td>
-          <td data-label="Status">{{ invitation.status|title }}</td>
+          <td data-sort-value="{{ invitation.domain_invitation.created_at|date:"U" }}" data-label="Date created">{{ invitation.domain_invitation.created_at|date }} </td>
+          {% if not portfolio %}<td data-label="Status">{{ invitation.domain_invitation.status|title }}</td>{% endif %}
           <td>
-            {% if invitation.status == invitation.DomainInvitationStatus.INVITED %}
-            <form method="POST" action="{% url "invitation-delete" pk=invitation.id %}">
+            {% if invitation.domain_invitation.status == invitation.domain_invitation.DomainInvitationStatus.INVITED %}
+            <form method="POST" action="{% url "invitation-delete" pk=invitation.domain_invitation.id %}">
               {% csrf_token %}<input type="submit" class="usa-button--unstyled text-no-underline cursor-pointer" value="Cancel">
             </form>
             {% endif %}

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</th>")
+        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 portfolio view doesn't contain Role column and does contain Admin
+        self.assertNotContains(response, "Role</th>")
+        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):

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