Merge branch 'main' into backup/3004-org-permission-emails

This commit is contained in:
David Kennedy 2025-02-06 14:03:19 -05:00
commit 7718d6882a
No known key found for this signature in database
GPG key ID: 6528A5386E66B96B
23 changed files with 208 additions and 265 deletions

View file

@ -1,61 +0,0 @@
name: Story
description: Capture actionable sprint work
labels: ["story"]
body:
- type: markdown
id: help
attributes:
value: |
> **Note**
> GitHub Issues use [GitHub Flavored Markdown](https://docs.github.com/en/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax) for formatting.
- type: textarea
id: story
attributes:
label: Story
description: |
Please add the "as a, I want, so that" details that describe the story.
If more than one "as a, I want, so that" describes the story, add multiple.
Example:
As an analyst
I want the ability to approve a domain request
so that a request can be fulfilled and a new .gov domain can be provisioned
value: |
As a
I want
so that
validations:
required: true
- type: textarea
id: acceptance-criteria
attributes:
label: Acceptance Criteria
description: |
Please add the acceptance criteria that best describe the desired outcomes when this work is completed
Example:
- Application sends an email when analysts approve domain requests
- Domain request status is "approved"
Example ("given, when, then" format):
Given that I am an analyst who has finished reviewing a domain request
When I click to approve a domain request
Then the domain provisioning process should be initiated, and the applicant should receive an email update.
validations:
required: true
- type: textarea
id: additional-context
attributes:
label: Additional Context
description: "Please include additional references (screenshots, design links, documentation, etc.) that are relevant"
- type: textarea
id: issue-links
attributes:
label: Issue Links
description: |
What other issues does this story relate to and how?
Example:
- 🚧 Blocked by: #123
- 🔄 Relates to: #234

View file

@ -56,12 +56,11 @@ def add_path_to_context(request):
def portfolio_permissions(request): def portfolio_permissions(request):
"""Make portfolio permissions for the request user available in global context""" """Make portfolio permissions for the request user available in global context"""
portfolio_context = { portfolio_context = {
"has_base_portfolio_permission": False, "has_view_portfolio_permission": False,
"has_edit_portfolio_permission": False,
"has_any_domains_portfolio_permission": False, "has_any_domains_portfolio_permission": False,
"has_any_requests_portfolio_permission": False, "has_any_requests_portfolio_permission": False,
"has_edit_request_portfolio_permission": False, "has_edit_request_portfolio_permission": False,
"has_view_suborganization_portfolio_permission": False,
"has_edit_suborganization_portfolio_permission": False,
"has_view_members_portfolio_permission": False, "has_view_members_portfolio_permission": False,
"has_edit_members_portfolio_permission": False, "has_edit_members_portfolio_permission": False,
"portfolio": None, "portfolio": None,
@ -82,15 +81,11 @@ def portfolio_permissions(request):
} }
) )
# Linting: line too long
view_suborg = request.user.has_view_suborganization_portfolio_permission(portfolio)
edit_suborg = request.user.has_edit_suborganization_portfolio_permission(portfolio)
if portfolio: if portfolio:
return { return {
"has_base_portfolio_permission": request.user.has_base_portfolio_permission(portfolio), "has_view_portfolio_permission": request.user.has_view_portfolio_permission(portfolio),
"has_edit_portfolio_permission": request.user.has_edit_portfolio_permission(portfolio),
"has_edit_request_portfolio_permission": request.user.has_edit_request_portfolio_permission(portfolio), "has_edit_request_portfolio_permission": request.user.has_edit_request_portfolio_permission(portfolio),
"has_view_suborganization_portfolio_permission": view_suborg,
"has_edit_suborganization_portfolio_permission": edit_suborg,
"has_any_domains_portfolio_permission": request.user.has_any_domains_portfolio_permission(portfolio), "has_any_domains_portfolio_permission": request.user.has_any_domains_portfolio_permission(portfolio),
"has_any_requests_portfolio_permission": request.user.has_any_requests_portfolio_permission(portfolio), "has_any_requests_portfolio_permission": request.user.has_any_requests_portfolio_permission(portfolio),
"has_view_members_portfolio_permission": request.user.has_view_members_portfolio_permission(portfolio), "has_view_members_portfolio_permission": request.user.has_view_members_portfolio_permission(portfolio),

View file

@ -3,7 +3,6 @@ from django.utils import timezone
import logging import logging
import random import random
from faker import Faker from faker import Faker
from django.db import transaction
from registrar.fixtures.fixtures_requests import DomainRequestFixture from registrar.fixtures.fixtures_requests import DomainRequestFixture
from registrar.fixtures.fixtures_users import UserFixture from registrar.fixtures.fixtures_users import UserFixture
@ -29,19 +28,18 @@ class DomainFixture(DomainRequestFixture):
def load(cls): def load(cls):
# Lumped under .atomic to ensure we don't make redundant DB calls. # Lumped under .atomic to ensure we don't make redundant DB calls.
# This bundles them all together, and then saves it in a single call. # This bundles them all together, and then saves it in a single call.
with transaction.atomic(): try:
try: # Get the usernames of users created in the UserFixture
# Get the usernames of users created in the UserFixture created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
# Filter users to only include those created by the fixture # Filter users to only include those created by the fixture
users = list(User.objects.filter(username__in=created_usernames)) users = list(User.objects.filter(username__in=created_usernames))
except Exception as e: except Exception as e:
logger.warning(e) logger.warning(e)
return return
# Approve each user associated with `in review` status domains # Approve each user associated with `in review` status domains
cls._approve_domain_requests(users) cls._approve_domain_requests(users)
@staticmethod @staticmethod
def _generate_fake_expiration_date(days_in_future=365): def _generate_fake_expiration_date(days_in_future=365):

View file

@ -1,7 +1,6 @@
import logging import logging
import random import random
from faker import Faker from faker import Faker
from django.db import transaction
from registrar.models import User, DomainRequest, FederalAgency from registrar.models import User, DomainRequest, FederalAgency
from registrar.models.portfolio import Portfolio from registrar.models.portfolio import Portfolio
@ -84,42 +83,38 @@ class PortfolioFixture:
def load(cls): def load(cls):
"""Creates portfolios.""" """Creates portfolios."""
logger.info("Going to load %s portfolios" % len(cls.PORTFOLIOS)) logger.info("Going to load %s portfolios" % len(cls.PORTFOLIOS))
try:
user = User.objects.all().last()
except Exception as e:
logger.warning(e)
return
# Lumped under .atomic to ensure we don't make redundant DB calls. portfolios_to_create = []
# This bundles them all together, and then saves it in a single call. for portfolio_data in cls.PORTFOLIOS:
with transaction.atomic(): organization_name = portfolio_data["organization_name"]
# Check if portfolio with the organization name already exists
if Portfolio.objects.filter(organization_name=organization_name).exists():
logger.info(
f"Portfolio with organization name '{organization_name}' already exists, skipping creation."
)
continue
try:
portfolio = Portfolio(
creator=user,
organization_name=portfolio_data["organization_name"],
)
cls._set_non_foreign_key_fields(portfolio, portfolio_data)
cls._set_foreign_key_fields(portfolio, portfolio_data, user)
portfolios_to_create.append(portfolio)
except Exception as e:
logger.warning(e)
# Bulk create portfolios
if len(portfolios_to_create) > 0:
try: try:
user = User.objects.all().last() Portfolio.objects.bulk_create(portfolios_to_create)
logger.info(f"Successfully created {len(portfolios_to_create)} portfolios")
except Exception as e: except Exception as e:
logger.warning(e) logger.warning(f"Error bulk creating portfolios: {e}")
return
portfolios_to_create = []
for portfolio_data in cls.PORTFOLIOS:
organization_name = portfolio_data["organization_name"]
# Check if portfolio with the organization name already exists
if Portfolio.objects.filter(organization_name=organization_name).exists():
logger.info(
f"Portfolio with organization name '{organization_name}' already exists, skipping creation."
)
continue
try:
portfolio = Portfolio(
creator=user,
organization_name=portfolio_data["organization_name"],
)
cls._set_non_foreign_key_fields(portfolio, portfolio_data)
cls._set_foreign_key_fields(portfolio, portfolio_data, user)
portfolios_to_create.append(portfolio)
except Exception as e:
logger.warning(e)
# Bulk create domain requests
if len(portfolios_to_create) > 0:
try:
Portfolio.objects.bulk_create(portfolios_to_create)
logger.info(f"Successfully created {len(portfolios_to_create)} portfolios")
except Exception as e:
logger.warning(f"Error bulk creating portfolios: {e}")

View file

@ -3,7 +3,6 @@ from django.utils import timezone
import logging import logging
import random import random
from faker import Faker from faker import Faker
from django.db import transaction
from registrar.fixtures.fixtures_portfolios import PortfolioFixture from registrar.fixtures.fixtures_portfolios import PortfolioFixture
from registrar.fixtures.fixtures_suborganizations import SuborganizationFixture from registrar.fixtures.fixtures_suborganizations import SuborganizationFixture
@ -303,24 +302,17 @@ class DomainRequestFixture:
def load(cls): def load(cls):
"""Creates domain requests for each user in the database.""" """Creates domain requests for each user in the database."""
logger.info("Going to load %s domain requests" % len(cls.DOMAINREQUESTS)) logger.info("Going to load %s domain requests" % len(cls.DOMAINREQUESTS))
try:
# Get the usernames of users created in the UserFixture
created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
# Lumped under .atomic to ensure we don't make redundant DB calls. # Filter users to only include those created by the fixture
# This bundles them all together, and then saves it in a single call. users = list(User.objects.filter(username__in=created_usernames))
# The atomic block will cause the code to stop executing if one instance in the except Exception as e:
# nested iteration fails, which will cause an early exit and make it hard to debug. logger.warning(e)
# Comment out with transaction.atomic() when debugging. return
with transaction.atomic():
try:
# Get the usernames of users created in the UserFixture
created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
# Filter users to only include those created by the fixture cls._create_domain_requests(users)
users = list(User.objects.filter(username__in=created_usernames))
except Exception as e:
logger.warning(e)
return
cls._create_domain_requests(users)
@classmethod @classmethod
def _create_domain_requests(cls, users): # noqa: C901 def _create_domain_requests(cls, users): # noqa: C901

View file

@ -1,6 +1,5 @@
import logging import logging
from faker import Faker from faker import Faker
from django.db import transaction
from registrar.models.portfolio import Portfolio from registrar.models.portfolio import Portfolio
from registrar.models.suborganization import Suborganization from registrar.models.suborganization import Suborganization
@ -34,14 +33,12 @@ class SuborganizationFixture:
def load(cls): def load(cls):
"""Creates suborganizations.""" """Creates suborganizations."""
logger.info(f"Going to load {len(cls.SUBORGS)} suborgs") logger.info(f"Going to load {len(cls.SUBORGS)} suborgs")
portfolios = cls._get_portfolios()
if not portfolios:
return
with transaction.atomic(): suborgs_to_create = cls._prepare_suborgs_to_create(portfolios)
portfolios = cls._get_portfolios() cls._bulk_create_suborgs(suborgs_to_create)
if not portfolios:
return
suborgs_to_create = cls._prepare_suborgs_to_create(portfolios)
cls._bulk_create_suborgs(suborgs_to_create)
@classmethod @classmethod
def _get_portfolios(cls): def _get_portfolios(cls):

View file

@ -1,7 +1,6 @@
import logging import logging
import random import random
from faker import Faker from faker import Faker
from django.db import transaction
from registrar.fixtures.fixtures_portfolios import PortfolioFixture from registrar.fixtures.fixtures_portfolios import PortfolioFixture
from registrar.fixtures.fixtures_users import UserFixture from registrar.fixtures.fixtures_users import UserFixture
@ -26,56 +25,55 @@ class UserPortfolioPermissionFixture:
# Lumped under .atomic to ensure we don't make redundant DB calls. # Lumped under .atomic to ensure we don't make redundant DB calls.
# This bundles them all together, and then saves it in a single call. # This bundles them all together, and then saves it in a single call.
with transaction.atomic(): try:
try: # Get the usernames of users created in the UserFixture
# Get the usernames of users created in the UserFixture created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
created_usernames = [user_data["username"] for user_data in UserFixture.ADMINS + UserFixture.STAFF]
# Filter users to only include those created by the fixture # Filter users to only include those created by the fixture
users = list(User.objects.filter(username__in=created_usernames)) users = list(User.objects.filter(username__in=created_usernames))
organization_names = [portfolio["organization_name"] for portfolio in PortfolioFixture.PORTFOLIOS] organization_names = [portfolio["organization_name"] for portfolio in PortfolioFixture.PORTFOLIOS]
portfolios = list(Portfolio.objects.filter(organization_name__in=organization_names)) portfolios = list(Portfolio.objects.filter(organization_name__in=organization_names))
if not users: if not users:
logger.warning("User fixtures missing.") logger.warning("User fixtures missing.")
return
if not portfolios:
logger.warning("Portfolio fixtures missing.")
return
except Exception as e:
logger.warning(e)
return return
user_portfolio_permissions_to_create = [] if not portfolios:
for user in users: logger.warning("Portfolio fixtures missing.")
# Assign a random portfolio to a user return
portfolio = random.choice(portfolios) # nosec
try:
if not UserPortfolioPermission.objects.filter(user=user, portfolio=portfolio).exists():
user_portfolio_permission = UserPortfolioPermission(
user=user,
portfolio=portfolio,
roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
additional_permissions=[
UserPortfolioPermissionChoices.EDIT_MEMBERS,
UserPortfolioPermissionChoices.EDIT_REQUESTS,
],
)
user_portfolio_permissions_to_create.append(user_portfolio_permission)
else:
logger.info(
f"Permission exists for user '{user.username}' "
f"on portfolio '{portfolio.organization_name}'."
)
except Exception as e:
logger.warning(e)
# Bulk create permissions except Exception as e:
cls._bulk_create_permissions(user_portfolio_permissions_to_create) logger.warning(e)
return
user_portfolio_permissions_to_create = []
for user in users:
# Assign a random portfolio to a user
portfolio = random.choice(portfolios) # nosec
try:
if not UserPortfolioPermission.objects.filter(user=user, portfolio=portfolio).exists():
user_portfolio_permission = UserPortfolioPermission(
user=user,
portfolio=portfolio,
roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN],
additional_permissions=[
UserPortfolioPermissionChoices.EDIT_MEMBERS,
UserPortfolioPermissionChoices.EDIT_REQUESTS,
],
)
user_portfolio_permissions_to_create.append(user_portfolio_permission)
else:
logger.info(
f"Permission exists for user '{user.username}' "
f"on portfolio '{portfolio.organization_name}'."
)
except Exception as e:
logger.warning(e)
# Bulk create permissions
cls._bulk_create_permissions(user_portfolio_permissions_to_create)
@classmethod @classmethod
def _bulk_create_permissions(cls, user_portfolio_permissions_to_create): def _bulk_create_permissions(cls, user_portfolio_permissions_to_create):

View file

@ -1,6 +1,5 @@
import logging import logging
from faker import Faker from faker import Faker
from django.db import transaction
from registrar.models import ( from registrar.models import (
User, User,
@ -455,10 +454,9 @@ class UserFixture:
@classmethod @classmethod
def load(cls): def load(cls):
with transaction.atomic(): cls.load_users(cls.ADMINS, "full_access_group", are_superusers=True)
cls.load_users(cls.ADMINS, "full_access_group", are_superusers=True) cls.load_users(cls.STAFF, "cisa_analysts_group")
cls.load_users(cls.STAFF, "cisa_analysts_group")
# Combine ADMINS and STAFF lists # Combine ADMINS and STAFF lists
all_users = cls.ADMINS + cls.STAFF all_users = cls.ADMINS + cls.STAFF
cls.load_allowed_emails(cls, all_users, additional_emails=cls.ADDITIONAL_ALLOWED_EMAILS) cls.load_allowed_emails(cls, all_users, additional_emails=cls.ADDITIONAL_ALLOWED_EMAILS)

View file

@ -149,9 +149,9 @@ class Command(BaseCommand):
) )
return return
with transaction.atomic(): # Try to delete the portfolios
# Try to delete the portfolios try:
try: with transaction.atomic():
summary = [] summary = []
for portfolio in portfolios_to_delete: for portfolio in portfolios_to_delete:
portfolio_summary = [f"---- CASCADE SUMMARY for {portfolio.organization_name} -----"] portfolio_summary = [f"---- CASCADE SUMMARY for {portfolio.organization_name} -----"]
@ -222,14 +222,14 @@ class Command(BaseCommand):
""" """
) )
except IntegrityError as e: except IntegrityError as e:
logger.info( logger.info(
f"""{TerminalColors.FAIL} f"""{TerminalColors.FAIL}
Could not delete some portfolios due to integrity constraints: Could not delete some portfolios due to integrity constraints:
{e} {e}
{TerminalColors.ENDC} {TerminalColors.ENDC}
""" """
) )
def handle(self, *args, **options): def handle(self, *args, **options):
# Get all Portfolio entries not in the allowed portfolios list # Get all Portfolio entries not in the allowed portfolios list

