From be8a618791a4e4f1d7941cbbe8a212d5a02207d8 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 5 Sep 2024 00:13:18 -0400 Subject: [PATCH 01/32] wip --- src/registrar/assets/js/get-gov.js | 86 +++++++++++++++++-- .../assets/sass/_theme/_accordions.scss | 3 +- src/registrar/assets/sass/_theme/_base.scss | 5 ++ src/registrar/assets/sass/_theme/_header.scss | 6 +- src/registrar/config/urls.py | 5 ++ .../includes/domain_requests_table.html | 7 +- .../templates/includes/domains_table.html | 1 - .../templates/includes/header_extended.html | 34 ++++++-- .../portfolio_no_domain_requests.html | 30 +++++++ ...domains.html => portfolio_no_domains.html} | 0 src/registrar/views/domain_requests_json.py | 24 +++++- src/registrar/views/portfolios.py | 33 ++++++- 12 files changed, 212 insertions(+), 22 deletions(-) create mode 100644 src/registrar/templates/portfolio_no_domain_requests.html rename src/registrar/templates/{no_portfolio_domains.html => portfolio_no_domains.html} (100%) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 70659b009..0aebb7ff6 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1168,7 +1168,6 @@ document.addEventListener('DOMContentLoaded', function() { const statusCheckboxes = document.querySelectorAll('input[name="filter-status"]'); 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; @@ -1226,7 +1225,7 @@ document.addEventListener('DOMContentLoaded', function() { let markupForSuborganizationRow = ''; - if (!noPortfolioFlag) { + if (portfolioValue) { markupForSuborganizationRow = ` ${suborganization} @@ -1485,6 +1484,8 @@ document.addEventListener('DOMContentLoaded', function() { const tableHeaders = document.querySelectorAll('.domain-requests__table th[data-sortable]'); const tableAnnouncementRegion = document.querySelector('.domain-requests__table-wrapper .usa-table__announcement-region'); const resetSearchButton = document.querySelector('.domain-requests__reset-search'); + const portfolioElement = document.getElementById('portfolio-js-value'); + const portfolioValue = portfolioElement ? portfolioElement.getAttribute('data-portfolio') : null; /** * Delete is actually a POST API that requires a csrf token. The token will be waiting for us in the template as a hidden input. @@ -1533,7 +1534,7 @@ document.addEventListener('DOMContentLoaded', function() { * @param {*} scroll - control for the scrollToElement functionality * @param {*} searchTerm - the search term */ - function loadDomainRequests(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, searchTerm = currentSearchTerm) { + function loadDomainRequests(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, searchTerm = currentSearchTerm, portfolio = portfolioValue) { // fetch json of page of domain requests, given params let baseUrl = document.getElementById("get_domain_requests_json_url"); if (!baseUrl) { @@ -1545,7 +1546,12 @@ document.addEventListener('DOMContentLoaded', function() { return; } - fetch(`${baseUrlValue}?page=${page}&sort_by=${sortBy}&order=${order}&search_term=${searchTerm}`) + // fetch json of page of requests, given params + let url = `${baseUrlValue}?page=${page}&sort_by=${sortBy}&order=${order}&search_term=${searchTerm}` + if (portfolio) + url += `&portfolio=${portfolio}` + + fetch(url) .then(response => response.json()) .then(data => { if (data.error) { @@ -1601,10 +1607,11 @@ document.addEventListener('DOMContentLoaded', function() { const actionLabel = request.action_label; const submissionDate = request.last_submitted_date ? new Date(request.last_submitted_date).toLocaleDateString('en-US', options) : `Not submitted`; - // Even if the request is not deletable, we may need this empty string for the td if the deletable column is displayed + // Delete markup will either be a simple trigger or a 3 dots menu with a hidden trigger (in the case of portfolio requests page) + // Even if the request is not deletable, we may need these empty strings for the td if the deletable column is displayed let modalTrigger = ''; - // If the request is deletable, create modal body and insert it + // If the request is deletable, create modal body and insert it. This is true for both requests and portfolio requests pages if (request.is_deletable) { let modalHeading = ''; let modalDescription = ''; @@ -1692,8 +1699,45 @@ document.addEventListener('DOMContentLoaded', function() { ` domainRequestsSectionWrapper.appendChild(modal); + + // Request is deletable, modal and modalTrigger are built. Now test is portfolio requests page and enhace the modalTrigger markup + if (portfolioValue) { + modalTrigger = ` +
+
+ +
+ +
+ ` + } } + + const row = document.createElement('tr'); row.innerHTML = ` @@ -1817,6 +1861,36 @@ document.addEventListener('DOMContentLoaded', function() { }); } + function closeMoreActionMenu(accordionIsOpen) { + if (accordionIsOpen.getAttribute("aria-expanded") === "true") { + accordionIsOpen.click(); + } + } + + document.addEventListener('focusin', function(event) { + const accordions = document.querySelectorAll('.usa-accordion--more-actions'); + const openAccordions = document.querySelectorAll('.usa-button--more-actions[aria-expanded="true"]'); + + openAccordions.forEach((openAccordionButton) => { + const accordion = openAccordionButton.closest('.usa-accordion--more-actions'); // Find the corresponding accordion + if (accordion && !accordion.contains(event.target)) { + closeMoreActionMenu(openAccordionButton); // Close the accordion if the focus is outside + } + }); + }); + + document.addEventListener('click', function(event) { + const accordions = document.querySelectorAll('.usa-accordion--more-actions'); + const openAccordions = document.querySelectorAll('.usa-button--more-actions[aria-expanded="true"]'); + + openAccordions.forEach((openAccordionButton) => { + const accordion = openAccordionButton.closest('.usa-accordion--more-actions'); // Find the corresponding accordion + if (accordion && !accordion.contains(event.target)) { + closeMoreActionMenu(openAccordionButton); // Close the accordion if the click is outside + } + }); + }); + // Initial load loadDomainRequests(1); } diff --git a/src/registrar/assets/sass/_theme/_accordions.scss b/src/registrar/assets/sass/_theme/_accordions.scss index 839d7ac42..b422945cb 100644 --- a/src/registrar/assets/sass/_theme/_accordions.scss +++ b/src/registrar/assets/sass/_theme/_accordions.scss @@ -1,6 +1,7 @@ @use "uswds-core" as *; -.usa-accordion--select { +.usa-accordion--select, +.usa-accordion--more-actions { display: inline-block; width: auto; position: relative; diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index 9f8a0cbb6..16a63c41c 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -192,3 +192,8 @@ abbr[title] { max-width: 50ch; } } + +// Boost this USWDS utility class for the accordions in the portfolio requests table +.left-auto { + left: auto!important; +} diff --git a/src/registrar/assets/sass/_theme/_header.scss b/src/registrar/assets/sass/_theme/_header.scss index 3d72a09cf..726460c6f 100644 --- a/src/registrar/assets/sass/_theme/_header.scss +++ b/src/registrar/assets/sass/_theme/_header.scss @@ -89,14 +89,16 @@ .usa-nav__primary { .usa-nav-link, .usa-nav-link:hover, - .usa-nav-link:active { + .usa-nav-link:active, + button { color: color('primary'); font-weight: font-weight('normal'); font-size: 16px; } .usa-current, .usa-current:hover, - .usa-current:active { + .usa-current:active, + button.usa-current { font-weight: font-weight('bold'); } } diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 19fa99809..12de5d2a7 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -78,6 +78,11 @@ views.PortfolioDomainRequestsView.as_view(), name="domain-requests", ), + path( + "no-organization-requests/", + views.PortfolioNoDomainRequestsView.as_view(), + name="no-portfolio-requests", + ), path( "organization/", views.PortfolioOrganizationView.as_view(), diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index bd909350c..e5a3e046d 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -5,10 +5,13 @@
- {% if not has_domain_requests_portfolio_permission %} + {% if not portfolio %}

Domain requests

+ {% else %} + + {% endif %}
@@ -45,7 +48,7 @@

Domain requests

Domain name - Date submitted + Submitted Status Action diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 48de2d98c..a132d47c7 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -9,7 +9,6 @@
{% if not portfolio %}

Domains

- {% else %} diff --git a/src/registrar/templates/includes/header_extended.html b/src/registrar/templates/includes/header_extended.html index 54ceb3a67..07eeba157 100644 --- a/src/registrar/templates/includes/header_extended.html +++ b/src/registrar/templates/includes/header_extended.html @@ -16,7 +16,7 @@
  • {% if has_domains_portfolio_permission %} {% url 'domains' as url %} - {%else %} + {% else %} {% url 'no-portfolio-domains' as url %} {% endif %} @@ -29,14 +29,38 @@
    • - {% if has_domain_requests_portfolio_permission %} -
  • +
  • + {% if has_domain_requests_portfolio_permission %} {% url 'domain-requests' as url %} + + + {% else %} + {% url 'no-portfolio-requests' as url %} Domain requests -
    • - {% endif %} + {% endif %} + + +
  • Members diff --git a/src/registrar/templates/portfolio_no_domain_requests.html b/src/registrar/templates/portfolio_no_domain_requests.html new file mode 100644 index 000000000..bfee87bd2 --- /dev/null +++ b/src/registrar/templates/portfolio_no_domain_requests.html @@ -0,0 +1,30 @@ +{% extends 'portfolio_base.html' %} + +{% load static %} + +{% block title %} Domain Requests | {% endblock %} + +{% block portfolio_content %} +

    Current domain requests

    +
    +
    +

    You don’t have access to domain requests.

    + {% if portfolio_administrators %} +

    If you believe you should have access to a request, reach out to your organization’s administrators.

    +

    Your organizations administrators:

    +
      + {% for administrator in portfolio_administrators %} + {% if administrator.email %} +
    • {{ administrator.email }}
      • + {% else %} +
    • {{ administrator }}
      • + {% endif %} + {% endfor %} +
    + {% else %} +

    No administrators were found on your organization.

    +

    If you believe you should have access to a request, email help@get.gov.

    + {% endif %} +
    +
    +{% endblock %} diff --git a/src/registrar/templates/no_portfolio_domains.html b/src/registrar/templates/portfolio_no_domains.html similarity index 100% rename from src/registrar/templates/no_portfolio_domains.html rename to src/registrar/templates/portfolio_no_domains.html diff --git a/src/registrar/views/domain_requests_json.py b/src/registrar/views/domain_requests_json.py index 6b0b346f8..a1564e24b 100644 --- a/src/registrar/views/domain_requests_json.py +++ b/src/registrar/views/domain_requests_json.py @@ -12,9 +12,9 @@ def get_domain_requests_json(request): """Given the current request, get all domain requests that are associated with the request user and exclude the APPROVED ones""" - domain_requests = DomainRequest.objects.filter(creator=request.user).exclude( - status=DomainRequest.DomainRequestStatus.APPROVED - ) + domain_request_ids = get_domain_requests_ids_from_request(request) + + domain_requests = DomainRequest.objects.filter(id__in=domain_request_ids) unfiltered_total = domain_requests.count() # Handle sorting @@ -97,3 +97,21 @@ def get_domain_requests_json(request): "unfiltered_total": unfiltered_total, } ) + +def get_domain_requests_ids_from_request(request): + """Get domain request ids from request. + + If portfolio specified, return domain request ids associated with portfolio. + Otherwise, return domain request ids associated with request.user. + """ + portfolio = request.GET.get("portfolio") + if portfolio: + domain_requests = DomainRequest.objects.filter(portfolio=portfolio).exclude( + status=DomainRequest.DomainRequestStatus.APPROVED + ) + else: + domain_requests = DomainRequest.objects.filter(creator=request.user).exclude( + status=DomainRequest.DomainRequestStatus.APPROVED + ) + + return domain_requests.values_list("id", flat=True) \ No newline at end of file diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 0232b50d7..e4b91d4bf 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -42,12 +42,41 @@ def get(self, request): class PortfolioNoDomainsView(NoPortfolioDomainsPermissionView, View): - """Some users have access to the underlying portfolio, but not any domains. + """Some users have access to the underlying portfolio, but not any domains. This is a custom view which explains that to the user - and denotes who to contact. """ model = Portfolio - template_name = "no_portfolio_domains.html" + template_name = "portfolio_no_domains.html" + + def get(self, request): + return render(request, self.template_name, context=self.get_context_data()) + + def get_context_data(self, **kwargs): + """Add additional context data to the template.""" + # We can override the base class. This view only needs this item. + context = {} + portfolio = self.request.session.get("portfolio") + if portfolio: + admin_ids = UserPortfolioPermission.objects.filter( + portfolio=portfolio, + roles__overlap=[ + UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + ], + ).values_list("user__id", flat=True) + + admin_users = User.objects.filter(id__in=admin_ids) + context["portfolio_administrators"] = admin_users + return context + + +class PortfolioNoDomainRequestsView(NoPortfolioDomainsPermissionView, View): + """Some users have access to the underlying portfolio, but not any domain requests. + This is a custom view which explains that to the user - and denotes who to contact. + """ + + model = Portfolio + template_name = "portfolio_no_domain_requests.html" def get(self, request): return render(request, self.template_name, context=self.get_context_data()) From ba20e740b8807e3541517812e65c47346a85ace3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 5 Sep 2024 11:51:54 -0400 Subject: [PATCH 02/32] refactored domain_requests_json --- src/registrar/views/domain_requests_json.py | 155 +++++++++++--------- 1 file changed, 86 insertions(+), 69 deletions(-) diff --git a/src/registrar/views/domain_requests_json.py b/src/registrar/views/domain_requests_json.py index a1564e24b..6eefe02fc 100644 --- a/src/registrar/views/domain_requests_json.py +++ b/src/registrar/views/domain_requests_json.py @@ -14,81 +14,21 @@ def get_domain_requests_json(request): domain_request_ids = get_domain_requests_ids_from_request(request) - domain_requests = DomainRequest.objects.filter(id__in=domain_request_ids) - unfiltered_total = domain_requests.count() + objects = DomainRequest.objects.filter(id__in=domain_request_ids) + unfiltered_total = objects.count() - # Handle sorting - sort_by = request.GET.get("sort_by", "id") # Default to 'id' - order = request.GET.get("order", "asc") # Default to 'asc' - search_term = request.GET.get("search_term") - - if search_term: - search_term_lower = search_term.lower() - new_domain_request_text = "new domain request" + objects = apply_search(objects, request) + objects = apply_sorting(objects, request) - # Check if the search term is a substring of 'New domain request' - # If yes, we should return domain requests that do not have a - # requested_domain (those display as New domain request in the UI) - if search_term_lower in new_domain_request_text: - domain_requests = domain_requests.filter( - Q(requested_domain__name__icontains=search_term) | Q(requested_domain__isnull=True) - ) - else: - domain_requests = domain_requests.filter(Q(requested_domain__name__icontains=search_term)) - - if order == "desc": - sort_by = f"-{sort_by}" - domain_requests = domain_requests.order_by(sort_by) + paginator = Paginator(objects, 10) page_number = request.GET.get("page", 1) - paginator = Paginator(domain_requests, 10) page_obj = paginator.get_page(page_number) - domain_requests_data = [ - { - "requested_domain": domain_request.requested_domain.name if domain_request.requested_domain else None, - "last_submitted_date": domain_request.last_submitted_date, - "status": domain_request.get_status_display(), - "created_at": format(domain_request.created_at, "c"), # Serialize to ISO 8601 - "id": domain_request.id, - "is_deletable": domain_request.status - in [DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.WITHDRAWN], - "action_url": ( - reverse("edit-domain-request", kwargs={"id": domain_request.id}) - if domain_request.status - in [ - DomainRequest.DomainRequestStatus.STARTED, - DomainRequest.DomainRequestStatus.ACTION_NEEDED, - DomainRequest.DomainRequestStatus.WITHDRAWN, - ] - else reverse("domain-request-status", kwargs={"pk": domain_request.id}) - ), - "action_label": ( - "Edit" - if domain_request.status - in [ - DomainRequest.DomainRequestStatus.STARTED, - DomainRequest.DomainRequestStatus.ACTION_NEEDED, - DomainRequest.DomainRequestStatus.WITHDRAWN, - ] - else "Manage" - ), - "svg_icon": ( - "edit" - if domain_request.status - in [ - DomainRequest.DomainRequestStatus.STARTED, - DomainRequest.DomainRequestStatus.ACTION_NEEDED, - DomainRequest.DomainRequestStatus.WITHDRAWN, - ] - else "settings" - ), - } - for domain_request in page_obj - ] + domain_requests = [serialize_domain_request(domain_request) for domain_request in page_obj.object_list] return JsonResponse( { - "domain_requests": domain_requests_data, + "domain_requests": domain_requests, "has_next": page_obj.has_next(), "has_previous": page_obj.has_previous(), "page": page_obj.number, @@ -98,6 +38,7 @@ def get_domain_requests_json(request): } ) + def get_domain_requests_ids_from_request(request): """Get domain request ids from request. @@ -113,5 +54,81 @@ def get_domain_requests_ids_from_request(request): domain_requests = DomainRequest.objects.filter(creator=request.user).exclude( status=DomainRequest.DomainRequestStatus.APPROVED ) - - return domain_requests.values_list("id", flat=True) \ No newline at end of file + + return domain_requests.values_list("id", flat=True) + + +def apply_search(queryset, request): + search_term = request.GET.get("search_term") + + if search_term: + search_term_lower = search_term.lower() + new_domain_request_text = "new domain request" + + # Check if the search term is a substring of 'New domain request' + # If yes, we should return domain requests that do not have a + # requested_domain (those display as New domain request in the UI) + if search_term_lower in new_domain_request_text: + queryset = queryset.filter( + Q(requested_domain__name__icontains=search_term) | Q(requested_domain__isnull=True) + ) + else: + queryset = queryset.filter(Q(requested_domain__name__icontains=search_term)) + return queryset + + +def apply_sorting(queryset, request): + sort_by = request.GET.get("sort_by", "id") # Default to 'id' + order = request.GET.get("order", "asc") # Default to 'asc' + + if order == "desc": + sort_by = f"-{sort_by}" + return queryset.order_by(sort_by) + + +def serialize_domain_request(domain_request): + is_deletable = domain_request.status in [ + DomainRequest.DomainRequestStatus.STARTED, + DomainRequest.DomainRequestStatus.WITHDRAWN, + ] + action_url = ( + reverse("edit-domain-request", kwargs={"id": domain_request.id}) + if domain_request.status + in [ + DomainRequest.DomainRequestStatus.STARTED, + DomainRequest.DomainRequestStatus.ACTION_NEEDED, + DomainRequest.DomainRequestStatus.WITHDRAWN, + ] + else reverse("domain-request-status", kwargs={"pk": domain_request.id}) + ) + action_label = ( + "Edit" + if domain_request.status + in [ + DomainRequest.DomainRequestStatus.STARTED, + DomainRequest.DomainRequestStatus.ACTION_NEEDED, + DomainRequest.DomainRequestStatus.WITHDRAWN, + ] + else "Manage" + ) + svg_icon = ( + "edit" + if domain_request.status + in [ + DomainRequest.DomainRequestStatus.STARTED, + DomainRequest.DomainRequestStatus.ACTION_NEEDED, + DomainRequest.DomainRequestStatus.WITHDRAWN, + ] + else "settings" + ) + return { + "requested_domain": domain_request.requested_domain.name if domain_request.requested_domain else None, + "last_submitted_date": domain_request.last_submitted_date, + "status": domain_request.get_status_display(), + "created_at": format(domain_request.created_at, "c"), # Serialize to ISO 8601 + "id": domain_request.id, + "is_deletable": is_deletable, + "action_url": action_url, + "action_label": action_label, + "svg_icon": svg_icon, + } \ No newline at end of file From 8f8d9a8692735f1c0923011d4a6e4b2eb01ca3d7 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 5 Sep 2024 12:23:09 -0400 Subject: [PATCH 03/32] last row accordion placement --- src/registrar/assets/sass/_theme/_accordions.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/registrar/assets/sass/_theme/_accordions.scss b/src/registrar/assets/sass/_theme/_accordions.scss index b422945cb..93cd405dd 100644 --- a/src/registrar/assets/sass/_theme/_accordions.scss +++ b/src/registrar/assets/sass/_theme/_accordions.scss @@ -32,3 +32,8 @@ margin-top: 0 !important; } } + +tr:last-child .usa-accordion--more-actions .usa-accordion__content { + top: auto; + bottom: 20px; +} From 9bd67cfdbe2cf7c2034355a1fe18873019edade4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 5 Sep 2024 11:38:16 -0600 Subject: [PATCH 04/32] fix bug with registrar and basic redirect link stuff --- src/registrar/registrar_middleware.py | 15 +++++++++++---- .../templates/domain_request_status.html | 17 +++++++++++++++-- src/registrar/views/domain_request.py | 9 ++++++++- src/registrar/views/domain_requests_json.py | 2 ++ 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 4b590db1e..3ce6fde5d 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -144,8 +144,15 @@ def process_view(self, request, view_func, view_args, view_kwargs): if not request.user.is_authenticated: return None - # set the portfolio in the session if it is not set - if "portfolio" not in request.session or request.session["portfolio"] is None: + old_updated_at = None + if request.session.get("portfolio"): + old_updated_at = request.session.get("portfolio__updated_at") + request.session["portfolio__updated_at"] = request.session.get("portfolio").updated_at + + should_update_portfolio = ( + not request.session.get("portfolio") or old_updated_at != request.session.get("portfolio__updated_at") + ) + if request.user.is_org_user(request) or should_update_portfolio: # if multiple portfolios are allowed for this user if flag_is_active(request, "multiple_portfolios"): # NOTE: we will want to change later to have a workflow for selecting @@ -156,8 +163,8 @@ def process_view(self, request, view_func, view_args, view_kwargs): else: request.session["portfolio"] = None - if request.session["portfolio"] is not None and current_path == self.home: - if request.user.is_org_user(request): + if request.session.get("portfolio"): + if current_path == self.home: if request.user.has_domains_portfolio_permission(request.session["portfolio"]): portfolio_redirect = reverse("domains") else: diff --git a/src/registrar/templates/domain_request_status.html b/src/registrar/templates/domain_request_status.html index 183a8be81..40722c41e 100644 --- a/src/registrar/templates/domain_request_status.html +++ b/src/registrar/templates/domain_request_status.html @@ -8,13 +8,26 @@ {% block content %}
    - + {% comment %} + The back button should redirect to the domain request page if we are in the portfolio view. + Otherwise, lets just redirect back to home. + {% endcomment %} + {% if portfolio %} + {% url 'domain-requests' as url%} + {% else %} + {% url 'home' as url%} + {% endif %} +

    - Back to manage your domains + {% if portfolio %} + Back to manage your domains requests + {% else %} + Back to manage your domains + {% endif %}

    Domain request for {{ DomainRequest.requested_domain.name }}

    diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index b691549cd..fbadeb51e 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -152,7 +152,14 @@ def domain_request(self) -> DomainRequest: except DomainRequest.DoesNotExist: logger.debug("DomainRequest id %s did not have a DomainRequest" % id) - self._domain_request = DomainRequest.objects.create(creator=self.request.user) + # If a user is creating a request, we assume that perms are handled upstream + if self.request.user.is_org_user(self.request): + self._domain_request = DomainRequest.objects.create( + creator=self.request.user, + portfolio=self.request.session.get("portfolio"), + ) + else: + self._domain_request = DomainRequest.objects.create(creator=self.request.user) self.storage["domain_request_id"] = self._domain_request.id return self._domain_request diff --git a/src/registrar/views/domain_requests_json.py b/src/registrar/views/domain_requests_json.py index 6eefe02fc..85e46b261 100644 --- a/src/registrar/views/domain_requests_json.py +++ b/src/registrar/views/domain_requests_json.py @@ -47,6 +47,8 @@ def get_domain_requests_ids_from_request(request): """ portfolio = request.GET.get("portfolio") if portfolio: + # Question: + # Do we need to exclude started if the creator isn't the current request user domain_requests = DomainRequest.objects.filter(portfolio=portfolio).exclude( status=DomainRequest.DomainRequestStatus.APPROVED ) From d802b9a614faef2f42edc61162b22c5e0ab81833 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 5 Sep 2024 15:01:37 -0400 Subject: [PATCH 05/32] Add created by column --- src/registrar/assets/js/get-gov.js | 16 +++++++++++++--- .../includes/domain_requests_table.html | 3 +++ src/registrar/views/domain_requests_json.py | 1 + 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 33e6ed384..fd4b3f941 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1611,6 +1611,16 @@ document.addEventListener('DOMContentLoaded', function() { // Even if the request is not deletable, we may need these empty strings for the td if the deletable column is displayed let modalTrigger = ''; + let markupCreatorRow = ''; + + if (portfolioValue) { + markupCreatorRow = ` + + ${request.creator ? request.creator : ''} + + ` + } + // If the request is deletable, create modal body and insert it. This is true for both requests and portfolio requests pages if (request.is_deletable) { let modalHeading = ''; @@ -1634,7 +1644,7 @@ document.addEventListener('DOMContentLoaded', function() { role="button" id="button-toggle-delete-domain-alert-${request.id}" href="#toggle-delete-domain-alert-${request.id}" - class="usa-button--unstyled text-no-underline late-loading-modal-trigger" + class="usa-button usa-button--unstyled text-no-underline late-loading-modal-trigger" aria-controls="toggle-delete-domain-alert-${request.id}" data-open-modal > @@ -1707,7 +1717,7 @@ document.addEventListener('DOMContentLoaded', function() {
    " @@ -418,6 +422,7 @@ def get_context_data(self): You’ll only be able to withdraw your request.", "review_form_is_complete": True, "user": self.request.user, + "requested_domain__name": requested_domain_name, } else: # form is not complete modal_button = '' @@ -433,6 +438,7 @@ def get_context_data(self): Return to the request and visit the steps that are marked as "incomplete."', "review_form_is_complete": False, "user": self.request.user, + "requested_domain__name": requested_domain_name, } return context_stuff @@ -512,7 +518,10 @@ def post(self, request, *args, **kwargs) -> HttpResponse: # if user opted to save progress and return, # return them to the home page if button == "save_and_return": - return HttpResponseRedirect(reverse("home")) + if request.user.is_org_user(request): + return HttpResponseRedirect(reverse("domain-requests")) + else: + return HttpResponseRedirect(reverse("home")) # otherwise, proceed as normal return self.goto_next_step() @@ -781,7 +790,10 @@ def get(self, *args, **kwargs): domain_request = DomainRequest.objects.get(id=self.kwargs["pk"]) domain_request.withdraw() domain_request.save() - return HttpResponseRedirect(reverse("home")) + if self.request.user.is_org_user(self.request): + return HttpResponseRedirect(reverse("domain-requests")) + else: + return HttpResponseRedirect(reverse("home")) class DomainRequestDeleteView(DomainRequestPermissionDeleteView): From 9ea0f312fca798eb5cd92154af1c4f4fb51214b3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 5 Sep 2024 15:36:08 -0400 Subject: [PATCH 07/32] CSS tweaks --- src/registrar/assets/js/get-gov.js | 6 +++--- src/registrar/assets/sass/_theme/_base.scss | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index fd4b3f941..734e7918d 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1616,7 +1616,7 @@ document.addEventListener('DOMContentLoaded', function() { if (portfolioValue) { markupCreatorRow = ` - ${request.creator ? request.creator : ''} + ${request.creator ? request.creator : ''} ` } @@ -1644,7 +1644,7 @@ document.addEventListener('DOMContentLoaded', function() { role="button" id="button-toggle-delete-domain-alert-${request.id}" href="#toggle-delete-domain-alert-${request.id}" - class="usa-button usa-button--unstyled text-no-underline late-loading-modal-trigger" + class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger" aria-controls="toggle-delete-domain-alert-${request.id}" data-open-modal > @@ -1732,7 +1732,7 @@ document.addEventListener('DOMContentLoaded', function() { role="button" id="button-toggle-delete-domain-alert-${request.id}" href="#toggle-delete-domain-alert-${request.id}" - class="usa-button--unstyled text-no-underline late-loading-modal-trigger" + class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger" aria-controls="toggle-delete-domain-alert-${request.id}" data-open-modal > diff --git a/src/registrar/assets/sass/_theme/_base.scss b/src/registrar/assets/sass/_theme/_base.scss index 7df783604..4ea9d2054 100644 --- a/src/registrar/assets/sass/_theme/_base.scss +++ b/src/registrar/assets/sass/_theme/_base.scss @@ -204,3 +204,7 @@ abbr[title] { .left-auto { left: auto!important; } + +.break-word { + word-break: break-word; +} From faf57902f08613223bb8f34a46dfd2c94d08b5cc Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 5 Sep 2024 19:22:31 -0400 Subject: [PATCH 08/32] filter requests based on permissions, updated actions based on permissions --- src/registrar/context_processors.py | 18 ++-- src/registrar/models/user.py | 42 ++++----- .../models/utility/portfolio_helper.py | 1 - src/registrar/templates/domain_detail.html | 4 +- src/registrar/templates/domain_sidebar.html | 2 +- .../templates/domain_suborganization.html | 2 +- .../templates/includes/domains_table.html | 2 +- .../templates/includes/header_extended.html | 2 +- src/registrar/tests/test_models.py | 24 +++--- src/registrar/views/domain_requests_json.py | 85 +++++++++---------- src/registrar/views/utility/mixins.py | 2 +- 11 files changed, 91 insertions(+), 93 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index ea04dca80..41dfb9cad 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -66,20 +66,20 @@ def portfolio_permissions(request): return { "has_base_portfolio_permission": request.user.has_base_portfolio_permission(portfolio), "has_domains_portfolio_permission": request.user.has_domains_portfolio_permission(portfolio), - "has_domain_requests_portfolio_permission": request.user.has_domain_requests_portfolio_permission( + "has_requests_portfolio_permission": request.user.has_requests_portfolio_permission( portfolio ), - "has_view_suborganization": request.user.has_view_suborganization(portfolio), - "has_edit_suborganization": request.user.has_edit_suborganization(portfolio), + "has_view_suborganization_portfolio_permission": request.user.has_view_suborganization_portfolio_permission(portfolio), + "has_edit_suborganization_portfolio_permission": request.user.has_edit_suborganization_portfolio_permission(portfolio), "portfolio": portfolio, "has_organization_feature_flag": True, } return { "has_base_portfolio_permission": False, "has_domains_portfolio_permission": False, - "has_domain_requests_portfolio_permission": False, - "has_view_suborganization": False, - "has_edit_suborganization": False, + "has_requests_portfolio_permission": False, + "has_view_suborganization_portfolio_permission": False, + "has_edit_suborganization_portfolio_permission": False, "portfolio": None, "has_organization_feature_flag": False, } @@ -89,9 +89,9 @@ def portfolio_permissions(request): return { "has_base_portfolio_permission": False, "has_domains_portfolio_permission": False, - "has_domain_requests_portfolio_permission": False, - "has_view_suborganization": False, - "has_edit_suborganization": False, + "has_requests_portfolio_permission": False, + "has_view_suborganization_portfolio_permission": False, + "has_edit_suborganization_portfolio_permission": False, "portfolio": None, "has_organization_feature_flag": False, } diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 8d91c2a8c..0b5dc3f8d 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -223,20 +223,27 @@ def has_domains_portfolio_permission(self, portfolio): portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) - def has_domain_requests_portfolio_permission(self, portfolio): + def has_view_all_domains_portfolio_permission(self, portfolio): + """Determines if the current user can view all available domains in a given portfolio""" + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + + def has_requests_portfolio_permission(self, portfolio): return self._has_portfolio_permission( portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS - ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) + ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) - def has_view_all_domains_permission(self, portfolio): - """Determines if the current user can view all available domains in a given portfolio""" - return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + def has_view_all_requests_portfolio_permission(self, portfolio): + """Determines if the current user can view all available domain requests in a given portfolio""" + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) + def has_edit_request_portfolio_permission(self, portfolio): + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) + # Field specific permission checks - def has_view_suborganization(self, portfolio): + def has_view_suborganization_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) - def has_edit_suborganization(self, portfolio): + def has_edit_suborganization_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) def get_first_portfolio(self): @@ -245,34 +252,31 @@ def get_first_portfolio(self): return permission.portfolio return None - def has_edit_requests(self, portfolio): - return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) - def portfolio_role_summary(self, portfolio): """Returns a list of roles based on the user's permissions.""" roles = [] # Define the conditions and their corresponding roles conditions_roles = [ - (self.has_edit_suborganization(portfolio), ["Admin"]), + (self.has_edit_suborganization_portfolio_permission(portfolio), ["Admin"]), ( - self.has_view_all_domains_permission(portfolio) - and self.has_domain_requests_portfolio_permission(portfolio) - and self.has_edit_requests(portfolio), + self.has_view_all_domains_portfolio_permission(portfolio) + and self.has_requests_portfolio_permission(portfolio) + and self.has_edit_request_portfolio_permission(portfolio), ["View-only admin", "Domain requestor"], ), ( - self.has_view_all_domains_permission(portfolio) - and self.has_domain_requests_portfolio_permission(portfolio), + self.has_view_all_domains_portfolio_permission(portfolio) + and self.has_requests_portfolio_permission(portfolio), ["View-only admin"], ), ( self.has_base_portfolio_permission(portfolio) - and self.has_edit_requests(portfolio) + and self.has_edit_request_portfolio_permission(portfolio) and self.has_domains_portfolio_permission(portfolio), ["Domain requestor", "Domain manager"], ), - (self.has_base_portfolio_permission(portfolio) and self.has_edit_requests(portfolio), ["Domain requestor"]), + (self.has_base_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), ["Domain requestor"]), ( self.has_base_portfolio_permission(portfolio) and self.has_domains_portfolio_permission(portfolio), ["Domain manager"], @@ -443,7 +447,7 @@ def is_org_user(self, request): def get_user_domain_ids(self, request): """Returns either the domains ids associated with this user on UserDomainRole or Portfolio""" portfolio = request.session.get("portfolio") - if self.is_org_user(request) and self.has_view_all_domains_permission(portfolio): + if self.is_org_user(request) and self.has_view_all_domains_portfolio_permission(portfolio): return DomainInformation.objects.filter(portfolio=portfolio).values_list("domain_id", flat=True) else: return UserDomainRole.objects.filter(user=self).values_list("domain_id", flat=True) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 86aaa5e16..d87f981c7 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -21,7 +21,6 @@ class UserPortfolioPermissionChoices(models.TextChoices): EDIT_MEMBER = "edit_member", "Create and edit members" VIEW_ALL_REQUESTS = "view_all_requests", "View all requests" - VIEW_CREATED_REQUESTS = "view_created_requests", "View created requests" EDIT_REQUESTS = "edit_requests", "Create and edit requests" VIEW_PORTFOLIO = "view_portfolio", "View organization" diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index d7bc277b3..d93f313c1 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -72,9 +72,9 @@

    DNS name servers

    {% include "includes/summary_item.html" with title='DNSSEC' value='Not Enabled' edit_link=url editable=is_editable %} {% endif %} - {% if portfolio and has_domains_portfolio_permission and has_view_suborganization %} + {% if portfolio and has_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 %} + {% 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 %} {% else %} {% url 'domain-org-name-address' pk=domain.id as url %} {% include "includes/summary_item.html" with title='Organization name and mailing address' value=domain.domain_info address='true' edit_link=url editable=is_editable %} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 24f92bf16..82a3f8ed1 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -61,7 +61,7 @@ {% if portfolio %} {% comment %} Only show this menu option if the user has the perms to do so {% endcomment %} - {% if has_domains_portfolio_permission and has_view_suborganization %} + {% if has_domains_portfolio_permission and has_view_suborganization_portfolio_permission %} {% with url_name="domain-suborganization" %} {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %} {% endwith %} diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html index 823629213..d0c41ff09 100644 --- a/src/registrar/templates/domain_suborganization.html +++ b/src/registrar/templates/domain_suborganization.html @@ -15,7 +15,7 @@

    Suborganization

    If you believe there is an error please contact help@get.gov.

    - {% if has_domains_portfolio_permission and has_edit_suborganization %} + {% if has_domains_portfolio_permission and has_edit_suborganization_portfolio_permission %}
    {% csrf_token %} {% input_with_errors form.sub_organization %} diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index f4cf5a0fe..6c805ff84 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -156,7 +156,7 @@

    Status

    Domain name Expires Status - {% if portfolio and has_view_suborganization %} + {% if portfolio and has_view_suborganization_portfolio_permission %} Suborganization {% endif %}
  • - {% if has_domain_requests_portfolio_permission %} + {% if has_requests_portfolio_permission %} {% url 'domain-requests' as url %}
    • - {% endif %} + {% endif %} {% endcomment %} {% endif %} {% block form_messages %} diff --git a/src/registrar/templates/domain_request_status.html b/src/registrar/templates/domain_request_status.html index c2cb45ede..f4defc14e 100644 --- a/src/registrar/templates/domain_request_status.html +++ b/src/registrar/templates/domain_request_status.html @@ -9,15 +9,9 @@
    {% comment %} - The back button should redirect to the domain request page if we are in the portfolio view. - Otherwise, lets just redirect back to home. - {% endcomment %} + TODO: Uncomment in #2596 {% if portfolio %} {% url 'domain-requests' as url %} - {% else %} - {% url 'home' as url %} - {% endif %} - {% if portfolio %} - {% else %} - - - -

    - Back to manage your domains -

    -     - {% endif %} + {% else %}{% endcomment %} + {% url 'home' as url %} + + + +

    + Back to manage your domains +

    +     + {% comment %} {% endif %}{% endcomment %}

    Domain request for {{ DomainRequest.requested_domain.name }}

    Date: Fri, 6 Sep 2024 12:54:36 -0400 Subject: [PATCH 11/32] CSS done --- src/registrar/assets/js/get-gov.js | 17 ++++++++-- .../assets/sass/_theme/_accordions.scss | 12 +++++-- src/registrar/assets/sass/_theme/_base.scss | 17 ++++++++++ .../assets/sass/_theme/_buttons.scss | 13 +++----- src/registrar/assets/sass/_theme/_header.scss | 6 ++++ src/registrar/context_processors.py | 31 ++++++++--------- src/registrar/templates/domain_dsdata.html | 2 +- .../templates/domain_nameservers.html | 2 +- .../domain_request_other_contacts.html | 2 +- .../templates/includes/header_extended.html | 10 +++++- ...quests.html => portfolio_no_requests.html} | 0 .../templates/portfolio_requests.html | 33 ++++++++++++------- src/registrar/views/portfolios.py | 2 +- 13 files changed, 100 insertions(+), 47 deletions(-) rename src/registrar/templates/{portfolio_no_domain_requests.html => portfolio_no_requests.html} (100%) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 734e7918d..d216de49a 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1713,7 +1713,20 @@ document.addEventListener('DOMContentLoaded', function() { // Request is deletable, modal and modalTrigger are built. Now test is portfolio requests page and enhace the modalTrigger markup if (portfolioValue) { modalTrigger = ` -
    + + Delete ${domainName} + + +
    -
  • - {% if has_requests_portfolio_permission %} + + {% if has_edit_request_portfolio_permission %} {% url 'domain-requests' as url %}
    • + + {% elif has_requests_portfolio_permission %} + {% url 'domain-requests' as url %} + + Domain requests + + {% else %} {% url 'no-portfolio-requests' as url %} diff --git a/src/registrar/templates/portfolio_no_domain_requests.html b/src/registrar/templates/portfolio_no_requests.html similarity index 100% rename from src/registrar/templates/portfolio_no_domain_requests.html rename to src/registrar/templates/portfolio_no_requests.html diff --git a/src/registrar/templates/portfolio_requests.html b/src/registrar/templates/portfolio_requests.html index 9f97a25aa..868c9bd2a 100644 --- a/src/registrar/templates/portfolio_requests.html +++ b/src/registrar/templates/portfolio_requests.html @@ -11,18 +11,27 @@ {% block portfolio_content %}

    Domain requests

    - - {% comment %} - IMPORTANT: - If this button is added on any other page, make sure to update the - relevant view to reset request.session["new_request"] = True - {% endcomment %} -

    - - Start a new domain request - -

    +
    +
    +

    Domain requests can only be modified by the person who created the request.

    +
    + {% if has_edit_request_portfolio_permission %} +
    + {% comment %} + IMPORTANT: + If this button is added on any other page, make sure to update the + relevant view to reset request.session["new_request"] = True + {% endcomment %} +

    + + Start a new domain request + +

    +
    + {% endif %} +
    + {% include "includes/domain_requests_table.html" with portfolio=portfolio %}
    diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index e4b91d4bf..885dca636 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -76,7 +76,7 @@ class PortfolioNoDomainRequestsView(NoPortfolioDomainsPermissionView, View): """ model = Portfolio - template_name = "portfolio_no_domain_requests.html" + template_name = "portfolio_no_requests.html" def get(self, request): return render(request, self.template_name, context=self.get_context_data()) From 1e9ae7befc61c76c3df93935f0b22f84737320bb Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 6 Sep 2024 13:46:52 -0400 Subject: [PATCH 12/32] cleanup --- src/registrar/assets/js/get-gov.js | 4 ++-- src/registrar/assets/sass/_theme/_base.scss | 12 +++++----- .../assets/sass/_theme/_buttons.scss | 2 ++ src/registrar/context_processors.py | 8 +++---- src/registrar/models/user.py | 12 +++++----- src/registrar/registrar_middleware.py | 2 +- src/registrar/templates/domain_detail.html | 2 +- src/registrar/templates/domain_sidebar.html | 2 +- .../templates/domain_suborganization.html | 2 +- .../templates/includes/header_extended.html | 4 ++-- src/registrar/tests/test_models.py | 24 +++++++++---------- src/registrar/tests/test_views_portfolio.py | 6 ++--- src/registrar/views/domain.py | 2 +- src/registrar/views/utility/mixins.py | 4 ++-- 14 files changed, 44 insertions(+), 42 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index aa186dc31..1cb1e6828 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1717,7 +1717,7 @@ document.addEventListener('DOMContentLoaded', function() { role="button" id="button-toggle-delete-domain-alert-${request.id}" href="#toggle-delete-domain-alert-${request.id}" - class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger margin-top-2 visible-mobile" + class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger margin-top-2 visible-mobile-flex" aria-controls="toggle-delete-domain-alert-${request.id}" data-open-modal > @@ -1726,7 +1726,7 @@ document.addEventListener('DOMContentLoaded', function() { Delete ${domainName} -
    +
    • - {% if has_domains_portfolio_permission %} + {% if has_any_domains_portfolio_permission %} {% url 'domains' as url %} {% else %} {% url 'no-portfolio-domains' as url %} @@ -77,7 +77,7 @@
    - {% elif has_requests_portfolio_permission %} + {% elif has_any_requests_portfolio_permission %} {% url 'domain-requests' as url %} Domain requests diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index cdf008c57..26f4c381f 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1334,7 +1334,7 @@ def test_portfolio_role_summary_admin(self, mock_edit_suborganization): @patch.multiple( User, has_view_all_domains_portfolio_permission=lambda self, portfolio: True, - has_requests_portfolio_permission=lambda self, portfolio: True, + has_any_requests_portfolio_permission=lambda self, portfolio: True, has_edit_request_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_view_only_admin_and_domain_requestor(self): @@ -1344,7 +1344,7 @@ def test_portfolio_role_summary_view_only_admin_and_domain_requestor(self): @patch.multiple( User, has_view_all_domains_portfolio_permission=lambda self, portfolio: True, - has_requests_portfolio_permission=lambda self, portfolio: True, + has_any_requests_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_view_only_admin(self): # Test if the user is recognized as a View-only admin @@ -1354,7 +1354,7 @@ def test_portfolio_role_summary_view_only_admin(self): User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_request_portfolio_permission=lambda self, portfolio: True, - has_domains_portfolio_permission=lambda self, portfolio: True, + has_any_domains_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_requestor_domain_manager(self): # Test if the user has 'Member', 'Domain requestor', and 'Domain manager' roles @@ -1370,7 +1370,7 @@ def test_portfolio_role_summary_member_domain_requestor(self): @patch.multiple( User, has_base_portfolio_permission=lambda self, portfolio: True, - has_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): # Test if the user has 'Member' and 'Domain manager' roles @@ -1546,8 +1546,8 @@ def test_has_portfolio_permission(self): portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") - user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) - user_can_view_all_requests = self.user.has_requests_portfolio_permission(portfolio) + user_can_view_all_domains = self.user.has_any_domains_portfolio_permission(portfolio) + user_can_view_all_requests = self.user.has_any_requests_portfolio_permission(portfolio) self.assertFalse(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) @@ -1558,8 +1558,8 @@ def test_has_portfolio_permission(self): additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS], ) - user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) - user_can_view_all_requests = self.user.has_requests_portfolio_permission(portfolio) + user_can_view_all_domains = self.user.has_any_domains_portfolio_permission(portfolio) + user_can_view_all_requests = self.user.has_any_requests_portfolio_permission(portfolio) self.assertTrue(user_can_view_all_domains) self.assertFalse(user_can_view_all_requests) @@ -1568,16 +1568,16 @@ def test_has_portfolio_permission(self): portfolio_permission.save() portfolio_permission.refresh_from_db() - user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) - user_can_view_all_requests = self.user.has_requests_portfolio_permission(portfolio) + user_can_view_all_domains = self.user.has_any_domains_portfolio_permission(portfolio) + user_can_view_all_requests = self.user.has_any_requests_portfolio_permission(portfolio) self.assertTrue(user_can_view_all_domains) self.assertTrue(user_can_view_all_requests) UserDomainRole.objects.get_or_create(user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER) - user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) - user_can_view_all_requests = self.user.has_requests_portfolio_permission(portfolio) + user_can_view_all_domains = self.user.has_any_domains_portfolio_permission(portfolio) + user_can_view_all_requests = self.user.has_any_requests_portfolio_permission(portfolio) self.assertTrue(user_can_view_all_domains) self.assertTrue(user_can_view_all_requests) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index c5d1a9830..2e7bb978e 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -502,7 +502,7 @@ def test_org_member_can_only_see_domains_with_appropriate_permissions(self): self.client.force_login(self.user) response = self.client.get(reverse("home"), follow=True) - self.assertFalse(self.user.has_domains_portfolio_permission(response.wsgi_request.session.get("portfolio"))) + self.assertFalse(self.user.has_any_domains_portfolio_permission(response.wsgi_request.session.get("portfolio"))) self.assertEqual(response.status_code, 200) self.assertContains(response, "You aren") @@ -517,7 +517,7 @@ def test_org_member_can_only_see_domains_with_appropriate_permissions(self): # Test the domains page - this user should have access response = self.client.get(reverse("domains")) - self.assertTrue(self.user.has_domains_portfolio_permission(response.wsgi_request.session.get("portfolio"))) + self.assertTrue(self.user.has_any_domains_portfolio_permission(response.wsgi_request.session.get("portfolio"))) self.assertEqual(response.status_code, 200) self.assertContains(response, "Domain name") @@ -528,7 +528,7 @@ def test_org_member_can_only_see_domains_with_appropriate_permissions(self): # Test the domains page - this user should have access response = self.client.get(reverse("domains")) - self.assertTrue(self.user.has_domains_portfolio_permission(response.wsgi_request.session.get("portfolio"))) + self.assertTrue(self.user.has_any_domains_portfolio_permission(response.wsgi_request.session.get("portfolio"))) self.assertEqual(response.status_code, 200) self.assertContains(response, "Domain name") permission.delete() diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 003f8dd0d..511c55228 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -175,7 +175,7 @@ def can_access_domain_via_portfolio(self, pk): If particular views allow permissions, they will need to override this function.""" portfolio = self.request.session.get("portfolio") - if self.request.user.has_domains_portfolio_permission(portfolio): + if self.request.user.has_any_domains_portfolio_permission(portfolio): if Domain.objects.filter(id=pk).exists(): domain = Domain.objects.get(id=pk) if domain.domain_info.portfolio == portfolio: diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 48c0ccb61..24483b6ef 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -433,7 +433,7 @@ def has_permission(self): up from the portfolio's primary key in self.kwargs["pk"]""" portfolio = self.request.session.get("portfolio") - if not self.request.user.has_domains_portfolio_permission(portfolio): + if not self.request.user.has_any_domains_portfolio_permission(portfolio): return False return super().has_permission() @@ -450,7 +450,7 @@ def has_permission(self): up from the portfolio's primary key in self.kwargs["pk"]""" portfolio = self.request.session.get("portfolio") - if not self.request.user.has_requests_portfolio_permission(portfolio): + if not self.request.user.has_any_requests_portfolio_permission(portfolio): return False return super().has_permission() From 7a67845b6daeff5562623199473148596ba717e2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 6 Sep 2024 12:11:49 -0600 Subject: [PATCH 13/32] bug fix --- src/registrar/registrar_middleware.py | 4 ++++ src/registrar/tests/test_views_portfolio.py | 5 +++++ src/registrar/views/domain_request.py | 1 + 3 files changed, 10 insertions(+) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 0bedec3ca..5323d513f 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -162,6 +162,10 @@ def process_view(self, request, view_func, view_args, view_kwargs): request.session["portfolio"] = request.user.get_first_portfolio() else: request.session["portfolio"] = None + else: + # Edge case: waffle flag is changed while the user is logged in + if not request.user.is_org_user(request) and request.session.get("portfolio"): + request.session["portfolio"] = None if request.session.get("portfolio"): if current_path == self.home: diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 2e7bb978e..b8ed5c074 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -2,6 +2,7 @@ from api.tests.common import less_console_noise_decorator from registrar.config import settings from registrar.models import Portfolio, SeniorOfficial +from unittest import skip from django_webtest import WebTest # type: ignore from registrar.models import ( DomainRequest, @@ -532,3 +533,7 @@ def test_org_member_can_only_see_domains_with_appropriate_permissions(self): self.assertEqual(response.status_code, 200) self.assertContains(response, "Domain name") permission.delete() + + @skip("TODO") + def test_portfolio_cache_updates_when_modified(self): + pass diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 7c19c3af2..5beb74e94 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -522,6 +522,7 @@ def post(self, request, *args, **kwargs) -> HttpResponse: return HttpResponseRedirect(reverse("domain-requests")) else: return HttpResponseRedirect(reverse("home")) + # otherwise, proceed as normal return self.goto_next_step() From 1af08a6b4803bd755a5588af19b181c0a3e4dd9b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 6 Sep 2024 16:44:35 -0400 Subject: [PATCH 14/32] model and view unit tests --- src/registrar/tests/test_models.py | 67 ++++++++++++ src/registrar/tests/test_views_portfolio.py | 112 +++++++++++++++++++- 2 files changed, 178 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 26f4c381f..936e644d0 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1385,6 +1385,73 @@ def test_portfolio_role_summary_empty(self): # Test if the user has no roles self.assertEqual(self.user.portfolio_role_summary(self.portfolio), []) + @patch('registrar.models.User._has_portfolio_permission') + def test_has_base_portfolio_permission(self, mock_has_permission): + mock_has_permission.return_value = True + + self.assertTrue(self.user.has_base_portfolio_permission(self.portfolio)) + mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO) + + @patch('registrar.models.User._has_portfolio_permission') + def test_has_edit_org_portfolio_permission(self, mock_has_permission): + mock_has_permission.return_value = True + + self.assertTrue(self.user.has_edit_org_portfolio_permission(self.portfolio)) + mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_PORTFOLIO) + + @patch('registrar.models.User._has_portfolio_permission') + def test_has_any_domains_portfolio_permission(self, mock_has_permission): + mock_has_permission.side_effect = [False, True] # First permission false, second permission true + + self.assertTrue(self.user.has_any_domains_portfolio_permission(self.portfolio)) + self.assertEqual(mock_has_permission.call_count, 2) + mock_has_permission.assert_any_call(self.portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + mock_has_permission.assert_any_call(self.portfolio, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) + + @patch('registrar.models.User._has_portfolio_permission') + def test_has_view_all_domains_portfolio_permission(self, mock_has_permission): + mock_has_permission.return_value = True + + self.assertTrue(self.user.has_view_all_domains_portfolio_permission(self.portfolio)) + mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + + @patch('registrar.models.User._has_portfolio_permission') + def test_has_any_requests_portfolio_permission(self, mock_has_permission): + mock_has_permission.side_effect = [False, True] # First permission false, second permission true + + self.assertTrue(self.user.has_any_requests_portfolio_permission(self.portfolio)) + self.assertEqual(mock_has_permission.call_count, 2) + mock_has_permission.assert_any_call(self.portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) + mock_has_permission.assert_any_call(self.portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) + + @patch('registrar.models.User._has_portfolio_permission') + def test_has_view_all_requests_portfolio_permission(self, mock_has_permission): + mock_has_permission.return_value = True + + self.assertTrue(self.user.has_view_all_requests_portfolio_permission(self.portfolio)) + mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS) + + @patch('registrar.models.User._has_portfolio_permission') + def test_has_edit_request_portfolio_permission(self, mock_has_permission): + mock_has_permission.return_value = True + + self.assertTrue(self.user.has_edit_request_portfolio_permission(self.portfolio)) + 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 def test_check_transition_domains_without_domains_on_login(self): """A user's on_each_login callback does not check transition domains. diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index b8ed5c074..7d3c6f3be 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -13,9 +13,10 @@ ) from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from .common import create_test_user +from .common import MockSESClient, completed_domain_request, create_test_user from waffle.testutils import override_flag from django.contrib.sessions.middleware import SessionMiddleware +import boto3_mocking # type: ignore import logging @@ -534,6 +535,115 @@ def test_org_member_can_only_see_domains_with_appropriate_permissions(self): self.assertContains(response, "Domain name") permission.delete() + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_portfolio_domain_requests_page_when_user_has_no_permissions(self): + """Test the no requests page""" + UserPortfolioPermission.objects.get_or_create( + user=self.user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) + self.client.force_login(self.user) + # create and submit a domain request + domain_request = completed_domain_request(user=self.user) + mock_client = MockSESClient() + with boto3_mocking.clients.handler_for("sesv2", mock_client): + domain_request.submit() + domain_request.save() + + requests_page = self.client.get(reverse("no-portfolio-requests"), follow=True) + + self.assertContains(requests_page, "You don’t have access to domain requests.") + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_main_nav_when_user_has_no_permissions(self): + """Test the nav contains a link to the no requests page""" + UserPortfolioPermission.objects.get_or_create( + user=self.user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) + self.client.force_login(self.user) + # create and submit a domain request + domain_request = completed_domain_request(user=self.user) + mock_client = MockSESClient() + with boto3_mocking.clients.handler_for("sesv2", mock_client): + domain_request.submit() + domain_request.save() + + portfolio_landing_page = self.client.get(reverse("home"), follow=True) + + # link to no requests + self.assertContains(portfolio_landing_page, "no-organization-requests/") + # dropdown + self.assertNotContains(portfolio_landing_page, "basic-nav-section-two") + # link to requests + self.assertNotContains(portfolio_landing_page, 'href="/requests/') + # link to create + self.assertNotContains(portfolio_landing_page, 'href="/request/') + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_main_nav_when_user_has_all_permissions(self): + """Test the nav contains a dropdown with a link to create and another link to view requests + Also test for the existence of the Create a new request btn on the requests page""" + UserPortfolioPermission.objects.get_or_create( + user=self.user, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + self.client.force_login(self.user) + # create and submit a domain request + domain_request = completed_domain_request(user=self.user) + mock_client = MockSESClient() + with boto3_mocking.clients.handler_for("sesv2", mock_client): + domain_request.submit() + domain_request.save() + + portfolio_landing_page = self.client.get(reverse("home"), follow=True) + + # link to no requests + self.assertNotContains(portfolio_landing_page, "no-organization-requests/") + # dropdown + self.assertContains(portfolio_landing_page, "basic-nav-section-two") + # link to requests + self.assertContains(portfolio_landing_page, 'href="/requests/') + # link to create + self.assertContains(portfolio_landing_page, 'href="/request/') + + requests_page = self.client.get(reverse("domain-requests")) + + # create new request btn + self.assertContains(requests_page, 'Start a new domain request') + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_main_nav_when_user_has_view_but_not_edit_permissions(self): + """Test the nav contains a simple link to view requests + Also test for the existence of the Create a new request btn on the requests page""" + UserPortfolioPermission.objects.get_or_create( + user=self.user, portfolio=self.portfolio, roles=[UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS] + ) + self.client.force_login(self.user) + # create and submit a domain request + domain_request = completed_domain_request(user=self.user) + mock_client = MockSESClient() + with boto3_mocking.clients.handler_for("sesv2", mock_client): + domain_request.submit() + domain_request.save() + + portfolio_landing_page = self.client.get(reverse("home"), follow=True) + + # link to no requests + self.assertNotContains(portfolio_landing_page, "no-organization-requests/") + # dropdown + self.assertNotContains(portfolio_landing_page, "basic-nav-section-two") + # link to requests + self.assertContains(portfolio_landing_page, 'href="/requests/') + # link to create + self.assertNotContains(portfolio_landing_page, 'href="/request/') + + requests_page = self.client.get(reverse("domain-requests")) + + # create new request btn + self.assertNotContains(requests_page, 'Start a new domain request') + @skip("TODO") def test_portfolio_cache_updates_when_modified(self): pass From 9f557fc1e171e4f20278f768575563534bd97848 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 6 Sep 2024 17:27:56 -0400 Subject: [PATCH 15/32] requests_json tests and some linting --- src/registrar/context_processors.py | 28 ++-- src/registrar/models/user.py | 9 +- src/registrar/registrar_middleware.py | 4 +- src/registrar/tests/test_models.py | 4 +- .../tests/test_views_requests_json.py | 141 ++++++++++++++++++ src/registrar/views/domain_requests_json.py | 16 +- 6 files changed, 175 insertions(+), 27 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index 5d04d5b70..ac30bb0af 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -61,27 +61,29 @@ def add_has_profile_feature_flag_to_context(request): def portfolio_permissions(request): """Make portfolio permissions for the request user available in global context""" default_context = { - "has_base_portfolio_permission": False, - "has_domains_portfolio_permission": False, - "has_requests_portfolio_permission": False, - "has_edit_request_portfolio_permission": False, - "has_view_suborganization_portfolio_permission": False, - "has_edit_suborganization_portfolio_permission": False, - "portfolio": None, - "has_organization_feature_flag": False, - } + "has_base_portfolio_permission": False, + "has_domains_portfolio_permission": False, + "has_requests_portfolio_permission": False, + "has_edit_request_portfolio_permission": False, + "has_view_suborganization_portfolio_permission": False, + "has_edit_suborganization_portfolio_permission": False, + "portfolio": None, + "has_organization_feature_flag": False, + } try: portfolio = request.session.get("portfolio") if portfolio: return { "has_base_portfolio_permission": request.user.has_base_portfolio_permission(portfolio), "has_domains_portfolio_permission": request.user.has_domains_portfolio_permission(portfolio), - "has_requests_portfolio_permission": request.user.has_requests_portfolio_permission( + "has_requests_portfolio_permission": request.user.has_requests_portfolio_permission(portfolio), + "has_edit_request_portfolio_permission": request.user.has_edit_request_portfolio_permission(portfolio), + "has_view_suborganization_portfolio_permission": request.user.has_view_suborganization_portfolio_permission( + portfolio + ), + "has_edit_suborganization_portfolio_permission": request.user.has_edit_suborganization_portfolio_permission( portfolio ), - "has_edit_request_portfolio_permission": request.user.has_edit_request_portfolio_permission(portfolio), - "has_view_suborganization_portfolio_permission": request.user.has_view_suborganization_portfolio_permission(portfolio), - "has_edit_suborganization_portfolio_permission": request.user.has_edit_suborganization_portfolio_permission(portfolio), "portfolio": portfolio, "has_organization_feature_flag": True, } diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 0b5dc3f8d..9b4924231 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -226,7 +226,7 @@ def has_domains_portfolio_permission(self, portfolio): def has_view_all_domains_portfolio_permission(self, portfolio): """Determines if the current user can view all available domains in a given portfolio""" return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) - + def has_requests_portfolio_permission(self, portfolio): return self._has_portfolio_permission( portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS @@ -238,7 +238,7 @@ def has_view_all_requests_portfolio_permission(self, portfolio): def has_edit_request_portfolio_permission(self, portfolio): 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) @@ -276,7 +276,10 @@ def portfolio_role_summary(self, portfolio): and self.has_domains_portfolio_permission(portfolio), ["Domain requestor", "Domain manager"], ), - (self.has_base_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), ["Domain requestor"]), + ( + self.has_base_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), + ["Domain requestor"], + ), ( self.has_base_portfolio_permission(portfolio) and self.has_domains_portfolio_permission(portfolio), ["Domain manager"], diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 3ce6fde5d..85f7b7ffc 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -149,8 +149,8 @@ def process_view(self, request, view_func, view_args, view_kwargs): old_updated_at = request.session.get("portfolio__updated_at") request.session["portfolio__updated_at"] = request.session.get("portfolio").updated_at - should_update_portfolio = ( - not request.session.get("portfolio") or old_updated_at != request.session.get("portfolio__updated_at") + should_update_portfolio = not request.session.get("portfolio") or old_updated_at != request.session.get( + "portfolio__updated_at" ) if request.user.is_org_user(request) or should_update_portfolio: # if multiple portfolios are allowed for this user diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index cdf008c57..140299be5 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1361,7 +1361,9 @@ def test_portfolio_role_summary_member_domain_requestor_domain_manager(self): self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor", "Domain manager"]) @patch.multiple( - User, has_base_portfolio_permission=lambda self, portfolio: True, has_edit_request_portfolio_permission=lambda self, portfolio: True + User, + has_base_portfolio_permission=lambda self, portfolio: True, + has_edit_request_portfolio_permission=lambda self, portfolio: True, ) def test_portfolio_role_summary_member_domain_requestor(self): # Test if the user has 'Member' and 'Domain requestor' roles diff --git a/src/registrar/tests/test_views_requests_json.py b/src/registrar/tests/test_views_requests_json.py index 20a4069f7..cef608567 100644 --- a/src/registrar/tests/test_views_requests_json.py +++ b/src/registrar/tests/test_views_requests_json.py @@ -2,9 +2,14 @@ from django.urls import reverse from registrar.models.draft_domain import DraftDomain +from registrar.models.portfolio import Portfolio +from registrar.models.user import User +from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from .test_views import TestWithUser from django_webtest import WebTest # type: ignore from django.utils.dateparse import parse_datetime +from waffle.testutils import override_flag class GetRequestsJsonTest(TestWithUser, WebTest): @@ -20,6 +25,19 @@ def setUpClass(cls): beef_chuck, _ = DraftDomain.objects.get_or_create(name="beef-chuck.gov") stew_beef, _ = DraftDomain.objects.get_or_create(name="stew-beef.gov") + # Create Portfolio + cls.portfolio = Portfolio.objects.create(creator=cls.user, organization_name="Example org") + + # create a second user to assign requests to + cls.user2 = User.objects.create( + username="test_user2", + first_name="Second", + last_name="last", + email="info2@example.com", + phone="8003111234", + title="title", + ) + # Create domain requests for the user cls.domain_requests = [ DomainRequest.objects.create( @@ -28,6 +46,7 @@ def setUpClass(cls): last_submitted_date="2024-01-01", status=DomainRequest.DomainRequestStatus.STARTED, created_at="2024-01-01", + portfolio=cls.portfolio, ), DomainRequest.objects.create( creator=cls.user, @@ -42,6 +61,7 @@ def setUpClass(cls): last_submitted_date="2024-03-01", status=DomainRequest.DomainRequestStatus.REJECTED, created_at="2024-03-01", + portfolio=cls.portfolio, ), DomainRequest.objects.create( creator=cls.user, @@ -113,6 +133,14 @@ def setUpClass(cls): status=DomainRequest.DomainRequestStatus.APPROVED, created_at="2024-12-01", ), + DomainRequest.objects.create( + creator=cls.user2, + requested_domain=None, + last_submitted_date="2024-12-01", + status=DomainRequest.DomainRequestStatus.STARTED, + created_at="2024-12-01", + portfolio=cls.portfolio, + ), ] @classmethod @@ -120,6 +148,7 @@ def tearDownClass(cls): super().tearDownClass() DomainRequest.objects.all().delete() DraftDomain.objects.all().delete() + Portfolio.objects.all().delete() def test_get_domain_requests_json_authenticated(self): """Test that domain requests are returned properly for an authenticated user.""" @@ -262,6 +291,118 @@ def test_get_domain_requests_json_search_new_domains(self): for expected_value, actual_value in zip(expected_domain_values, requested_domains): self.assertEqual(expected_value, actual_value) + @override_flag("organization_feature", active=True) + def test_get_domain_requests_json_with_portfolio_view_all_requests(self): + """Test that an authenticated user gets the list of 3 requests for portfolio. The 3 requests + are the requests that are associated with the portfolio.""" + + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS], + ) + + response = self.app.get(reverse("get_domain_requests_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 requests + self.assertEqual(len(data["domain_requests"]), 3) + + # Expected domain requests + expected_domain_requests = [self.domain_requests[0], self.domain_requests[2], self.domain_requests[13]] + + # Extract fields from response + domain_request_ids = [domain_request["id"] for domain_request in data["domain_requests"]] + requested_domain = [domain_request["requested_domain"] for domain_request in data["domain_requests"]] + creator = [domain_request["creator"] for domain_request in data["domain_requests"]] + status = [domain_request["status"] for domain_request in data["domain_requests"]] + action_urls = [domain_request["action_url"] for domain_request in data["domain_requests"]] + action_labels = [domain_request["action_label"] for domain_request in data["domain_requests"]] + svg_icons = [domain_request["svg_icon"] for domain_request in data["domain_requests"]] + + # Check fields for each domain_request + for i, expected_domain_request in enumerate(expected_domain_requests): + self.assertEqual(expected_domain_request.id, domain_request_ids[i]) + if expected_domain_request.requested_domain: + self.assertEqual(expected_domain_request.requested_domain.name, requested_domain[i]) + else: + self.assertIsNone(requested_domain[i]) + self.assertEqual(expected_domain_request.creator.email, creator[i]) + # Check action url, action label and svg icon + # Example domain requests will test each of below three scenarios + if creator[i] != self.user.email: + # Test case where action is View + self.assertEqual("View", action_labels[i]) + self.assertEqual("#", action_urls[i]) + self.assertEqual("visibility", svg_icons[i]) + elif status[i] in [ + DomainRequest.DomainRequestStatus.STARTED.label, + DomainRequest.DomainRequestStatus.ACTION_NEEDED.label, + DomainRequest.DomainRequestStatus.WITHDRAWN.label, + ]: + # Test case where action is Edit + self.assertEqual("Edit", action_labels[i]) + self.assertEqual( + reverse("edit-domain-request", kwargs={"id": expected_domain_request.id}), action_urls[i] + ) + self.assertEqual("edit", svg_icons[i]) + else: + # Test case where action is Manage + self.assertEqual("Manage", action_labels[i]) + self.assertEqual( + reverse("domain-request-status", kwargs={"pk": expected_domain_request.id}), action_urls[i] + ) + self.assertEqual("settings", svg_icons[i]) + + @override_flag("organization_feature", active=True) + def test_get_domain_requests_json_with_portfolio_edit_requests(self): + """Test that an authenticated user gets the list of 2 requests for portfolio. The 2 requests + are the requests that are associated with the portfolio and owned by self.user.""" + + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS], + ) + + response = self.app.get(reverse("get_domain_requests_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 requests + self.assertEqual(len(data["domain_requests"]), 2) + + # Expected domain requests + expected_domain_requests = [self.domain_requests[0], self.domain_requests[2]] + + # Extract fields from response, since other tests test all fields, only ids and requested + # domains tested in this test + domain_request_ids = [domain_request["id"] for domain_request in data["domain_requests"]] + requested_domain = [domain_request["requested_domain"] for domain_request in data["domain_requests"]] + + # Check fields for each domain_request + for i, expected_domain_request in enumerate(expected_domain_requests): + self.assertEqual(expected_domain_request.id, domain_request_ids[i]) + if expected_domain_request.requested_domain: + self.assertEqual(expected_domain_request.requested_domain.name, requested_domain[i]) + else: + self.assertIsNone(requested_domain[i]) + def test_pagination(self): """Test that pagination works properly. There are 11 total non-approved requests and a page size of 10""" diff --git a/src/registrar/views/domain_requests_json.py b/src/registrar/views/domain_requests_json.py index ffb4d51c9..fe86e7a7f 100644 --- a/src/registrar/views/domain_requests_json.py +++ b/src/registrar/views/domain_requests_json.py @@ -24,7 +24,9 @@ def get_domain_requests_json(request): page_number = request.GET.get("page", 1) page_obj = paginator.get_page(page_number) - domain_requests = [serialize_domain_request(domain_request, request.user) for domain_request in page_obj.object_list] + domain_requests = [ + serialize_domain_request(domain_request, request.user) for domain_request in page_obj.object_list + ] return JsonResponse( { @@ -52,7 +54,9 @@ def get_domain_request_ids_from_request(request): filter_condition = Q(portfolio=portfolio) else: filter_condition = Q(portfolio=portfolio, creator=request.user) - domain_requests = DomainRequest.objects.filter(filter_condition).exclude(status=DomainRequest.DomainRequestStatus.APPROVED) + domain_requests = DomainRequest.objects.filter(filter_condition).exclude( + status=DomainRequest.DomainRequestStatus.APPROVED + ) return domain_requests.values_list("id", flat=True) @@ -107,14 +111,10 @@ def serialize_domain_request(domain_request, user): action_url_map = { "Edit": reverse("edit-domain-request", kwargs={"id": domain_request.id}), "Manage": reverse("domain-request-status", kwargs={"pk": domain_request.id}), - "View": "#" + "View": "#", } - svg_icon_map = { - "Edit": "edit", - "Manage": "settings", - "View": "visibility" - } + svg_icon_map = {"Edit": "edit", "Manage": "settings", "View": "visibility"} return { "requested_domain": domain_request.requested_domain.name if domain_request.requested_domain else None, From 9019792bc8363f64566604270db11386cdc3bf93 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 6 Sep 2024 15:31:26 -0600 Subject: [PATCH 16/32] Fix existing tests --- src/registrar/registrar_middleware.py | 42 ++++++++++----------- src/registrar/tests/test_models.py | 2 +- src/registrar/tests/test_views_portfolio.py | 2 +- 3 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 5323d513f..bdc76bd25 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -6,7 +6,7 @@ from urllib.parse import parse_qs from django.urls import reverse from django.http import HttpResponseRedirect -from registrar.models.user import User +from registrar.models import User, Portfolio from waffle.decorators import flag_is_active from registrar.models.utility.generic_helper import replace_url_queryparams @@ -144,28 +144,17 @@ def process_view(self, request, view_func, view_args, view_kwargs): if not request.user.is_authenticated: return None - old_updated_at = None - if request.session.get("portfolio"): + portfolio = request.session.get("portfolio") + + # if multiple portfolios are allowed for this user + if flag_is_active(request, "organization_feature"): old_updated_at = request.session.get("portfolio__updated_at") - request.session["portfolio__updated_at"] = request.session.get("portfolio").updated_at - - should_update_portfolio = ( - not request.session.get("portfolio") or old_updated_at != request.session.get("portfolio__updated_at") - ) - if request.user.is_org_user(request) or should_update_portfolio: - # if multiple portfolios are allowed for this user - if flag_is_active(request, "multiple_portfolios"): - # NOTE: we will want to change later to have a workflow for selecting - # portfolio and another for switching portfolio; for now, select first - request.session["portfolio"] = request.user.get_first_portfolio() - elif flag_is_active(request, "organization_feature"): - request.session["portfolio"] = request.user.get_first_portfolio() - else: - request.session["portfolio"] = None - else: - # Edge case: waffle flag is changed while the user is logged in - if not request.user.is_org_user(request) and request.session.get("portfolio"): - request.session["portfolio"] = None + request.session["portfolio__updated_at"] = portfolio.updated_at if portfolio else None + if request.user.is_org_user(request) or old_updated_at != request.session.get("portfolio__updated_at"): + self.set_portfolio_in_session(request) + elif request.session.get("portfolio"): + # Edge case: User disables flag while already logged in + request.session["portfolio"] = None if request.session.get("portfolio"): if current_path == self.home: @@ -173,7 +162,14 @@ def process_view(self, request, view_func, view_args, view_kwargs): portfolio_redirect = reverse("domains") else: portfolio_redirect = reverse("no-portfolio-domains") - return HttpResponseRedirect(portfolio_redirect) return None + + def set_portfolio_in_session(self, request): + # NOTE: we will want to change later to have a workflow for selecting + # portfolio and another for switching portfolio; for now, select first + if flag_is_active(request, "multiple_portfolios"): + request.session["portfolio"] = request.user.get_first_portfolio() + else: + request.session["portfolio"] = request.user.get_first_portfolio() diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 936e644d0..b8cf2a824 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1135,7 +1135,7 @@ def setUp(self): self.portfolio, _ = Portfolio.objects.get_or_create(creator=self.user2, organization_name="Hotel California") self.portfolio_role_base = UserPortfolioRoleChoices.ORGANIZATION_MEMBER self.portfolio_role_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN - self.portfolio_permission_1 = UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS + self.portfolio_permission_1 = UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS self.portfolio_permission_2 = UserPortfolioPermissionChoices.EDIT_REQUESTS self.invitation, _ = PortfolioInvitation.objects.get_or_create( email=self.email, diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 7d3c6f3be..10c8724f5 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -78,7 +78,7 @@ def test_portfolio_senior_official(self): def test_middleware_does_not_redirect_if_no_permission(self): """Test that user with no portfolio permission is not redirected when attempting to access home""" self.app.set_user(self.user.username) - portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + UserPortfolioPermission.objects.get_or_create( user=self.user, portfolio=self.portfolio, additional_permissions=[] ) self.user.portfolio = self.portfolio From 540bb641e252cdac02f1701335b993a6406911b8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 6 Sep 2024 15:43:48 -0600 Subject: [PATCH 17/32] update session on each request --- src/registrar/registrar_middleware.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index bdc76bd25..aa0b98c5a 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -144,14 +144,9 @@ def process_view(self, request, view_func, view_args, view_kwargs): if not request.user.is_authenticated: return None - portfolio = request.session.get("portfolio") - # if multiple portfolios are allowed for this user if flag_is_active(request, "organization_feature"): - old_updated_at = request.session.get("portfolio__updated_at") - request.session["portfolio__updated_at"] = portfolio.updated_at if portfolio else None - if request.user.is_org_user(request) or old_updated_at != request.session.get("portfolio__updated_at"): - self.set_portfolio_in_session(request) + self.set_portfolio_in_session(request) elif request.session.get("portfolio"): # Edge case: User disables flag while already logged in request.session["portfolio"] = None From 3ed5e8c4d72aea939152b9a9d199d5538ad7e584 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 6 Sep 2024 17:46:55 -0400 Subject: [PATCH 18/32] Fix delete btn alignment --- src/registrar/assets/js/get-gov.js | 6 +++--- src/registrar/templates/domain_dsdata.html | 2 +- src/registrar/templates/domain_nameservers.html | 2 +- src/registrar/templates/domain_request_other_contacts.html | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 1cb1e6828..4bff491c9 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1644,7 +1644,7 @@ document.addEventListener('DOMContentLoaded', function() { role="button" id="button-toggle-delete-domain-alert-${request.id}" href="#toggle-delete-domain-alert-${request.id}" - class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger" + class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger line-height-sans-5" aria-controls="toggle-delete-domain-alert-${request.id}" data-open-modal > @@ -1717,7 +1717,7 @@ document.addEventListener('DOMContentLoaded', function() { role="button" id="button-toggle-delete-domain-alert-${request.id}" href="#toggle-delete-domain-alert-${request.id}" - class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger margin-top-2 visible-mobile-flex" + class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger margin-top-2 visible-mobile-flex line-height-sans-5" aria-controls="toggle-delete-domain-alert-${request.id}" data-open-modal > @@ -1745,7 +1745,7 @@ document.addEventListener('DOMContentLoaded', function() { role="button" id="button-toggle-delete-domain-alert-${request.id}" href="#toggle-delete-domain-alert-${request.id}" - class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger margin-top-2" + class="usa-button text-secondary usa-button--unstyled text-no-underline late-loading-modal-trigger margin-top-2 line-height-sans-5" aria-controls="toggle-delete-domain-alert-${request.id}" data-open-modal > diff --git a/src/registrar/templates/domain_dsdata.html b/src/registrar/templates/domain_dsdata.html index bc3cbc008..6e18bce13 100644 --- a/src/registrar/templates/domain_dsdata.html +++ b/src/registrar/templates/domain_dsdata.html @@ -63,7 +63,7 @@

    DS data record {{forloop.counter}}

    -
    -