From c7544dabe2f1edddfc7651ca3e96bf82fd96b430 Mon Sep 17 00:00:00 2001
From: Rachid Mrad <rachid.mrad@ecstech.com>
Date: Thu, 20 Jun 2024 12:55:42 -0400
Subject: [PATCH] Unit tests and cleanup

---
 src/registrar/registrar_middleware.py         | 15 ++--
 .../includes/domain_requests_table.html       |  8 +-
 .../templates/includes/domains_table.html     |  8 +-
 .../{organizations.html => organization.html} | 27 +------
 .../templates/organization_domains.html       |  8 ++
 .../templates/organization_requests.html      | 21 +++++
 .../templates/organization_sidebar.html       |  4 +-
 src/registrar/tests/test_views.py             | 76 ++++++++++++++++++-
 src/registrar/views/index.py                  |  2 +-
 src/registrar/views/organizations.py          | 12 +--
 10 files changed, 131 insertions(+), 50 deletions(-)
 rename src/registrar/templates/{organizations.html => organization.html} (60%)
 create mode 100644 src/registrar/templates/organization_domains.html
 create mode 100644 src/registrar/templates/organization_requests.html

diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py
index 30137ee63..79a728310 100644
--- a/src/registrar/registrar_middleware.py
+++ b/src/registrar/registrar_middleware.py
@@ -14,6 +14,7 @@ from registrar.models.utility.generic_helper import replace_url_queryparams
 
 logger = logging.getLogger(__name__)
 
+
 class NoCacheMiddleware:
     """
     Middleware to add Cache-control: no-cache to every response.
@@ -122,9 +123,12 @@ class CheckUserProfileMiddleware:
         else:
             # Process the view as normal
             return None
-        
+
+
 class CheckOrganizationMiddleware:
     """