View file

@ -0,0 +1,60 @@
# Generated by Django 4.2.10 on 2025-02-04 11:18
import django.contrib.postgres.fields
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("registrar", "0139_alter_domainrequest_action_needed_reason"),
]
operations = [
migrations.AlterField(
model_name="portfolioinvitation",
name="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_members", "View members"),
("edit_members", "Create and edit members"),
("view_all_requests", "View all 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,
),
),
migrations.AlterField(
model_name="userportfoliopermission",
name="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_members", "View members"),
("edit_members", "Create and edit members"),
("view_all_requests", "View all 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,
),
),
]

View file

@ -946,7 +946,7 @@ class DomainRequest(TimeStampedModel):
try: try:
if not context: if not context:
has_organization_feature_flag = flag_is_active_for_user(recipient, "organization_feature") has_organization_feature_flag = flag_is_active_for_user(recipient, "organization_feature")
is_org_user = has_organization_feature_flag and recipient.has_base_portfolio_permission(self.portfolio) is_org_user = has_organization_feature_flag and recipient.has_view_portfolio_permission(self.portfolio)
context = { context = {
"domain_request": self, "domain_request": self,
# This is the user that we refer to in the email # This is the user that we refer to in the email

View file

@ -210,10 +210,10 @@ class User(AbstractUser):
return portfolio_permission in user_portfolio_perms._get_portfolio_permissions() return portfolio_permission in user_portfolio_perms._get_portfolio_permissions()
def has_base_portfolio_permission(self, portfolio): def has_view_portfolio_permission(self, portfolio):
return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO) return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO)
def has_edit_org_portfolio_permission(self, portfolio): def has_edit_portfolio_permission(self, portfolio):
return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_PORTFOLIO) return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_PORTFOLIO)
def has_any_domains_portfolio_permission(self, portfolio): def has_any_domains_portfolio_permission(self, portfolio):
@ -268,13 +268,6 @@ class User(AbstractUser):
def has_edit_request_portfolio_permission(self, portfolio): def has_edit_request_portfolio_permission(self, portfolio):
return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS)
# Field specific permission checks
def has_view_suborganization_portfolio_permission(self, portfolio):
return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION)
def has_edit_suborganization_portfolio_permission(self, portfolio):
return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION)
def is_portfolio_admin(self, portfolio): def is_portfolio_admin(self, portfolio):
return "Admin" in self.portfolio_role_summary(portfolio) return "Admin" in self.portfolio_role_summary(portfolio)
@ -293,7 +286,7 @@ class User(AbstractUser):
# Define the conditions and their corresponding roles # Define the conditions and their corresponding roles
conditions_roles = [ conditions_roles = [
(self.has_edit_suborganization_portfolio_permission(portfolio), ["Admin"]), (self.has_edit_portfolio_permission(portfolio), ["Admin"]),
( (
self.has_view_all_domains_portfolio_permission(portfolio) self.has_view_all_domains_portfolio_permission(portfolio)
and self.has_any_requests_portfolio_permission(portfolio) and self.has_any_requests_portfolio_permission(portfolio)
@ -306,20 +299,20 @@ class User(AbstractUser):
["View-only admin"], ["View-only admin"],
), ),
( (
self.has_base_portfolio_permission(portfolio) self.has_view_portfolio_permission(portfolio)
and self.has_edit_request_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio)
and self.has_any_domains_portfolio_permission(portfolio), and self.has_any_domains_portfolio_permission(portfolio),
["Domain requestor", "Domain manager"], ["Domain requestor", "Domain manager"],
), ),
( (
self.has_base_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), self.has_view_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio),
["Domain requestor"], ["Domain requestor"],
), ),
( (
self.has_base_portfolio_permission(portfolio) and self.has_any_domains_portfolio_permission(portfolio), self.has_view_portfolio_permission(portfolio) and self.has_any_domains_portfolio_permission(portfolio),
["Domain manager"], ["Domain manager"],
), ),
(self.has_base_portfolio_permission(portfolio), ["Member"]), (self.has_view_portfolio_permission(portfolio), ["Member"]),
] ]
# Evaluate conditions and add roles # Evaluate conditions and add roles
@ -477,7 +470,7 @@ class User(AbstractUser):
def is_org_user(self, request): def is_org_user(self, request):
has_organization_feature_flag = flag_is_active(request, "organization_feature") has_organization_feature_flag = flag_is_active(request, "organization_feature")
portfolio = request.session.get("portfolio") portfolio = request.session.get("portfolio")
return has_organization_feature_flag and self.has_base_portfolio_permission(portfolio) return has_organization_feature_flag and self.has_view_portfolio_permission(portfolio)
def get_user_domain_ids(self, request): def get_user_domain_ids(self, request):
"""Returns either the domains ids associated with this user on UserDomainRole or Portfolio""" """Returns either the domains ids associated with this user on UserDomainRole or Portfolio"""

