From 4b8f77436d5693854238ef4703e1096b446cf507 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 13 Sep 2024 13:48:47 -0600
Subject: [PATCH 1/8] Add corrected delete logic

---
 src/registrar/views/domain_request.py       |  6 ++++++
 src/registrar/views/domain_requests_json.py | 18 +++++++++++++-----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py
index 0abe6d69a..864225dcc 100644
--- a/src/registrar/views/domain_request.py
+++ b/src/registrar/views/domain_request.py
@@ -796,6 +796,12 @@ class DomainRequestDeleteView(DomainRequestPermissionDeleteView):
         if status not in valid_statuses:
             return False
 
+        # Portfolio users cannot delete their requests if they aren't permissioned to do so 
+        if self.request.user.is_org_user(self.request):
+            portfolio = self.request.session.get("portfolio")
+            if not self.request.user.has_edit_request_portfolio_permission(portfolio):
+                return False
+
         return True
 
     def get_success_url(self):
diff --git a/src/registrar/views/domain_requests_json.py b/src/registrar/views/domain_requests_json.py
index 7b86cd9ef..bc880cdaf 100644
--- a/src/registrar/views/domain_requests_json.py
+++ b/src/registrar/views/domain_requests_json.py
@@ -25,9 +25,8 @@ def get_domain_requests_json(request):
     paginator = Paginator(objects, 10)
     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
+        serialize_domain_request(request, domain_request, request.user) for domain_request in page_obj.object_list
     ]
 
     return JsonResponse(
@@ -90,13 +89,22 @@ def apply_sorting(queryset, request):
     return queryset.order_by(sort_by)
 
 
-def serialize_domain_request(domain_request, user):
-    # Determine if the request is deletable
-    is_deletable = domain_request.status in [
+def serialize_domain_request(request, domain_request, user):
+
+    deletable_statuses = [
         DomainRequest.DomainRequestStatus.STARTED,
         DomainRequest.DomainRequestStatus.WITHDRAWN,
     ]
 
+    # Determine if the request is deletable
+    if not user.is_org_user(request):
+        is_deletable = domain_request.status in deletable_statuses
+    else:
+        portfolio = request.session.get("portfolio")
+        is_deletable = (
+            domain_request.status in deletable_statuses and user.has_edit_request_portfolio_permission(portfolio)
+        ) and domain_request.creator == user
+
     # Determine action label based on user permissions and request status
     editable_statuses = [
         DomainRequest.DomainRequestStatus.STARTED,

From 939f94a4c7c8b3d565932723c64af4ee1b120604 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 13 Sep 2024 13:51:14 -0600
Subject: [PATCH 2/8] fix spacing issue

---
 src/registrar/templates/portfolio_requests.html | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/registrar/templates/portfolio_requests.html b/src/registrar/templates/portfolio_requests.html
index 868c9bd2a..70d17feae 100644
--- a/src/registrar/templates/portfolio_requests.html
+++ b/src/registrar/templates/portfolio_requests.html
@@ -12,10 +12,11 @@
 <div id="main-content">
     <h1 id="domain-requests-header">Domain requests</h1>
     <div class="grid-row grid-gap">
-        <div class="mobile:grid-col-12 tablet:grid-col-6">
-            <p class="margin-y-0">Domain requests can only be modified by the person who created the request.</p>
-        </div>
+
         {% if has_edit_request_portfolio_permission %}
+            <div class="mobile:grid-col-12 tablet:grid-col-6">
+                <p class="margin-y-0">Domain requests can only be modified by the person who created the request.</p>
+            </div>
             <div class="mobile:grid-col-12 tablet:grid-col-6">
                 {% comment %}
                 IMPORTANT:
@@ -29,6 +30,8 @@
                     </a>
                 </p>
             </div>
+        {% else %}
+            <p class="margin-y-0">Domain requests can only be modified by the person who created the request.</p>
         {% endif %}
     </div>
     

From b0b08dc74a34f78a9b99f939429c66bb3292ac91 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 13 Sep 2024 14:26:45 -0600
Subject: [PATCH 3/8] unit tests

---
 src/registrar/tests/test_views_portfolio.py | 96 +++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index aaafa3262..a960b2fd2 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -837,3 +837,99 @@ class TestPortfolio(WebTest):
             response = self.client.get(reverse("home"))
             self.assertIsNone(self.client.session.get("portfolio"))
             self.assertNotContains(response, "Hotel California")
+
+    @less_console_noise_decorator
+    @override_flag("organization_feature", active=True)
+    @override_flag("organization_requests", active=True)
+    def test_delete_domain_request_as_org_user_with_permission_and_deletable_status(self):
+        """Test that an org user with edit permission can delete their own DomainRequest with a deletable status."""
+
+        # Assign the user to a portfolio with edit permission
+        UserPortfolioPermission.objects.get_or_create(
+            user=self.user,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+            additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS]
+        )
+
+        # Create a domain request with status WITHDRAWN
+        domain_request = completed_domain_request(
+            name="test-domain.gov",
+            status=DomainRequest.DomainRequestStatus.WITHDRAWN,
+            portfolio=self.portfolio,
+        )
+
+        self.client.force_login(self.user)
+        # Perform delete
+        response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True)
+
+        # Check that the response is 200
+        self.assertEqual(response.status_code, 200)
+
+        # Check that the domain request no longer exists
+        self.assertFalse(DomainRequest.objects.filter(pk=domain_request.pk).exists())
+
+    @less_console_noise_decorator
+    @override_flag("organization_feature", active=True)
+    @override_flag("organization_requests", active=True)
+    def test_delete_domain_request_as_org_user_without_permission_with_deletable_status(self):
+        """Test that an org user without edit permission cannot delete their DomainRequest even if status is deletable."""
+
+        # Assign the user to a portfolio without edit permission
+        UserPortfolioPermission.objects.get_or_create(
+            user=self.user,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+            additional_permissions=[]
+        )
+
+        # Create a domain request with status STARTED
+        domain_request = completed_domain_request(
+            name="test-domain.gov",
+            status=DomainRequest.DomainRequestStatus.STARTED,
+            portfolio=self.portfolio,
+        )
+
+        self.client.force_login(self.user)
+        # Attempt to delete
+        response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True)
+
+        # Check response is 403 Forbidden
+        self.assertEqual(response.status_code, 403)
+
+        # Check that the domain request still exists
+        self.assertTrue(DomainRequest.objects.filter(pk=domain_request.pk).exists())
+
+    @less_console_noise_decorator
+    @override_flag("organization_feature", active=True)
+    @override_flag("organization_requests", active=True)
+    def test_delete_domain_request_as_org_user_not_creator_with_permission_and_deletable_status(self):
+        """Test that an org user with edit permission cannot delete DomainRequests they did not create."""
+
+        # Assign the user to a portfolio with edit permission
+        UserPortfolioPermission.objects.get_or_create(
+            user=self.user,
+            portfolio=self.portfolio,
+            roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
+            additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS]
+        )
+
+        # Create another user and a domain request
+        other_user = User.objects.create(username="other_user")
+        domain_request = completed_domain_request(
+            name="test-domain.gov",
+            status=DomainRequest.DomainRequestStatus.STARTED,
+            portfolio=self.portfolio,
+        )
+        domain_request.creator = other_user
+        domain_request.save()
+
+        self.client.force_login(self.user)
+        # Perform delete as self.user
+        response = self.client.post(reverse("domain-request-delete", kwargs={"pk": domain_request.pk}), follow=True)
+
+        # Check response is 403 Forbidden
+        self.assertEqual(response.status_code, 403)
+
+        # Check that the domain request still exists
+        self.assertTrue(DomainRequest.objects.filter(pk=domain_request.pk).exists())
\ No newline at end of file

