diff --git a/.github/workflows/load-fixtures.yaml b/.github/workflows/load-fixtures.yaml new file mode 100644 index 000000000..108a54564 --- /dev/null +++ b/.github/workflows/load-fixtures.yaml @@ -0,0 +1,50 @@ +# Manually load fixtures to an environment of choice. + +name: Load fixtures +run-name: Manually load fixtures to sandbox of choice + +on: + workflow_dispatch: + inputs: + environment: + description: Which environment should we load data for? + type: 'choice' + options: + - ab + - backup + - el + - cb + - dk + - es + - gd + - ko + - ky + - nl + - rb + - rh + - rjm + - meoward + - bob + - hotgov + - litterbox + - ms + - ad + - ag + +jobs: + load-fixtures: + runs-on: ubuntu-latest + env: + CF_USERNAME: CF_${{ github.event.inputs.environment }}_USERNAME + CF_PASSWORD: CF_${{ github.event.inputs.environment }}_PASSWORD + steps: + - uses: GitHubSecurityLab/actions-permissions/monitor@v1 + - name: Load fake data for ${{ github.event.inputs.environment }} + uses: cloud-gov/cg-cli-tools@main + with: + cf_username: ${{ secrets[env.CF_USERNAME] }} + cf_password: ${{ secrets[env.CF_PASSWORD] }} + cf_org: cisa-dotgov + cf_space: ${{ github.event.inputs.environment }} + cf_command: "run-task getgov-${{ github.event.inputs.environment }} --command 'python manage.py load' --name loaddata" + diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 2c10edc7a..5188c7312 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -2925,7 +2925,7 @@ document.addEventListener("DOMContentLoaded", () => { const selectParent = select?.parentElement; const suborgContainer = document.getElementById("suborganization-container"); const suborgDetailsContainer = document.getElementById("suborganization-container__details"); - const subOrgCreateNewOption = document.getElementById("option-to-add-suborg").value + const subOrgCreateNewOption = document.getElementById("option-to-add-suborg")?.value; // Make sure all crucial page elements exist before proceeding. // This more or less ensures that we are on the Requesting Entity page, and not elsewhere. if (!radios || !select || !selectParent || !suborgContainer || !suborgDetailsContainer) return; diff --git a/src/registrar/assets/sass/_theme/_forms.scss b/src/registrar/assets/sass/_theme/_forms.scss index 08e35b19f..9158de174 100644 --- a/src/registrar/assets/sass/_theme/_forms.scss +++ b/src/registrar/assets/sass/_theme/_forms.scss @@ -1,5 +1,6 @@ @use "uswds-core" as *; @use "cisa_colors" as *; +@use "typography" as *; .usa-form .usa-button { margin-top: units(3); @@ -69,9 +70,9 @@ legend.float-left-tablet + button.float-right-tablet { } .read-only-label { - font-size: size('body', 'sm'); + @extend .h4--sm-05; + font-weight: bold; color: color('primary-dark'); - margin-bottom: units(0.5); } .read-only-value { diff --git a/src/registrar/assets/sass/_theme/_typography.scss b/src/registrar/assets/sass/_theme/_typography.scss index d815ef6dd..466b6f975 100644 --- a/src/registrar/assets/sass/_theme/_typography.scss +++ b/src/registrar/assets/sass/_theme/_typography.scss @@ -23,6 +23,13 @@ h2 { color: color('primary-darker'); } +.h4--sm-05 { + font-size: size('body', 'sm'); + font-weight: normal; + color: color('primary'); + margin-bottom: units(0.5); +} + // Normalize typography in forms .usa-form, .usa-form fieldset { diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index ae35a8865..c1547ad88 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -68,6 +68,7 @@ def portfolio_permissions(request): "has_organization_feature_flag": False, "has_organization_requests_flag": False, "has_organization_members_flag": False, + "is_portfolio_admin": False, } try: portfolio = request.session.get("portfolio") @@ -88,6 +89,7 @@ def portfolio_permissions(request): "has_organization_feature_flag": True, "has_organization_requests_flag": request.user.has_organization_requests_flag(), "has_organization_members_flag": request.user.has_organization_members_flag(), + "is_portfolio_admin": request.user.is_portfolio_admin(portfolio), } return portfolio_context diff --git a/src/registrar/fixtures/fixtures_requests.py b/src/registrar/fixtures/fixtures_requests.py index f5b57491e..93167ec61 100644 --- a/src/registrar/fixtures/fixtures_requests.py +++ b/src/registrar/fixtures/fixtures_requests.py @@ -6,8 +6,10 @@ from faker import Faker from django.db import transaction from registrar.fixtures.fixtures_portfolios import PortfolioFixture +from registrar.fixtures.fixtures_suborganizations import SuborganizationFixture from registrar.fixtures.fixtures_users import UserFixture from registrar.models import User, DomainRequest, DraftDomain, Contact, Website, FederalAgency +from registrar.models.domain import Domain from registrar.models.portfolio import Portfolio from registrar.models.suborganization import Suborganization @@ -101,8 +103,13 @@ class DomainRequestFixture: } @classmethod - def fake_dot_gov(cls): - return f"{fake.slug()}.gov" + def fake_dot_gov(cls, max_attempts=100): + """Generate a unique .gov domain name without using an infinite loop.""" + for _ in range(max_attempts): + fake_name = f"{fake.slug()}.gov" + if not Domain.objects.filter(name=fake_name).exists(): + return DraftDomain.objects.create(name=fake_name) + raise RuntimeError(f"Failed to generate a unique .gov domain after {max_attempts} attempts") @classmethod def fake_expiration_date(cls): @@ -189,7 +196,9 @@ class DomainRequestFixture: if not request.requested_domain: if "requested_domain" in request_dict and request_dict["requested_domain"] is not None: return DraftDomain.objects.get_or_create(name=request_dict["requested_domain"])[0] - return DraftDomain.objects.create(name=cls.fake_dot_gov()) + + # Generate a unique fake domain + return cls.fake_dot_gov() return request.requested_domain @classmethod @@ -213,7 +222,7 @@ class DomainRequestFixture: if not request.sub_organization: if "sub_organization" in request_dict and request_dict["sub_organization"] is not None: return Suborganization.objects.get_or_create(name=request_dict["sub_organization"])[0] - return cls._get_random_sub_organization() + return cls._get_random_sub_organization(request) return request.sub_organization @classmethod @@ -228,10 +237,19 @@ class DomainRequestFixture: return None @classmethod - def _get_random_sub_organization(cls): + def _get_random_sub_organization(cls, request): try: - suborg_options = [Suborganization.objects.first(), Suborganization.objects.last()] - return random.choice(suborg_options) # nosec + # Filter Suborganizations by the request's portfolio + portfolio_suborganizations = Suborganization.objects.filter(portfolio=request.portfolio) + + # Select a suborg that's defined in the fixtures + suborganization_names = [suborg["name"] for suborg in SuborganizationFixture.SUBORGS] + + # Further filter by names in suborganization_names + suborganization_options = portfolio_suborganizations.filter(name__in=suborganization_names) + + # Randomly choose one if any exist + return random.choice(suborganization_options) if suborganization_options.exists() else None # nosec except Exception as e: logger.warning(f"Expected fixture sub_organization, did not find it: {e}") return None @@ -273,6 +291,9 @@ class DomainRequestFixture: # 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. + # The atomic block will cause the code to stop executing if one instance in the + # nested iteration fails, which will cause an early exit and make it hard to debug. + # Comment out with transaction.atomic() when debugging. with transaction.atomic(): try: # Get the usernames of users created in the UserFixture diff --git a/src/registrar/fixtures/fixtures_users.py b/src/registrar/fixtures/fixtures_users.py index e60be9872..a8cdb5b9a 100644 --- a/src/registrar/fixtures/fixtures_users.py +++ b/src/registrar/fixtures/fixtures_users.py @@ -267,54 +267,24 @@ class UserFixture: """Loads the users into the database and assigns them to the specified group.""" logger.info(f"Going to load {len(users)} users for group {group_name}") + # Step 1: Fetch the group group = UserGroup.objects.get(name=group_name) - # Prepare sets of existing usernames and IDs in one query - user_identifiers = [(user.get("username"), user.get("id")) for user in users] - existing_users = User.objects.filter( - username__in=[user[0] for user in user_identifiers] + [user[1] for user in user_identifiers] - ).values_list("username", "id") + # Step 2: Identify new and existing users + existing_usernames, existing_user_ids = cls._get_existing_users(users) + new_users = cls._prepare_new_users(users, existing_usernames, existing_user_ids, are_superusers) - existing_usernames = set(user[0] for user in existing_users) - existing_user_ids = set(user[1] for user in existing_users) - - # Filter out users with existing IDs or usernames - new_users = [ - User( - id=user_data.get("id"), - first_name=user_data.get("first_name"), - last_name=user_data.get("last_name"), - username=user_data.get("username"), - email=user_data.get("email", ""), - title=user_data.get("title", "Peon"), - phone=user_data.get("phone", "2022222222"), - is_active=user_data.get("is_active", True), - is_staff=True, - is_superuser=are_superusers, - ) - for user_data in users - if user_data.get("username") not in existing_usernames and user_data.get("id") not in existing_user_ids - ] - - # Perform bulk creation for new users - if new_users: - try: - User.objects.bulk_create(new_users) - logger.info(f"Created {len(new_users)} new users.") - except Exception as e: - logger.error(f"Unexpected error during user bulk creation: {e}") - else: - logger.info("No new users to create.") + # Step 3: Create new users + cls._create_new_users(new_users) + # Step 4: Update existing users # Get all users to be updated (both new and existing) created_or_existing_users = User.objects.filter(username__in=[user.get("username") for user in users]) + users_to_update = cls._get_users_to_update(created_or_existing_users) + cls._update_existing_users(users_to_update) - # Filter out users who are already in the group - users_not_in_group = created_or_existing_users.exclude(groups__id=group.id) - - # Add only users who are not already in the group - if users_not_in_group.exists(): - group.user_set.add(*users_not_in_group) + # Step 5: Assign users to the group + cls._assign_users_to_group(group, created_or_existing_users) logger.info(f"Users loaded for group {group_name}.") @@ -346,6 +316,76 @@ class UserFixture: else: logger.info("No allowed emails to load") + @staticmethod + def _get_existing_users(users): + user_identifiers = [(user.get("username"), user.get("id")) for user in users] + existing_users = User.objects.filter( + username__in=[user[0] for user in user_identifiers] + [user[1] for user in user_identifiers] + ).values_list("username", "id") + existing_usernames = set(user[0] for user in existing_users) + existing_user_ids = set(user[1] for user in existing_users) + return existing_usernames, existing_user_ids + + @staticmethod + def _prepare_new_users(users, existing_usernames, existing_user_ids, are_superusers): + return [ + User( + id=user_data.get("id"), + first_name=user_data.get("first_name"), + last_name=user_data.get("last_name"), + username=user_data.get("username"), + email=user_data.get("email", ""), + title=user_data.get("title", "Peon"), + phone=user_data.get("phone", "2022222222"), + is_active=user_data.get("is_active", True), + is_staff=True, + is_superuser=are_superusers, + ) + for user_data in users + if user_data.get("username") not in existing_usernames and user_data.get("id") not in existing_user_ids + ] + + @staticmethod + def _create_new_users(new_users): + if new_users: + try: + User.objects.bulk_create(new_users) + logger.info(f"Created {len(new_users)} new users.") + except Exception as e: + logger.error(f"Unexpected error during user bulk creation: {e}") + else: + logger.info("No new users to create.") + + @staticmethod + def _get_users_to_update(users): + users_to_update = [] + for user in users: + updated = False + if not user.title: + user.title = "Peon" + updated = True + if not user.phone: + user.phone = "2022222222" + updated = True + if not user.is_staff: + user.is_staff = True + updated = True + if updated: + users_to_update.append(user) + return users_to_update + + @staticmethod + def _update_existing_users(users_to_update): + if users_to_update: + User.objects.bulk_update(users_to_update, ["is_staff", "title", "phone"]) + logger.info(f"Updated {len(users_to_update)} existing users.") + + @staticmethod + def _assign_users_to_group(group, users): + users_not_in_group = users.exclude(groups__id=group.id) + if users_not_in_group.exists(): + group.user_set.add(*users_not_in_group) + @classmethod def load(cls): with transaction.atomic(): diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 2d65aa02e..6c9c37c92 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -258,6 +258,9 @@ class User(AbstractUser): def has_edit_suborganization_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) + def is_portfolio_admin(self, portfolio): + return "Admin" in self.portfolio_role_summary(portfolio) + def get_first_portfolio(self): permission = self.portfolio_permissions.first() if permission: diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 51f3fa3fe..319f15d67 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -2,7 +2,12 @@ from django.db import models from django.forms import ValidationError from registrar.models.user_domain_role import UserDomainRole from registrar.utility.waffle import flag_is_active_for_user -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices, DomainRequestPermissionDisplay, MemberPermissionDisplay +from registrar.models.utility.portfolio_helper import ( + UserPortfolioPermissionChoices, + UserPortfolioRoleChoices, + DomainRequestPermissionDisplay, + MemberPermissionDisplay, +) from .utility.time_stamped_model import TimeStampedModel from django.contrib.postgres.fields import ArrayField @@ -115,7 +120,7 @@ class UserPortfolioPermission(TimeStampedModel): UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS, ] - + if all(perm in all_permissions for perm in all_domain_perms): return DomainRequestPermissionDisplay.VIEWER_REQUESTER elif UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS in all_permissions: diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 9b661b316..f1a6cec7a 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -51,6 +51,7 @@ class DomainRequestPermissionDisplay(StrEnum): - VIEWER: "Viewer" - NONE: "None" """ + VIEWER_REQUESTER = "Viewer Requester" VIEWER = "Viewer" NONE = "None" @@ -64,6 +65,7 @@ class MemberPermissionDisplay(StrEnum): - VIEWER: "Viewer" - NONE: "None" """ + MANAGER = "Manager" VIEWER = "Viewer" NONE = "None" diff --git a/src/registrar/templates/domain_add_user.html b/src/registrar/templates/domain_add_user.html index fa3f8e821..1429127e6 100644 --- a/src/registrar/templates/domain_add_user.html +++ b/src/registrar/templates/domain_add_user.html @@ -5,6 +5,25 @@ {% block domain_content %} {% block breadcrumb %} + {% if portfolio %} + + + {% else %} {% url 'domain-users' pk=domain.id as url %} + {% endif %} {% endblock breadcrumb %}

