Skip to content

Commit

Permalink
Merge pull request #2500 from cisagov/dk/2350-portfolio-domain-permis…
Browse files Browse the repository at this point in the history
…sions

Issue #2350: Portfolio domain permissions
  • Loading branch information
dave-kennedy-ecs authored Aug 1, 2024
2 parents 152e4c0 + 9bc59fd commit 6e3d6bb
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 49 deletions.
11 changes: 9 additions & 2 deletions src/registrar/assets/js/get-gov.js
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,8 @@ document.addEventListener('DOMContentLoaded', function() {
const statusIndicator = document.querySelector('.domain__filter-indicator');
const statusToggle = document.querySelector('.usa-button--filter');
const noPortfolioFlag = document.getElementById('no-portfolio-js-flag');
const portfolioElement = document.getElementById('portfolio-js-value');
const portfolioValue = portfolioElement ? portfolioElement.getAttribute('data-portfolio') : null;

/**
* Loads rows in the domains list, as well as updates pagination around the domains list
Expand All @@ -1178,10 +1180,15 @@ document.addEventListener('DOMContentLoaded', function() {
* @param {*} order - the sort order {asc, desc}
* @param {*} scroll - control for the scrollToElement functionality
* @param {*} searchTerm - the search term
* @param {*} portfolio - the portfolio id
*/
function loadDomains(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, status = currentStatus, searchTerm = currentSearchTerm) {
function loadDomains(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, status = currentStatus, searchTerm = currentSearchTerm, portfolio = portfolioValue) {
// fetch json of page of domains, given params
fetch(`/get-domains-json/?page=${page}&sort_by=${sortBy}&order=${order}&status=${status}&search_term=${searchTerm}`)
let url = `/get-domains-json/?page=${page}&sort_by=${sortBy}&order=${order}&status=${status}&search_term=${searchTerm}`
if (portfolio)
url += `&portfolio=${portfolio}`

fetch(url)
.then(response => response.json())
.then(data => {
if (data.error) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Generated by Django 4.2.10 on 2024-07-25 12:45

import django.contrib.postgres.fields
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("registrar", "0113_user_portfolio_user_portfolio_additional_permissions_and_more"),
]

operations = [
migrations.AlterField(
model_name="user",
name="portfolio_additional_permissions",
field=django.contrib.postgres.fields.ArrayField(
base_field=models.CharField(
choices=[
("view_all_domains", "View all domains and domain reports"),
("view_managed_domains", "View managed domains"),
("view_member", "View members"),
("edit_member", "Create and edit members"),
("view_all_requests", "View all requests"),
("view_created_requests", "View created requests"),
("edit_requests", "Create and edit requests"),
("view_portfolio", "View organization"),
("edit_portfolio", "Edit organization"),
],
max_length=50,
),
blank=True,
help_text="Select one or more additional permissions.",
null=True,
size=None,
),
),
]
29 changes: 6 additions & 23 deletions src/registrar/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,6 @@ class UserPortfolioPermissionChoices(models.TextChoices):

VIEW_ALL_DOMAINS = "view_all_domains", "View all domains and domain reports"
VIEW_MANAGED_DOMAINS = "view_managed_domains", "View managed domains"
# EDIT_DOMAINS is really self.domains. We add is hear and leverage it in has_permission
# so we have one way to test for portfolio and domain edit permissions
# Do we need to check for portfolio domains specifically?
# NOTE: A user on an org can currently invite a user outside the org
EDIT_DOMAINS = "edit_domains", "User is a manager on a domain"

VIEW_MEMBER = "view_member", "View members"
EDIT_MEMBER = "edit_member", "Create and edit members"
Expand Down Expand Up @@ -268,11 +263,6 @@ def _get_portfolio_permissions(self):
def _has_portfolio_permission(self, portfolio_permission):
"""The views should only call this function when testing for perms and not rely on roles."""

# EDIT_DOMAINS === user is a manager on a domain (has UserDomainRole)
# NOTE: Should we check whether the domain is in the portfolio?
if portfolio_permission == self.UserPortfolioPermissionChoices.EDIT_DOMAINS and self.domains.exists():
return True

if not self.portfolio:
return False

Expand All @@ -286,21 +276,14 @@ def has_base_portfolio_permission(self):
return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_PORTFOLIO)

def has_domains_portfolio_permission(self):
return (
self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS)
or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)
# or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_DOMAINS)
)

def has_edit_domains_portfolio_permission(self):
return self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_DOMAINS)
return self._has_portfolio_permission(
User.UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS
) or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)

def has_domain_requests_portfolio_permission(self):
return (
self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS)
or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS)
# or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.EDIT_REQUESTS)
)
return self._has_portfolio_permission(
User.UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS
) or self._has_portfolio_permission(User.UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS)