View file

@ -31,13 +31,10 @@ class UserPortfolioPermission(TimeStampedModel):
UserPortfolioPermissionChoices.EDIT_MEMBERS, UserPortfolioPermissionChoices.EDIT_MEMBERS,
UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
UserPortfolioPermissionChoices.EDIT_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION,
UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION,
], ],
# NOTE: Check FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS before adding roles here. # NOTE: Check FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS before adding roles here.
UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [
UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION,
], ],
} }
@ -47,7 +44,6 @@ class UserPortfolioPermission(TimeStampedModel):
UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [
UserPortfolioPermissionChoices.EDIT_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
UserPortfolioPermissionChoices.EDIT_MEMBERS, UserPortfolioPermissionChoices.EDIT_MEMBERS,
UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION,
], ],
} }

View file

@ -42,10 +42,6 @@ class UserPortfolioPermissionChoices(models.TextChoices):
VIEW_PORTFOLIO = "view_portfolio", "View organization" VIEW_PORTFOLIO = "view_portfolio", "View organization"
EDIT_PORTFOLIO = "edit_portfolio", "Edit organization" EDIT_PORTFOLIO = "edit_portfolio", "Edit organization"
# Domain: field specific permissions
VIEW_SUBORGANIZATION = "view_suborganization", "View suborganization"
EDIT_SUBORGANIZATION = "edit_suborganization", "Edit suborganization"
@classmethod @classmethod
def get_user_portfolio_permission_label(cls, user_portfolio_permission): def get_user_portfolio_permission_label(cls, user_portfolio_permission):
return cls(user_portfolio_permission).label if user_portfolio_permission else None return cls(user_portfolio_permission).label if user_portfolio_permission else None

