Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2728: domain manager page updates [BOB] #3078

Merged
merged 5 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 31 additions & 21 deletions src/registrar/templates/domain_users.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,18 @@
{% 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment: Looking at the copy from the Figma, the bullet point with "All domain managers will be notified when updates are made to this domain." needs to be removed.

</p>
{% endif %}

<ul class="usa-list">
<li>There is no limit to the number of domain managers you can add.</li>
Expand All @@ -20,25 +28,26 @@ <h1>Domain managers</h1>
<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>
</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
Expand All @@ -52,15 +61,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 %}
Expand All @@ -71,11 +80,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>
Expand Down Expand Up @@ -111,7 +120,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">
Expand All @@ -120,21 +129,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 %}
Expand Down
30 changes: 25 additions & 5 deletions src/registrar/tests/test_views_domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 non-portfolio view contains Role column and doesn't contain Admin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment: Update comment to that this is the portfolio view and that it doesn't contain Role column and does contain Admin tag

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):
Expand Down
76 changes: 76 additions & 0 deletions src/registrar/views/domain.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Loading