Add a domain manager

{% if has_organization_feature_flag %} diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index 96ec4c5b6..add7ca725 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -3,6 +3,22 @@ {% load custom_filters %} {% block domain_content %} + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} + {{ block.super }}

{{ domain.name }}

@@ -74,13 +90,17 @@ {% include "includes/summary_item.html" with title='DNSSEC' value='Not Enabled' edit_link=url editable=is_editable %} {% endif %} - {% if portfolio and has_any_domains_portfolio_permission and has_view_suborganization_portfolio_permission %} - {% 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 %} + {% if portfolio %} + {% if has_any_domains_portfolio_permission and has_edit_suborganization_portfolio_permission %} + {% 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 %} + {% elif has_any_domains_portfolio_permission and has_view_suborganization_portfolio_permission %} + {% 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 %} + {% endif %} {% else %} {% url 'domain-org-name-address' pk=domain.id as url %} {% include "includes/summary_item.html" with title='Organization' value=domain.domain_info address='true' edit_link=url editable=is_editable %} - {% url 'domain-senior-official' pk=domain.id as url %} {% include "includes/summary_item.html" with title='Senior official' value=domain.domain_info.senior_official contact='true' edit_link=url editable=is_editable %} {% endif %} @@ -92,7 +112,11 @@ {% include "includes/summary_item.html" with title='Security email' value='None provided' edit_link=url editable=is_editable %} {% endif %} {% url 'domain-users' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='Domain managers' users='true' list=True value=domain.permissions.all edit_link=url editable=is_editable %} + {% if portfolio %} + {% include "includes/summary_item.html" with title='Domain managers' domain_permissions=True value=domain edit_link=url editable=is_editable %} + {% else %} + {% include "includes/summary_item.html" with title='Domain managers' list=True users=True value=domain.permissions.all edit_link=url editable=is_editable %} + {% endif %}
{% endblock %} {# domain_content #} diff --git a/src/registrar/templates/domain_dns.html b/src/registrar/templates/domain_dns.html index 291319a59..9a2070c64 100644 --- a/src/registrar/templates/domain_dns.html +++ b/src/registrar/templates/domain_dns.html @@ -4,6 +4,24 @@ {% block title %}DNS | {{ domain.name }} | {% endblock %} {% block domain_content %} + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %}

DNS

diff --git a/src/registrar/templates/domain_dnssec.html b/src/registrar/templates/domain_dnssec.html index 7742a329b..cfec053c2 100644 --- a/src/registrar/templates/domain_dnssec.html +++ b/src/registrar/templates/domain_dnssec.html @@ -5,6 +5,28 @@ {% block domain_content %} + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} +