From 8d58b38900e8c1454fd7095206e50897d2ebeed3 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 13 Sep 2024 14:27:56 -0600
Subject: [PATCH 4/8] Update test_views_portfolio.py

---
 src/registrar/tests/test_views_portfolio.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index a960b2fd2..6d50c9b0a 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -858,6 +858,8 @@ class TestPortfolio(WebTest):
             status=DomainRequest.DomainRequestStatus.WITHDRAWN,
             portfolio=self.portfolio,
         )
+        domain_request.creator = self.user
+        domain_request.save()
 
         self.client.force_login(self.user)
         # Perform delete
@@ -889,6 +891,8 @@ class TestPortfolio(WebTest):
             status=DomainRequest.DomainRequestStatus.STARTED,
             portfolio=self.portfolio,
         )
+        domain_request.creator = self.user
+        domain_request.save()
 
         self.client.force_login(self.user)
         # Attempt to delete

From a03769350ac4ec713db0ba332e27b80fa28eeeae Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 13 Sep 2024 14:29:28 -0600
Subject: [PATCH 5/8] lint

---
 src/registrar/tests/test_views_portfolio.py | 8 ++++----
 src/registrar/views/domain_request.py       | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index 6d50c9b0a..a952da5e7 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -849,7 +849,7 @@ class TestPortfolio(WebTest):
             user=self.user,
             portfolio=self.portfolio,
             roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
-            additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS]
+            additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS],
         )
 
         # Create a domain request with status WITHDRAWN
@@ -882,7 +882,7 @@ class TestPortfolio(WebTest):
             user=self.user,
             portfolio=self.portfolio,
             roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