View file

@ -103,12 +103,12 @@
{% endif %} {% endif %}
{% if portfolio %} {% if portfolio %}
{% if has_any_domains_portfolio_permission and has_edit_suborganization_portfolio_permission %} {% if has_any_domains_portfolio_permission and has_edit_portfolio_permission %}
{% url 'domain-suborganization' pk=domain.id as url %} {% url 'domain-suborganization' pk=domain.id as url %}
{% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_edit_suborganization_portfolio_permission %} {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_edit_portfolio_permission %}
{% elif has_any_domains_portfolio_permission and has_view_suborganization_portfolio_permission %} {% elif has_any_domains_portfolio_permission and has_view_portfolio_permission %}
{% url 'domain-suborganization' pk=domain.id as url %} {% url 'domain-suborganization' pk=domain.id as url %}
{% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_view_suborganization_portfolio_permission view_button=True %} {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_view_portfolio_permission view_button=True %}
{% endif %} {% endif %}
{% else %} {% else %}
{% url 'domain-org-name-address' pk=domain.id as url %} {% url 'domain-org-name-address' pk=domain.id as url %}

View file

@ -61,7 +61,7 @@
{% if portfolio %} {% if portfolio %}
{% comment %} Only show this menu option if the user has the perms to do so {% endcomment %} {% comment %} Only show this menu option if the user has the perms to do so {% endcomment %}
{% if has_any_domains_portfolio_permission and has_view_suborganization_portfolio_permission %} {% if has_any_domains_portfolio_permission and has_view_portfolio_permission %}
{% with url_name="domain-suborganization" %} {% with url_name="domain-suborganization" %}
{% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %} {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %}
{% endwith %} {% endwith %}