DNSSEC

DNSSEC, or DNS Security Extensions, is an additional security layer to protect your website. Enabling DNSSEC ensures that when someone visits your domain, they can be certain that it’s connecting to the correct server, preventing potential hijacking or tampering with your domain's records.

diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index ba742ab09..0f60235e1 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -4,6 +4,32 @@ {% block title %}DS data | {{ domain.name }} | {% endblock %} {% block domain_content %} + + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} + {% if domain.dnssecdata is None %}
diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html index cc1fc0164..a5fd171a2 100644 --- a/src/registrar/templates/domain_nameservers.html +++ b/src/registrar/templates/domain_nameservers.html @@ -4,6 +4,28 @@ {% block title %}DNS name servers | {{ domain.name }} | {% endblock %} {% block domain_content %} + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} + {# this is right after the messages block in the parent template #} {% for form in formset %} {% include "includes/form_errors.html" with form=form %} diff --git a/src/registrar/templates/domain_request_requesting_entity.html b/src/registrar/templates/domain_request_requesting_entity.html index f58ed0fae..9ed83f2d0 100644 --- a/src/registrar/templates/domain_request_requesting_entity.html +++ b/src/registrar/templates/domain_request_requesting_entity.html @@ -7,7 +7,11 @@ {% endblock %} {% block form_fields %} - + {% comment %} + Store the other option in a variable to be used by the js function handleRequestingEntity. + Injected into the 'sub_organization' option list. + {% endcomment %} +

Who will use the domain you’re requesting?

diff --git a/src/registrar/templates/domain_security_email.html b/src/registrar/templates/domain_security_email.html index e1755f85e..f5a58eb5d 100644 --- a/src/registrar/templates/domain_security_email.html +++ b/src/registrar/templates/domain_security_email.html @@ -4,6 +4,25 @@ {% block title %}Security email | {{ domain.name }} | {% endblock %} {% block domain_content %} + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} + {% include "includes/form_errors.html" with form=form %}

