From dcb63d194733b94e0fa0e15840beb04c06e6a6e6 Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Mon, 8 Jul 2024 18:57:46 -0400
Subject: [PATCH] Fix issues with org_user_status global context and
 logo_clickable profile context

---
 src/djangooidc/tests/test_views.py            |   4 +
 .../assets/sass/_theme/_identifier.scss       |   3 +-
 src/registrar/templates/base.html             |   6 +-
 src/registrar/templates/dashboard_base.html   |   1 -
 .../templates/finish_profile_setup.html       |   4 +-
 .../templates/includes/header_basic.html      |  73 ++++-----
 .../templates/includes/header_extended.html   |   6 +-
 .../templates/includes/header_selector.html   |   5 +
 src/registrar/templates/portfolio_base.html   |   2 -
 src/registrar/templates/profile.html          |   4 +-
 src/registrar/views/domains_json.py           | 151 ++++++++++--------
 11 files changed, 132 insertions(+), 127 deletions(-)
 create mode 100644 src/registrar/templates/includes/header_selector.html

diff --git a/src/djangooidc/tests/test_views.py b/src/djangooidc/tests/test_views.py
index bdd61b346..6ebe25d45 100644
--- a/src/djangooidc/tests/test_views.py
+++ b/src/djangooidc/tests/test_views.py
@@ -429,6 +429,10 @@ class ViewsTest(TestCase):
             # Create a mock request
             request = self.factory.get("/some-url")
             request.session = {"acr_value": ""}
+            # Mock user and its attributes
+            mock_user = MagicMock()
+            mock_user.is_authenticated = True
+            request.user = mock_user
             # Ensure that the CLIENT instance used in login_callback is the mock
             # patch _requires_step_up_auth to return False
             with patch("djangooidc.views._requires_step_up_auth", return_value=False), patch(
diff --git a/src/registrar/assets/sass/_theme/_identifier.scss b/src/registrar/assets/sass/_theme/_identifier.scss
index e56d887b9..58ccd659c 100644
--- a/src/registrar/assets/sass/_theme/_identifier.scss
+++ b/src/registrar/assets/sass/_theme/_identifier.scss
@@ -6,5 +6,4 @@
 
 .usa-identifier__logo {
     height: units(7);
-  }
-  
\ No newline at end of file
+}
diff --git a/src/registrar/templates/base.html b/src/registrar/templates/base.html
index a7b2bb783..e5bd1b0b9 100644
--- a/src/registrar/templates/base.html
+++ b/src/registrar/templates/base.html
@@ -135,11 +135,7 @@
 
   <div class="usa-overlay"></div>
   {% block header %}
-    {% if not is_org_user %}
-      {% include "includes/header_basic.html" %}
-    {% else %}
-      {% include "includes/header_extended.html" %}
-    {% endif %}
+    {% include "includes/header_selector.html" with logo_clickable=True %}
   {% endblock header %}
 
   {% block wrapper %}
diff --git a/src/registrar/templates/dashboard_base.html b/src/registrar/templates/dashboard_base.html
index 4f24b9280..6dd2ce8fd 100644
--- a/src/registrar/templates/dashboard_base.html
+++ b/src/registrar/templates/dashboard_base.html
@@ -9,7 +9,6 @@
       {% block content %}
         {% block messages %}
           {% if messages %}
-          <p>test</p>
           <ul class="messages">
             {% for message in messages %}
               <li {% if message.tags %} class="{{ message.tags }}" {% endif %}>