View file

@ -39,7 +39,7 @@
please contact <a href="mailto:help@get.gov" class="usa-link">help@get.gov</a>. please contact <a href="mailto:help@get.gov" class="usa-link">help@get.gov</a>.
</p> </p>
{% if has_any_domains_portfolio_permission and has_edit_suborganization_portfolio_permission %} {% if has_any_domains_portfolio_permission and has_edit_portfolio_permission %}
<form class="usa-form usa-form--large" method="post" novalidate id="form-container"> <form class="usa-form usa-form--large" method="post" novalidate id="form-container">
{% csrf_token %} {% csrf_token %}
{% input_with_errors form.sub_organization %} {% input_with_errors form.sub_organization %}

View file

@ -208,7 +208,7 @@
<th data-sortable="name" scope="col" role="columnheader">Domain name</th> <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="expiration_date" scope="col" role="columnheader">Expires</th>
<th data-sortable="state_display" scope="col" role="columnheader">Status</th> <th data-sortable="state_display" scope="col" role="columnheader">Status</th>
{% if portfolio and has_view_suborganization_portfolio_permission %} {% if portfolio and has_view_portfolio_permission %}
<th data-sortable="domain_info__sub_organization" scope="col" role="columnheader">Suborganization</th> <th data-sortable="domain_info__sub_organization" scope="col" role="columnheader">Suborganization</th>
{% endif %} {% endif %}
<th <th