@classmethod
def needs_identity_verification(cls, email, uuid):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

<section class="section--outlined domain-requests" id="domain-requests">
<div class="grid-row">
<!-- Use portfolio_base_permission when merging into 2366 and then delete this comment -->
{% if portfolio is None %}
<div class="mobile:grid-col-12 desktop:grid-col-6">
<h2 id="domain-requests-header" class="flex-6">Domain requests</h2>
Expand Down
14 changes: 8 additions & 6 deletions src/registrar/templates/includes/domains_table.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
{% load static %}

<section class="section--outlined domains{% if portfolio is not None %} margin-top-0{% endif %}" id="domains">
<div class="section--outlined__header margin-bottom-3 {% if portfolio is None %} section--outlined__header--no-portfolio justify-content-space-between{% else %} grid-row{% endif %}">
<!-- Use portfolio_base_permission when merging into 2366 then delete this comment -->
<div class="grid-row">
{% if portfolio is None %}
<h2 id="domains-header" class="display-inline-block">Domains</h2>
<span class="display-none" id="no-portfolio-js-flag"></span>
<div class="mobile:grid-col-12 desktop:grid-col-6">
<h2 id="domains-header" class="flex-6">Domains</h2>
</div>
<span class="display-none" id="no-portfolio-js-flag"></span>
{% else %}
<!-- Embedding the portfolio value in a data attribute -->
<span id="portfolio-js-value" data-portfolio="{{ portfolio.id }}"></span>
{% endif %}
<div class="section--outlined__search {% if portfolio %} mobile:grid-col-12 desktop:grid-col-6{% endif %}">
<section aria-label="Domains search component" class="margin-top-2">
Expand Down Expand Up @@ -45,7 +49,6 @@ <h2 id="domains-header" class="display-inline-block">Domains</h2>
</section>
</div>
</div>
<!-- Use portfolio_base_permission when merging into 2366 then delete this comment -->
{% if portfolio %}
<div class="display-flex flex-align-center">
<span class="margin-right-2 margin-top-neg-1 usa-prose text-base-darker">Filter by</span>
Expand Down Expand Up @@ -149,7 +152,6 @@ <h2>Status</h2>
<th data-sortable="name" scope="col" role="columnheader">Domain name</th>
<th data-sortable="expiration_date" scope="col" role="columnheader">Expires</th>
<th data-sortable="state_display" scope="col" role="columnheader">Status</th>
<!-- Use portfolio_base_permission when merging into 2366 then delete this comment -->
{% if portfolio %}
<th data-sortable="suborganization" scope="col" role="columnheader">Suborganization</th>
{% endif %}
Expand Down
9 changes: 0 additions & 9 deletions src/registrar/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1292,7 +1292,6 @@ def test_has_portfolio_permission(self):
1. Returns False when a user does not have a portfolio
2. Returns True when user has direct permission
3. Returns True when user has permission through a role
4. Returns True EDIT_DOMAINS when user does not have the perm but has UserDomainRole
Note: This tests _get_portfolio_permissions as a side effect
"""
Expand All @@ -1304,47 +1303,39 @@ def test_has_portfolio_permission(self):

user_can_view_all_domains = self.user.has_domains_portfolio_permission()
user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission()
user_can_edit_domains = self.user.has_edit_domains_portfolio_permission()

self.assertFalse(user_can_view_all_domains)
self.assertFalse(user_can_view_all_requests)
self.assertFalse(user_can_edit_domains)

self.user.portfolio = portfolio
self.user.save()
self.user.refresh_from_db()

user_can_view_all_domains = self.user.has_domains_portfolio_permission()
user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission()
user_can_edit_domains = self.user.has_edit_domains_portfolio_permission()

self.assertTrue(user_can_view_all_domains)
self.assertFalse(user_can_view_all_requests)
self.assertFalse(user_can_edit_domains)

self.user.portfolio_roles = [User.UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
self.user.save()
self.user.refresh_from_db()

user_can_view_all_domains = self.user.has_domains_portfolio_permission()
user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission()
user_can_edit_domains = self.user.has_edit_domains_portfolio_permission()

self.assertTrue(user_can_view_all_domains)
self.assertTrue(user_can_view_all_requests)
self.assertFalse(user_can_edit_domains)

UserDomainRole.objects.all().get_or_create(
user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER
)

user_can_view_all_domains = self.user.has_domains_portfolio_permission()
user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission()
user_can_edit_domains = self.user.has_edit_domains_portfolio_permission()

self.assertTrue(user_can_view_all_domains)
self.assertTrue(user_can_view_all_requests)
self.assertTrue(user_can_edit_domains)

Portfolio.objects.all().delete()

Expand Down
90 changes: 88 additions & 2 deletions src/registrar/tests/test_views_domains_json.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from registrar.models import UserDomainRole, Domain
from registrar.models import UserDomainRole, Domain, DomainInformation, Portfolio
from django.urls import reverse
from .test_views import TestWithUser
from django_webtest import WebTest # type: ignore
Expand All @@ -15,15 +15,25 @@ def setUp(self):
self.domain1 = Domain.objects.create(name="example1.com", expiration_date="2024-01-01", state="unknown")
self.domain2 = Domain.objects.create(name="example2.com", expiration_date="2024-02-01", state="dns needed")
self.domain3 = Domain.objects.create(name="example3.com", expiration_date="2024-03-01", state="ready")
self.domain4 = Domain.objects.create(name="example4.com", expiration_date="2024-03-01", state="ready")

# Create UserDomainRoles
UserDomainRole.objects.create(user=self.user, domain=self.domain1)
UserDomainRole.objects.create(user=self.user, domain=self.domain2)
UserDomainRole.objects.create(user=self.user, domain=self.domain3)

# Create Portfolio
self.portfolio = Portfolio.objects.create(creator=self.user, organization_name="Example org")

# Add domain3 and domain4 to portfolio
DomainInformation.objects.create(creator=self.user, domain=self.domain3, portfolio=self.portfolio)
DomainInformation.objects.create(creator=self.user, domain=self.domain4, portfolio=self.portfolio)

def tearDown(self):
super().tearDown()
UserDomainRole.objects.all().delete()
DomainInformation.objects.all().delete()
Portfolio.objects.all().delete()
super().tearDown()

@less_console_noise_decorator
def test_get_domains_json_unauthenticated(self):
Expand Down Expand Up @@ -104,6 +114,82 @@ def test_get_domains_json_authenticated(self):
)
self.assertEqual(svg_icon_expected, svg_icons[i])

@less_console_noise_decorator
def test_get_domains_json_with_portfolio(self):
"""Test that an authenticated user gets the list of 2 domains for portfolio."""

response = self.app.get(reverse("get_domains_json"), {"portfolio": self.portfolio.id})
self.assertEqual(response.status_code, 200)
data = response.json

# Check pagination info
self.assertEqual(data["page"], 1)
self.assertFalse(data["has_next"])
self.assertFalse(data["has_previous"])
self.assertEqual(data["num_pages"], 1)

# Check the number of domains
self.assertEqual(len(data["domains"]), 2)

# Expected domains
expected_domains = [self.domain3, self.domain4]

# Extract fields from response
domain_ids = [domain["id"] for domain in data["domains"]]
names = [domain["name"] for domain in data["domains"]]
expiration_dates = [domain["expiration_date"] for domain in data["domains"]]
states = [domain["state"] for domain in data["domains"]]
state_displays = [domain["state_display"] for domain in data["domains"]]
get_state_help_texts = [domain["get_state_help_text"] for domain in data["domains"]]
action_urls = [domain["action_url"] for domain in data["domains"]]
action_labels = [domain["action_label"] for domain in data["domains"]]
svg_icons = [domain["svg_icon"] for domain in data["domains"]]

# Check fields for each domain
for i, expected_domain in enumerate(expected_domains):
self.assertEqual(expected_domain.id, domain_ids[i])
self.assertEqual(expected_domain.name, names[i])
self.assertEqual(expected_domain.expiration_date, expiration_dates[i])
self.assertEqual(expected_domain.state, states[i])

# Parsing the expiration date from string to date
parsed_expiration_date = parse_date(expiration_dates[i])
expected_domain.expiration_date = parsed_expiration_date

# Check state_display and get_state_help_text
self.assertEqual(expected_domain.state_display(), state_displays[i])
self.assertEqual(expected_domain.get_state_help_text(), get_state_help_texts[i])

self.assertEqual(reverse("domain", kwargs={"pk": expected_domain.id}), action_urls[i])

# Check action_label
user_domain_role_exists = UserDomainRole.objects.filter(
domain_id=expected_domains[i].id, user=self.user
).exists()
action_label_expected = (
"View"
if not user_domain_role_exists
or expected_domains[i].state
in [
Domain.State.DELETED,
Domain.State.ON_HOLD,
]
else "Manage"
)
self.assertEqual(action_label_expected, action_labels[i])

# Check svg_icon
svg_icon_expected = (
"visibility"
if expected_domains[i].state
in [
Domain.State.DELETED,
Domain.State.ON_HOLD,
]
else "settings"
)
self.assertEqual(svg_icon_expected, svg_icons[i])

@less_console_noise_decorator
def test_get_domains_json_search(self):
"""Test search."""
Expand Down
Loading

0 comments on commit 6e3d6bb

Please sign in to comment.