diff --git a/src/registrar/templates/finish_profile_setup.html b/src/registrar/templates/finish_profile_setup.html
index 6e35ad5da..61cc192bf 100644
--- a/src/registrar/templates/finish_profile_setup.html
+++ b/src/registrar/templates/finish_profile_setup.html
@@ -4,8 +4,8 @@
 {% block title %} Finish setting up your profile | {% endblock %}
 
 {# Disable the redirect  #}
-{% block logo %}
-    {% include "includes/gov_extended_logo.html" with logo_clickable=user_finished_setup %}
+{% block header %}
+    {% include "includes/header_selector.html" with logo_clickable=user_finished_setup %}
 {% endblock %}
 
 {# Add the new form #}
diff --git a/src/registrar/templates/includes/header_basic.html b/src/registrar/templates/includes/header_basic.html
index 7076fcb89..fd3b71000 100644
--- a/src/registrar/templates/includes/header_basic.html
+++ b/src/registrar/templates/includes/header_basic.html
@@ -1,42 +1,39 @@
 {% load static %}
 
 <header class="usa-header usa-header--basic">
-    <div class="usa-nav-container">
-      <div class="usa-navbar">
-          {% block logo %}
-            {% include "includes/gov_extended_logo.html" with logo_clickable=True %}
-          {% endblock %}
-        <button type="button" class="usa-menu-btn">Menu</button>
-      </div>
-      {% block usa_nav %}
-      <nav class="usa-nav" aria-label="Primary navigation">
-        <button type="button" class="usa-nav__close">
-          <img src="{%static 'img/usa-icons/close.svg'%}" role="img" alt="Close" />
-        </button>
-        <ul class="usa-nav__primary usa-accordion">
-          <li class="usa-nav__primary-item">
-            {% if user.is_authenticated %}
-            <span class="usa-nav__username ellipsis">{{ user.email }}</span>
-          </li>
-          {% if has_profile_feature_flag %}
-          <li class="usa-nav__primary-item">
-            {% url 'user-profile' as user_profile_url %}
-            {% url 'finish-user-profile-setup' as finish_setup_url %}
-            <a class="usa-nav-link {% if request.path == user_profile_url or request.path == finish_setup_url %}usa-current{% endif %}" href="{{ user_profile_url }}">
-                <span class="text-primary">Your profile</span>
-            </a>
-          </li>
-          {% endif %}
-          <li class="usa-nav__primary-item">
-            <a href="{% url 'logout' %}"><span class="text-primary">Sign out</span></a>
-            {% else %}
-            <a href="{% url 'login' %}"><span>Sign in</span></a>
-            {% endif %}
-          </li>
-        </ul>
-      </nav>
-      {% block usa_nav_secondary %}{% endblock %}
-      {% endblock %}
+  <div class="usa-nav-container">
+    <div class="usa-navbar">
+        {% include "includes/gov_extended_logo.html" with logo_clickable=logo_clickable %}
+      <button type="button" class="usa-menu-btn">Menu</button>
     </div>
-  </header>
-  
\ No newline at end of file
+    {% block usa_nav %}
+    <nav class="usa-nav" aria-label="Primary navigation">
+      <button type="button" class="usa-nav__close">
+        <img src="{%static 'img/usa-icons/close.svg'%}" role="img" alt="Close" />
+      </button>
+      <ul class="usa-nav__primary usa-accordion">
+        <li class="usa-nav__primary-item">
+          {% if user.is_authenticated %}
+          <span class="usa-nav__username ellipsis">{{ user.email }}</span>
+        </li>
+        {% if has_profile_feature_flag %}
+        <li class="usa-nav__primary-item">
+          {% url 'user-profile' as user_profile_url %}
+          {% url 'finish-user-profile-setup' as finish_setup_url %}
+          <a class="usa-nav-link {% if request.path == user_profile_url or request.path == finish_setup_url %}usa-current{% endif %}" href="{{ user_profile_url }}">
+              <span class="text-primary">Your profile</span>
+          </a>
+        </li>
+        {% endif %}
+        <li class="usa-nav__primary-item">
+          <a href="{% url 'logout' %}"><span class="text-primary">Sign out</span></a>
+          {% else %}
+          <a href="{% url 'login' %}"><span>Sign in</span></a>
+          {% endif %}
+        </li>
+      </ul>
+    </nav>
+    {% block usa_nav_secondary %}{% endblock %}
+    {% endblock %}
+  </div>
+</header>
diff --git a/src/registrar/templates/includes/header_extended.html b/src/registrar/templates/includes/header_extended.html
index ccba93ea3..f388a71bc 100644
--- a/src/registrar/templates/includes/header_extended.html
+++ b/src/registrar/templates/includes/header_extended.html
@@ -2,9 +2,7 @@
 
 <header class="usa-header usa-header--extended">
     <div class="usa-navbar">
-        {% block logo %}
-        {% include "includes/gov_extended_logo.html" with logo_clickable=True %}
-        {% endblock %}
+        {% include "includes/gov_extended_logo.html" with logo_clickable=logo_clickable %}
     <button type="button" class="usa-menu-btn">Menu</button>
     </div>
     {% block usa_nav %}
@@ -71,5 +69,5 @@
     </div>
     </nav>
     {% endblock %}
-  </header>
+</header>
   
\ No newline at end of file
diff --git a/src/registrar/templates/includes/header_selector.html b/src/registrar/templates/includes/header_selector.html
new file mode 100644
index 000000000..3cb2bd51b
--- /dev/null
+++ b/src/registrar/templates/includes/header_selector.html
@@ -0,0 +1,5 @@
+{% if not is_org_user %}
+    {% include "includes/header_basic.html" with logo_clickable=logo_clickable %}
+{% else %}
+    {% include "includes/header_extended.html" with logo_clickable=logo_clickable %}
+{% endif %}
diff --git a/src/registrar/templates/portfolio_base.html b/src/registrar/templates/portfolio_base.html
index 4cb8145f8..d4ba4dab2 100644
--- a/src/registrar/templates/portfolio_base.html
+++ b/src/registrar/templates/portfolio_base.html
@@ -36,6 +36,4 @@
 
     {% block content_bottom %}{% endblock %}
   </div>
-
-
 {% endblock wrapper %}
diff --git a/src/registrar/templates/profile.html b/src/registrar/templates/profile.html
index 41471fe88..6e1e7781f 100644
--- a/src/registrar/templates/profile.html
+++ b/src/registrar/templates/profile.html
@@ -6,8 +6,8 @@ Edit your User Profile |
 {% load static url_helpers %}
 
 {# Disable the redirect  #}
-{% block logo %}
-    {% include "includes/gov_extended_logo.html" with logo_clickable=user_finished_setup %}
+{% block header %}
+    {% include "includes/header_selector.html" with logo_clickable=user_finished_setup %}
 {% endblock %}
 
 {% block content %}
diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py
index f700f4396..59bc3e778 100644
--- a/src/registrar/views/domains_json.py
+++ b/src/registrar/views/domains_json.py
@@ -17,82 +17,15 @@ def get_domains_json(request):
     objects = Domain.objects.filter(id__in=domain_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'
-
-    # Handle search term
-    search_term = request.GET.get("search_term")
-    if search_term:
-        objects = objects.filter(Q(name__icontains=search_term))
-
-    # Handle state
-    status_param = request.GET.get("status")
-    if status_param:
-        status_list = status_param.split(",")
-
-        # if unknown is in status_list, append 'dns needed' since both
-        # unknown and dns needed display as DNS Needed, and both are
-        # searchable via state parameter of 'unknown'
-        if "unknown" in status_list:
-            status_list.append("dns needed")
-
-        # Split the status list into normal states and custom states
-        normal_states = [state for state in status_list if state in Domain.State.values]
-        custom_states = [state for state in status_list if state == "expired"]
-
-        # Construct Q objects for normal states that can be queried through ORM
-        state_query = Q()
-        if normal_states:
-            state_query |= Q(state__in=normal_states)
-
-        # Handle custom states in Python, as expired can not be queried through ORM
-        if "expired" in custom_states:
-            expired_domain_ids = [domain.id for domain in objects if domain.state_display() == "Expired"]
-            state_query |= Q(id__in=expired_domain_ids)
-
-        # Apply the combined query
-        objects = objects.filter(state_query)
-
-        # If there are filtered states, and expired is not one of them, domains with
-        # state_display of 'Expired' must be removed
-        if "expired" not in custom_states:
-            expired_domain_ids = [domain.id for domain in objects if domain.state_display() == "Expired"]
-            objects = objects.exclude(id__in=expired_domain_ids)
-
-    if sort_by == "state_display":
-        # Fetch the objects and sort them in Python
-        objects = list(objects)  # Evaluate queryset to a list
-        objects.sort(key=lambda domain: domain.state_display(), reverse=(order == "desc"))
-    else:
-        if order == "desc":
-            sort_by = f"-{sort_by}"
-        objects = objects.order_by(sort_by)
+    objects = apply_search(objects, request)
+    objects = apply_state_filter(objects, request)
+    objects = apply_sorting(objects, request)
 
     paginator = Paginator(objects, 10)
     page_number = request.GET.get("page")
     page_obj = paginator.get_page(page_number)
 
-    # Convert objects to JSON-serializable format
-    domains = [
-        {
-            "id": domain.id,
-            "name": domain.name,
-            "expiration_date": domain.expiration_date,
-            "state": domain.state,
-            "state_display": domain.state_display(),
-            "get_state_help_text": domain.get_state_help_text(),
-            "action_url": reverse("domain", kwargs={"pk": domain.id}),
-            "action_label": ("View" if domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] else "Manage"),
-            "svg_icon": ("visibility" if domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] else "settings"),
-            "suborganization": (
-                domain.domain_info.sub_organization.name
-                if domain.domain_info and domain.domain_info.sub_organization
-                else None
-            ),
-        }
-        for domain in page_obj.object_list
-    ]
+    domains = [serialize_domain(domain) for domain in page_obj.object_list]
 
     return JsonResponse(
         {
@@ -105,3 +38,79 @@ def get_domains_json(request):
             "unfiltered_total": unfiltered_total,
         }
     )
+
+
+def apply_search(queryset, request):
+    search_term = request.GET.get("search_term")
+    if search_term:
+        queryset = queryset.filter(Q(name__icontains=search_term))
+    return queryset
+
+
+def apply_state_filter(queryset, request):
+    status_param = request.GET.get("status")
+    if status_param:
+        status_list = status_param.split(",")
+        # if unknown is in status_list, append 'dns needed' since both
+        # unknown and dns needed display as DNS Needed, and both are
+        # searchable via state parameter of 'unknown'
+        if "unknown" in status_list:
+            status_list.append("dns needed")
+        # Split the status list into normal states and custom states
+        normal_states = [state for state in status_list if state in Domain.State.values]
+        custom_states = [state for state in status_list if state == "expired"]
+        # Construct Q objects for normal states that can be queried through ORM
+        state_query = Q()
+        if normal_states:
+            state_query |= Q(state__in=normal_states)
+        # Handle custom states in Python, as expired can not be queried through ORM
+        if "expired" in custom_states:
+            expired_domain_ids = [domain.id for domain in queryset if domain.state_display() == "Expired"]
+            state_query |= Q(id__in=expired_domain_ids)
+        # Apply the combined query
+        queryset = queryset.filter(state_query)
+        # If there are filtered states, and expired is not one of them, domains with
+        # state_display of 'Expired' must be removed
+        if "expired" not in custom_states:
+            expired_domain_ids = [domain.id for domain in queryset if domain.state_display() == "Expired"]
+            queryset = queryset.exclude(id__in=expired_domain_ids)
+
+    return queryset
+
+
+def apply_sorting(queryset, request):
+    sort_by = request.GET.get("sort_by", "id")
+    order = request.GET.get("order", "asc")
+    if sort_by == "state_display":
+        objects = list(queryset)
+        objects.sort(key=lambda domain: domain.state_display(), reverse=(order == "desc"))
+        return objects
+    else:
+        if order == "desc":
+            sort_by = f"-{sort_by}"
+        return queryset.order_by(sort_by)
+
+
+def serialize_domain(domain):
+    suborganization_name = None
+    try:
+        domain_info = domain.domain_info
+        if domain_info:
+            suborganization = domain_info.sub_organization
+            if suborganization:
+                suborganization_name = suborganization.name
+    except Domain.domain_info.RelatedObjectDoesNotExist:
+        domain_info = None
+
+    return {
+        "id": domain.id,
+        "name": domain.name,
+        "expiration_date": domain.expiration_date,
+        "state": domain.state,
+        "state_display": domain.state_display(),
+        "get_state_help_text": domain.get_state_help_text(),
+        "action_url": reverse("domain", kwargs={"pk": domain.id}),
+        "action_label": ("View" if domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] else "Manage"),
+        "svg_icon": ("visibility" if domain.state in [Domain.State.DELETED, Domain.State.ON_HOLD] else "settings"),
+        "suborganization": suborganization_name,
+    }