-            additional_permissions=[]
+            additional_permissions=[],
         )
 
         # Create a domain request with status STARTED
@@ -915,7 +915,7 @@ class TestPortfolio(WebTest):
             user=self.user,
             portfolio=self.portfolio,
             roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER],
-            additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS]
+            additional_permissions=[UserPortfolioPermissionChoices.EDIT_REQUESTS],
         )
 
         # Create another user and a domain request
@@ -936,4 +936,4 @@ class TestPortfolio(WebTest):
         self.assertEqual(response.status_code, 403)
 
         # Check that the domain request still exists
-        self.assertTrue(DomainRequest.objects.filter(pk=domain_request.pk).exists())
\ No newline at end of file
+        self.assertTrue(DomainRequest.objects.filter(pk=domain_request.pk).exists())
diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py
index 864225dcc..5fed89215 100644
--- a/src/registrar/views/domain_request.py
+++ b/src/registrar/views/domain_request.py
@@ -796,7 +796,7 @@ class DomainRequestDeleteView(DomainRequestPermissionDeleteView):
         if status not in valid_statuses:
             return False
 
-        # Portfolio users cannot delete their requests if they aren't permissioned to do so 
+        # Portfolio users cannot delete their requests if they aren't permissioned to do so
         if self.request.user.is_org_user(self.request):
             portfolio = self.request.session.get("portfolio")
             if not self.request.user.has_edit_request_portfolio_permission(portfolio):

From e078248e02e73e45023df72147ee54227a932650 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 13 Sep 2024 14:32:57 -0600
Subject: [PATCH 6/8] Update test_views_portfolio.py

---
 src/registrar/tests/test_views_portfolio.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index a952da5e7..edac1d783 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -875,7 +875,7 @@ class TestPortfolio(WebTest):
     @override_flag("organization_feature", active=True)
     @override_flag("organization_requests", active=True)
     def test_delete_domain_request_as_org_user_without_permission_with_deletable_status(self):
-        """Test that an org user without edit permission cannot delete their DomainRequest even if status is deletable."""
+        """Test that an org user without edit permission cant delete their DomainRequest even if status is deletable."""
 
         # Assign the user to a portfolio without edit permission
         UserPortfolioPermission.objects.get_or_create(

From 897fd86e5753fa7c5f9d0174d93cf64c393a6725 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 13 Sep 2024 14:36:15 -0600
Subject: [PATCH 7/8] add delete

---
 src/registrar/tests/test_views_portfolio.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index edac1d783..e238ea0d6 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -870,6 +870,7 @@ class TestPortfolio(WebTest):
 
         # Check that the domain request no longer exists
         self.assertFalse(DomainRequest.objects.filter(pk=domain_request.pk).exists())
+        domain_request.delete()
 
     @less_console_noise_decorator
     @override_flag("organization_feature", active=True)
@@ -903,6 +904,7 @@ class TestPortfolio(WebTest):
 
         # Check that the domain request still exists
         self.assertTrue(DomainRequest.objects.filter(pk=domain_request.pk).exists())
+        domain_request.delete()
 
     @less_console_noise_decorator
     @override_flag("organization_feature", active=True)
@@ -937,3 +939,4 @@ class TestPortfolio(WebTest):
 
         # Check that the domain request still exists
         self.assertTrue(DomainRequest.objects.filter(pk=domain_request.pk).exists())
+        domain_request.delete()

From ef40468ec847403fde3e02a04d1e7d6c0f936be4 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 17 Sep 2024 08:32:51 -0600
Subject: [PATCH 8/8] Rename tests to be actually readable

---
 src/registrar/tests/test_views_portfolio.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index e238ea0d6..b8392a370 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -841,7 +841,7 @@ class TestPortfolio(WebTest):
     @less_console_noise_decorator
     @override_flag("organization_feature", active=True)
     @override_flag("organization_requests", active=True)
-    def test_delete_domain_request_as_org_user_with_permission_and_deletable_status(self):
+    def test_org_user_can_delete_own_domain_request_with_permission(self):
         """Test that an org user with edit permission can delete their own DomainRequest with a deletable status."""
 
         # Assign the user to a portfolio with edit permission
@@ -909,7 +909,7 @@ class TestPortfolio(WebTest):
     @less_console_noise_decorator
     @override_flag("organization_feature", active=True)
     @override_flag("organization_requests", active=True)
-    def test_delete_domain_request_as_org_user_not_creator_with_permission_and_deletable_status(self):
+    def test_org_user_cannot_delete_others_domain_requests(self):
         """Test that an org user with edit permission cannot delete DomainRequests they did not create."""
 
         # Assign the user to a portfolio with edit permission