View file

@ -28,7 +28,7 @@
<p>The name of your organization will be publicly listed as the domain registrant.</p> <p>The name of your organization will be publicly listed as the domain registrant.</p>
{% if has_edit_org_portfolio_permission %} {% if has_edit_portfolio_permission %}
<p> <p>
Your organization name cant be updated here. Your organization name cant be updated here.
To suggest an update, email <a href="mailto:help@get.gov" class="usa-link">help@get.gov</a>. To suggest an update, email <a href="mailto:help@get.gov" class="usa-link">help@get.gov</a>.

View file

@ -1191,8 +1191,8 @@ class TestUser(TestCase):
User.objects.all().delete() User.objects.all().delete()
UserDomainRole.objects.all().delete() UserDomainRole.objects.all().delete()
@patch.object(User, "has_edit_suborganization_portfolio_permission", return_value=True) @patch.object(User, "has_edit_portfolio_permission", return_value=True)
def test_portfolio_role_summary_admin(self, mock_edit_suborganization): def test_portfolio_role_summary_admin(self, mock_edit_org):
# Test if the user is recognized as an Admin # Test if the user is recognized as an Admin
self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Admin"]) self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Admin"])
@ -1217,7 +1217,7 @@ class TestUser(TestCase):
@patch.multiple( @patch.multiple(
User, User,
has_base_portfolio_permission=lambda self, portfolio: True, has_view_portfolio_permission=lambda self, portfolio: True,
has_edit_request_portfolio_permission=lambda self, portfolio: True, has_edit_request_portfolio_permission=lambda self, portfolio: True,
has_any_domains_portfolio_permission=lambda self, portfolio: True, has_any_domains_portfolio_permission=lambda self, portfolio: True,
) )
@ -1227,7 +1227,7 @@ class TestUser(TestCase):
@patch.multiple( @patch.multiple(
User, User,
has_base_portfolio_permission=lambda self, portfolio: True, has_view_portfolio_permission=lambda self, portfolio: True,
has_edit_request_portfolio_permission=lambda self, portfolio: True, has_edit_request_portfolio_permission=lambda self, portfolio: True,
) )
def test_portfolio_role_summary_member_domain_requestor(self): def test_portfolio_role_summary_member_domain_requestor(self):
@ -1236,14 +1236,14 @@ class TestUser(TestCase):
@patch.multiple( @patch.multiple(
User, User,
has_base_portfolio_permission=lambda self, portfolio: True, has_view_portfolio_permission=lambda self, portfolio: True,
has_any_domains_portfolio_permission=lambda self, portfolio: True, has_any_domains_portfolio_permission=lambda self, portfolio: True,
) )
def test_portfolio_role_summary_member_domain_manager(self): def test_portfolio_role_summary_member_domain_manager(self):
# Test if the user has 'Member' and 'Domain manager' roles # Test if the user has 'Member' and 'Domain manager' roles
self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain manager"]) self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain manager"])
@patch.multiple(User, has_base_portfolio_permission=lambda self, portfolio: True) @patch.multiple(User, has_view_portfolio_permission=lambda self, portfolio: True)
def test_portfolio_role_summary_member(self): def test_portfolio_role_summary_member(self):
# Test if the user is recognized as a Member # Test if the user is recognized as a Member
self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Member"]) self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Member"])
@ -1253,17 +1253,17 @@ class TestUser(TestCase):
self.assertEqual(self.user.portfolio_role_summary(self.portfolio), []) self.assertEqual(self.user.portfolio_role_summary(self.portfolio), [])
@patch("registrar.models.User._has_portfolio_permission") @patch("registrar.models.User._has_portfolio_permission")
def test_has_base_portfolio_permission(self, mock_has_permission): def test_has_view_portfolio_permission(self, mock_has_permission):
mock_has_permission.return_value = True mock_has_permission.return_value = True
self.assertTrue(self.user.has_base_portfolio_permission(self.portfolio)) self.assertTrue(self.user.has_view_portfolio_permission(self.portfolio))
mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO)
@patch("registrar.models.User._has_portfolio_permission") @patch("registrar.models.User._has_portfolio_permission")
def test_has_edit_org_portfolio_permission(self, mock_has_permission): def test_has_edit_portfolio_permission(self, mock_has_permission):
mock_has_permission.return_value = True mock_has_permission.return_value = True
self.assertTrue(self.user.has_edit_org_portfolio_permission(self.portfolio)) self.assertTrue(self.user.has_edit_portfolio_permission(self.portfolio))
mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_PORTFOLIO) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_PORTFOLIO)
@patch("registrar.models.User._has_portfolio_permission") @patch("registrar.models.User._has_portfolio_permission")
@ -1306,20 +1306,6 @@ class TestUser(TestCase):
self.assertTrue(self.user.has_edit_request_portfolio_permission(self.portfolio)) self.assertTrue(self.user.has_edit_request_portfolio_permission(self.portfolio))
mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS)
@patch("registrar.models.User._has_portfolio_permission")
def test_has_view_suborganization_portfolio_permission(self, mock_has_permission):
mock_has_permission.return_value = True
self.assertTrue(self.user.has_view_suborganization_portfolio_permission(self.portfolio))
mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION)
@patch("registrar.models.User._has_portfolio_permission")
def test_has_edit_suborganization_portfolio_permission(self, mock_has_permission):
mock_has_permission.return_value = True
self.assertTrue(self.user.has_edit_suborganization_portfolio_permission(self.portfolio))
mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION)
@less_console_noise_decorator @less_console_noise_decorator
def test_check_transition_domains_without_domains_on_login(self): def test_check_transition_domains_without_domains_on_login(self):
"""A user's on_each_login callback does not check transition domains. """A user's on_each_login callback does not check transition domains.