+    Checks if the current user has a portfolio
+    If they do, redirect them to the org homepage when they navigate to home.
     """
 
     def __init__(self, get_response):
@@ -137,10 +141,8 @@ class CheckOrganizationMiddleware:
 
     def process_view(self, request, view_func, view_args, view_kwargs):
         current_path = request.path
-        logger.debug(f"Current path: {current_path}")
 
         has_organization_feature_flag = flag_is_active(request, "organization_feature")
-        logger.debug(f"Flag is active: {has_organization_feature_flag}")
 
         if current_path == self.home:
             if has_organization_feature_flag:
@@ -148,9 +150,10 @@ class CheckOrganizationMiddleware:
                     user_portfolios = Portfolio.objects.filter(creator=request.user)
                     if user_portfolios.exists():
                         first_portfolio = user_portfolios.first()
-                        home_organization_with_portfolio = reverse("organization-domains", kwargs={'portfolio_id': first_portfolio.id})
-                        
+                        home_organization_with_portfolio = reverse(
+                            "organization-domains", kwargs={"portfolio_id": first_portfolio.id}
+                        )
+
                         if current_path != home_organization_with_portfolio:
-                            logger.debug(f"User has portfolios, redirecting to {home_organization_with_portfolio}")
                             return HttpResponseRedirect(home_organization_with_portfolio)
         return None
diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html
index 377f49d02..e760687b6 100644
--- a/src/registrar/templates/includes/domain_requests_table.html
+++ b/src/registrar/templates/includes/domain_requests_table.html
@@ -2,9 +2,11 @@
 
 <section class="section--outlined domain-requests">
     <div class="grid-row">
-      <div class="mobile:grid-col-12 desktop:grid-col-6">
-        <h2 id="domain-requests-header" class="flex-6">Domain requests</h2>
-      </div>
+      {% if portfolio is None %}
+        <div class="mobile:grid-col-12 desktop:grid-col-6">
+          <h2 id="domain-requests-header" class="flex-6">Domain requests</h2>
+        </div>
+      {% endif %}
       <div class="mobile:grid-col-12 desktop:grid-col-6">
         <section aria-label="Domain requests search component" class="flex-6 margin-y-2">
           <form class="usa-search usa-search--small" method="POST" role="search">
diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html
index 334dba3da..4347520cc 100644
--- a/src/registrar/templates/includes/domains_table.html
+++ b/src/registrar/templates/includes/domains_table.html
@@ -2,9 +2,11 @@
 
 <section class="section--outlined domains">
     <div class="grid-row">
-      <div class="mobile:grid-col-12 desktop:grid-col-6">
-        <h2 id="domains-header" class="flex-6">Domains</h2>
-      </div>
+      {% if portfolio is None %}
+        <div class="mobile:grid-col-12 desktop:grid-col-6">
+          <h2 id="domains-header" class="flex-6">Domains</h2>
+        </div>
+      {% endif %}
       <div class="mobile:grid-col-12 desktop:grid-col-6">
         <section aria-label="Domains search component" class="flex-6 margin-y-2">
           <form class="usa-search usa-search--small" method="POST" role="search">
diff --git a/src/registrar/templates/organizations.html b/src/registrar/templates/organization.html
similarity index 60%
rename from src/registrar/templates/organizations.html
rename to src/registrar/templates/organization.html
index b354fb1bf..33ab7ff6c 100644
--- a/src/registrar/templates/organizations.html
+++ b/src/registrar/templates/organization.html
@@ -7,35 +7,16 @@
   <div class="tablet:grid-col-12">
     <div class="grid-row grid-gap">
         <div class="tablet:grid-col-3">
-            {% include "organization_sidebar.html" with portfolio=portfolio current_path=content %}
+            {% include "organization_sidebar.html" with portfolio=portfolio %}
         </div>
         <div class="tablet:grid-col-9">
             {% block messages %}
                 {% include "includes/form_messages.html" %}
             {% endblock %}
             {# Note: Reimplement commented out functionality #}
-            <!--
-            <h1>Manage your domains</h1>
-            -->
-
-            {% 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 %}
-            <!--
-            <p class="margin-top-4">
-            <a href="{% url 'domain-request:' %}" class="usa-button"
-            >
-                Start a new domain request
-            </a>
-            </p>
-            -->
-            {% if content == 'domains' %}
-                {% include "includes/domains_table.html" with portfolio=portfolio %}
-            {% elif content == 'domain-requests' %}
-                {% include "includes/domain_requests_table.html" with portfolio=portfolio %}
-            {% endif %}
+            
+            {% block organization_content %}
+            {% endblock %}
 
             {# Note: Reimplement this after MVP #}
             <!--
diff --git a/src/registrar/templates/organization_domains.html b/src/registrar/templates/organization_domains.html
new file mode 100644
index 000000000..9dff1fb0b
--- /dev/null
+++ b/src/registrar/templates/organization_domains.html
@@ -0,0 +1,8 @@
+{% extends 'organization.html' %}
+
+{% load static %}
+
+{% block organization_content %}
+    <h1>Domains</h1>
+    {% include "includes/domains_table.html" with portfolio=portfolio %}
+{% endblock %}
diff --git a/src/registrar/templates/organization_requests.html b/src/registrar/templates/organization_requests.html
new file mode 100644
index 000000000..29d8e589d
--- /dev/null
+++ b/src/registrar/templates/organization_requests.html
@@ -0,0 +1,21 @@
+{% extends 'organization.html' %}
+
+{% load static %}
+
+{% block organization_content %}
+    <h1>Domain requests</h1>
+
+    {% 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 %}
+    <p class="margin-top-4">
+        <a href="{% url 'domain-request:' %}" class="usa-button"
+        >
+            Start a new domain request
+        </a>
+    </p>
+       
+    {% include "includes/domain_requests_table.html" with portfolio=portfolio %}
+{% endblock %}
diff --git a/src/registrar/templates/organization_sidebar.html b/src/registrar/templates/organization_sidebar.html
index 4f2779816..d2d7cd306 100644
--- a/src/registrar/templates/organization_sidebar.html
+++ b/src/registrar/templates/organization_sidebar.html
@@ -2,10 +2,8 @@
 
 <div class="margin-bottom-4 tablet:margin-bottom-0">
   <nav aria-label="">
+    <h2 class="margin-top-0 text-semibold">{{ portfolio.organization_name }}</h2>
     <ul class="usa-sidenav">
-      <li class="usa-sidenav__item">
-        {{ portfolio.organization_name }}
-      </li>
       <li class="usa-sidenav__item">
         {% url 'organization-domains' portfolio.id as url %} 
         <a href="{{ url }}" {% if request.path == url %}class="usa-current"{% endif %}> 
diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py
index fa63da17a..2554bed01 100644
--- a/src/registrar/tests/test_views.py
+++ b/src/registrar/tests/test_views.py
@@ -8,6 +8,7 @@ from api.tests.common import less_console_noise_decorator
 from registrar.models.contact import Contact
 from registrar.models.domain import Domain
 from registrar.models.draft_domain import DraftDomain
+from registrar.models.portfolio import Portfolio
 from registrar.models.public_contact import PublicContact
 from registrar.models.user import User
 from registrar.models.user_domain_role import UserDomainRole
@@ -652,7 +653,6 @@ class FinishUserProfileForOtherUsersTests(TestWithUser, WebTest):
         super().tearDown()
         PublicContact.objects.filter(domain=self.domain).delete()
         self.role.delete()
-        self.domain.delete()
         Domain.objects.all().delete()
         Website.objects.all().delete()
         Contact.objects.all().delete()
@@ -906,3 +906,77 @@ class UserProfileTests(TestWithUser, WebTest):
             profile_page = profile_page.follow()
             self.assertEqual(profile_page.status_code, 200)
             self.assertContains(profile_page, "Your profile has been updated")
+
+
+class OrganizationsTests(TestWithUser, WebTest):
+    """A series of tests that target the organizations"""
+
+    # csrf checks do not work well with WebTest.
+    # We disable them here.
+    csrf_checks = False
+
+    def setUp(self):
+        super().setUp()
+        self.user.save()
+        self.client.force_login(self.user)
+        self.domain, _ = Domain.objects.get_or_create(name="sampledomain.gov", state=Domain.State.READY)
+        self.role, _ = UserDomainRole.objects.get_or_create(
+            user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER
+        )
+        self.portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="xyz inc")
+
+    def tearDown(self):
+        Portfolio.objects.all().delete()
+        super().tearDown()
+        PublicContact.objects.filter(domain=self.domain).delete()
+        UserDomainRole.objects.all().delete()
+        Domain.objects.all().delete()
+        Website.objects.all().delete()
+        Contact.objects.all().delete()
+
+    def _set_session_cookie(self):
+        session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+
+    @less_console_noise_decorator
+    def test_middleware_redirects_to_organization_homepage(self):
+        """Tests that a user is redirected to the org homepage when organization_feature is on and
+        a portfolio belongs to the user, test for the special h1s which only exist in that version
+        of the homepage"""
+        self.app.set_user(self.user.username)
+        with override_flag("organization_feature", active=True):
+            # This will redirect the user to the org page.
+            # Follow implicity checks if our redirect is working.
+            org_page = self.app.get(reverse("home")).follow()
+            self._set_session_cookie()
+
+            # Assert that we're on the right page
+            self.assertContains(org_page, self.portfolio.organization_name)
+
+            self.assertContains(org_page, "<h1>Domains</h1>")
+
+    @less_console_noise_decorator
+    def test_no_redirect_when_org_flag_false(self):
+        """No redirect so no follow,
+        implicitely test for the presense of the h2 by looking up its id"""
+        self.app.set_user(self.user.username)
+        org_page = self.app.get(reverse("home"))
+        self._set_session_cookie()
+
+        self.assertNotContains(org_page, self.portfolio.organization_name)
+
+        self.assertContains(org_page, 'id="domain-requests-header"')
+
+    @less_console_noise_decorator
+    def test_no_redirect_when_user_has_no_portfolios(self):
+        """No redirect so no follow,
+        implicitely test for the presense of the h2 by looking up its id"""
+        self.portfolio.delete()
+        self.app.set_user(self.user.username)
+        with override_flag("organization_feature", active=True):
+            org_page = self.app.get(reverse("home"))
+            self._set_session_cookie()
+
+            self.assertNotContains(org_page, self.portfolio.organization_name)
+
+            self.assertContains(org_page, 'id="domain-requests-header"')
diff --git a/src/registrar/views/index.py b/src/registrar/views/index.py
index 5e546e8e7..95aca5587 100644
--- a/src/registrar/views/index.py
+++ b/src/registrar/views/index.py
@@ -14,6 +14,6 @@ def index(request):
         # This controls the creation of a new domain request in the wizard
         request.session["new_request"] = True
 
-        print('homepage view')
+        print("homepage view")
 
     return render(request, "home.html", context)
diff --git a/src/registrar/views/organizations.py b/src/registrar/views/organizations.py
index 8f0578859..d7496e60e 100644
--- a/src/registrar/views/organizations.py
+++ b/src/registrar/views/organizations.py
@@ -14,14 +14,9 @@ def organization_domains(request, portfolio_id):
         # Retrieve the portfolio object based on the provided portfolio_id
         portfolio = get_object_or_404(Portfolio, id=portfolio_id)
         context["portfolio"] = portfolio
-        context["content"] = "domains"
 
-        # This controls the creation of a new domain request in the wizard
-        request.session["new_request"] = True
+    return render(request, "organization_domains.html", context)
 
-        print('organization domains view')
-
-    return render(request, "organizations.html", context)
 
 def organization_domain_requests(request, portfolio_id):
     context = {}
@@ -34,11 +29,8 @@ def organization_domain_requests(request, portfolio_id):
         # Retrieve the portfolio object based on the provided portfolio_id
         portfolio = get_object_or_404(Portfolio, id=portfolio_id)
         context["portfolio"] = portfolio
-        context["content"] = "domain-requests"
 
         # This controls the creation of a new domain request in the wizard
         request.session["new_request"] = True
 
-        print('organization domain requests view')
-
-    return render(request, "organizations.html", context)
\ No newline at end of file
+    return render(request, "organization_requests.html", context)