Security email

diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index 67726e9d5..648563d58 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -4,9 +4,30 @@ {% block title %}Suborganization{% if suborganization_name %} | suborganization_name{% endif %} | {% endblock %} {% block domain_content %} + + {% block breadcrumb %} + {% if portfolio %} + + + {% endif %} + {% endblock breadcrumb %} + {# this is right after the messages block in the parent template #} {% include "includes/form_errors.html" with form=form %} +

Suborganization

diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index b8a622455..af292d9d5 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -4,6 +4,25 @@ {% block title %}Domain managers | {{ domain.name }} | {% endblock %} {% block domain_content %} + {% block breadcrumb %} + {% if portfolio %} + +

+ {% endif %} + {% endblock breadcrumb %} +

Domain managers

{% comment %}Copy below differs depending on whether view is in portfolio mode.{% endcomment %} diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 0600d7ea7..15cc0f67f 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -106,6 +106,26 @@ {% endfor %} {% endif %} + {% elif domain_permissions %} + {% if value.permissions.all %} + {% if value.permissions|length == 1 %} +

{{ value.permissions.0.user.email }}

+ {% else %} +
    + {% for item in value.permissions.all %} +
  • {{ item.user.email }}
  • + {% endfor %} +
+ {% endif %} + {% endif %} + {% if value.invitations.all %} +

Invited domain managers

+
    + {% for item in value.invitations.all %} +
  • {{ item.email }}
  • + {% endfor %} +
+ {% endif %} {% else %}

{% if value %} diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 0c1bdec2a..46604a44a 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -824,6 +824,15 @@ class TestUser(TestCase): cm.exception.message, "When portfolio roles or additional permissions are assigned, portfolio is required." ) + @less_console_noise_decorator + def test_user_with_admin_portfolio_role(self): + portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") + self.assertFalse(self.user.is_portfolio_admin(portfolio)) + UserPortfolioPermission.objects.get_or_create( + portfolio=portfolio, user=self.user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + self.assertTrue(self.user.is_portfolio_admin(portfolio)) + @less_console_noise_decorator def test_get_active_requests_count_in_portfolio_returns_zero_if_no_portfolio(self): # There is no portfolio referenced in session so should return 0 diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 678d5be82..25e8b0fb6 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -6,7 +6,7 @@ from django.urls import reverse from django.contrib.auth import get_user_model from waffle.testutils import override_flag from api.tests.common import less_console_noise_decorator -from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .common import MockEppLib, MockSESClient, create_user # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore @@ -142,6 +142,7 @@ class TestWithDomainPermissions(TestWithUser): def tearDown(self): try: UserDomainRole.objects.all().delete() + DomainInvitation.objects.all().delete() if hasattr(self.domain, "contacts"): self.domain.contacts.all().delete() DomainRequest.objects.all().delete() @@ -341,7 +342,7 @@ class TestDomainDetail(TestDomainOverview): detail_page = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) self.assertNotContains( - detail_page, "To manage information for this domain, you must add yourself as a domain manager." + detail_page, "If you need to make updates, contact one of the listed domain managers." ) @less_console_noise_decorator @@ -363,7 +364,12 @@ class TestDomainDetail(TestDomainOverview): DomainInformation.objects.get_or_create(creator=user, domain=domain, portfolio=portfolio) UserPortfolioPermission.objects.get_or_create( - user=user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + user=user, + portfolio=portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, + ], ) user.refresh_from_db() self.client.force_login(user) @@ -377,6 +383,45 @@ class TestDomainDetail(TestDomainOverview): ) # Check that user does not have option to Edit domain self.assertNotContains(detail_page, "Edit") + # Check that invited domain manager section not displayed when no invited domain managers + self.assertNotContains(detail_page, "Invited domain managers") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_domain_readonly_on_detail_page_for_org_admin_not_manager(self): + """Test that a domain, which is part of a portfolio, but for which the user is not a domain manager, + properly displays read only""" + + portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test org", creator=self.user) + # need to create a different user than self.user because the user needs permission assignments + user = get_user_model().objects.create( + first_name="Test", + last_name="User", + email="bogus@example.gov", + phone="8003111234", + title="test title", + ) + domain, _ = Domain.objects.get_or_create(name="bogusdomain.gov") + DomainInformation.objects.get_or_create(creator=user, domain=domain, portfolio=portfolio) + + UserPortfolioPermission.objects.get_or_create( + user=user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + # add a domain invitation + DomainInvitation.objects.get_or_create(email="invited@example.com", domain=domain) + user.refresh_from_db() + self.client.force_login(user) + detail_page = self.client.get(f"/domain/{domain.id}") + # Check that alert message displays properly + self.assertContains( + detail_page, + "If you need to make updates, contact one of the listed domain managers.", + ) + # Check that user does not have option to Edit domain + self.assertNotContains(detail_page, "Edit") + # Check that invited domain manager is displayed + self.assertContains(detail_page, "Invited domain managers") + self.assertContains(detail_page, "invited@example.com") class TestDomainManagers(TestDomainOverview): diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 6f6b2c744..a03e51de5 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -198,8 +198,8 @@ class BaseExport(ABC): # We can infer that if we're passing in annotations, # we want to grab the result of said annotation. - if computed_fields : - related_table_fields.extend(computed_fields .keys()) + if computed_fields: + related_table_fields.extend(computed_fields.keys()) # Get prexisting fields on the model model_fields = set() @@ -263,25 +263,6 @@ class BaseExport(ABC): def get_model_annotation_dict(cls, **kwargs): return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) - @classmethod - def export_data_to_csv(cls, csv_file, **kwargs): - """ - All domain metadata: - Exports domains of all statuses plus domain managers. - """ - writer = csv.writer(csv_file) - columns = cls.get_columns() - models_dict = cls.get_model_annotation_dict(**kwargs) - - # Write to csv file before the write_csv - cls.write_csv_before(writer, **kwargs) - - # Write the csv file - rows = cls.write_csv(writer, columns, models_dict) - - # Return rows that for easier parsing and testing - return rows - @classmethod def write_csv( cls, diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index 232c4056f..47e6da47f 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -49,6 +49,7 @@ class DefaultUserValues(StrEnum): - SYSTEM: "System" <= Default username - UNRETRIEVED: "Unretrieved" <= Default email state """ + HELP_EMAIL = "help@get.gov" SYSTEM = "System" UNRETRIEVED = "Unretrieved" diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 232ca2e6c..b5c608eab 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -1,7 +1,6 @@ from django.http import JsonResponse from django.core.paginator import Paginator from django.db.models import Value, F, CharField, TextField, Q, Case, When, OuterRef, Subquery -from django.db.models.expressions import Func from django.db.models.functions import Cast, Coalesce, Concat from django.contrib.postgres.aggregates import ArrayAgg from django.urls import reverse @@ -214,4 +213,3 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): "svg_icon": ("visibility" if view_only else "settings"), } return member_json -