View file

@ -725,7 +725,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip()
self.assertEqual(csv_content, expected_content) self.assertEqual(csv_content, expected_content)
# @less_console_noise_decorator @less_console_noise_decorator
def test_domain_request_data_full(self): def test_domain_request_data_full(self):
"""Tests the full domain request report.""" """Tests the full domain request report."""
# Remove "Submitted at" because we can't guess this immutable, dynamically generated test data # Remove "Submitted at" because we can't guess this immutable, dynamically generated test data

View file

@ -2190,7 +2190,7 @@ class TestDomainSuborganization(TestDomainOverview):
self.domain_information.refresh_from_db() self.domain_information.refresh_from_db()
# Add portfolio perms to the user object # Add portfolio perms to the user object
portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( UserPortfolioPermission.objects.get_or_create(
user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
) )

View file

@ -762,7 +762,7 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin):
"""Add additional context data to the template.""" """Add additional context data to the template."""
context = super().get_context_data(**kwargs) context = super().get_context_data(**kwargs)
portfolio = self.request.session.get("portfolio") portfolio = self.request.session.get("portfolio")
context["has_edit_org_portfolio_permission"] = self.request.user.has_edit_org_portfolio_permission(portfolio) context["has_edit_portfolio_permission"] = self.request.user.has_edit_portfolio_permission(portfolio)
return context return context
def get_object(self, queryset=None): def get_object(self, queryset=None):