From e1e812c5c71c7e814acc2b51744d89eae8d2b47f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 30 Aug 2024 11:51:24 -0400 Subject: [PATCH] Fix portfolio transfer --- src/registrar/models/user.py | 28 +------------ .../templates/admin/transfer_user.html | 24 +++++++++++ src/registrar/tests/test_admin.py | 41 +++++++++---------- src/registrar/views/transfer_user.py | 18 ++++++-- 4 files changed, 60 insertions(+), 51 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index a7ea1e14a..9cc1492d1 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -6,7 +6,7 @@ from django.db.models import Q from django.http import HttpRequest from registrar.models import DomainInformation, UserDomainRole -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices from .domain_invitation import DomainInvitation from .portfolio_invitation import PortfolioInvitation @@ -64,32 +64,6 @@ class User(AbstractUser): # after they login. FIXTURE_USER = "fixture_user", "Created by fixtures" - PORTFOLIO_ROLE_PERMISSIONS = { - UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.EDIT_MEMBER, - UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.EDIT_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.EDIT_PORTFOLIO, - # Domain: field specific permissions - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, - ], - UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - # Domain: field specific permissions - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - ], - UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - ], - } - # #### Constants for choice fields #### RESTRICTED = "restricted" STATUS_CHOICES = ((RESTRICTED, RESTRICTED),) diff --git a/src/registrar/templates/admin/transfer_user.html b/src/registrar/templates/admin/transfer_user.html index e49565afc..9e55346e7 100644 --- a/src/registrar/templates/admin/transfer_user.html +++ b/src/registrar/templates/admin/transfer_user.html @@ -111,6 +111,18 @@ None {% endif %} +
Portfolios:
+
+ {% if selected_user_portfolios %} + + {% else %} + None + {% endif %} +
{% else %}

No user selected yet.

@@ -167,6 +179,18 @@ None {% endif %} +
Portfolios:
+
+ {% if current_user_portfolios %} + + {% else %} + None + {% endif %} +
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index d34e81271..0293816fd 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -45,7 +45,8 @@ from registrar.models import ( from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial from registrar.models.user_domain_role import UserDomainRole -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from registrar.models.verified_by_staff import VerifiedByStaff from .common import ( MockDbForSharedTests, @@ -2162,6 +2163,14 @@ class TestTransferUser(WebTest): ) domain_request.status = DomainRequest.DomainRequestStatus.APPROVED domain_request.save() + portfolio1 = Portfolio.objects.create(organization_name="Hotel California", creator=self.user2) + UserPortfolioPermission.objects.create( + user=self.user1, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + portfolio2 = Portfolio.objects.create(organization_name="Tokyo Hotel", creator=self.user2) + UserPortfolioPermission.objects.create( + user=self.user2, portfolio=portfolio2, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) user_transfer_page = self.app.get(reverse("transfer_user", args=[self.user1.pk])) @@ -2171,6 +2180,7 @@ class TestTransferUser(WebTest): self.assertContains(user_transfer_page, "Road warrior") self.assertContains(user_transfer_page, "wasteland.gov") self.assertContains(user_transfer_page, "citadel.gov") + self.assertContains(user_transfer_page, "Hotel California") select_form = user_transfer_page.forms[0] select_form["selected_user"] = str(self.user2.id) @@ -2180,19 +2190,15 @@ class TestTransferUser(WebTest): self.assertContains(preview_result, "Furiosa") self.assertContains(preview_result, "Jabassa") self.assertContains(preview_result, "Imperator") + self.assertContains(preview_result, "Tokyo Hotel") @less_console_noise_decorator - def test_transfer_user_transfers_portfolio_roles_and_permissions_and_portfolio_creator(self): - """Assert that a portfolio gets copied over - NOTE: Should be revised for #2644""" - portfolio = Portfolio.objects.create(organization_name="Citadel", creator=self.user2) - self.user2.portfolio = portfolio - self.user2.portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - self.user2.portfolio_additional_permissions = [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.EDIT_PORTFOLIO, - ] - self.user2.save() + def test_transfer_user_transfers_user_portfolio_roles(self): + """Assert that a portfolio user role gets transferred""" + portfolio = Portfolio.objects.create(organization_name="Hotel California", creator=self.user2) + user_portfolio_permission = UserPortfolioPermission.objects.create( + user=self.user2, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) user_transfer_page = self.app.get(reverse("transfer_user", args=[self.user1.pk])) @@ -2200,16 +2206,9 @@ class TestTransferUser(WebTest): submit_form["selected_user"] = self.user2.pk submit_form.submit() - self.user1.refresh_from_db() - portfolio.refresh_from_db() + user_portfolio_permission.refresh_from_db() - self.assertEquals(portfolio.creator, self.user1) - self.assertEquals(self.user1.portfolio, portfolio) - self.assertEquals(self.user1.portfolio_roles, [UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) - self.assertEquals( - self.user1.portfolio_additional_permissions, - [UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.EDIT_PORTFOLIO], - ) + self.assertEquals(user_portfolio_permission.user, self.user1) @less_console_noise_decorator def test_transfer_user_transfers_domain_request_creator_and_investigator(self): diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py index cc901d5ab..ac51cd20b 100644 --- a/src/registrar/views/transfer_user.py +++ b/src/registrar/views/transfer_user.py @@ -11,7 +11,9 @@ from django.contrib.admin import site from django.contrib import messages from registrar.models.user_domain_role import UserDomainRole +from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.verified_by_staff import VerifiedByStaff +from typing import Any, List logger = logging.getLogger(__name__) @@ -26,9 +28,12 @@ class TransferUserView(View): (DomainRequest, "investigator"), (UserDomainRole, "user"), (VerifiedByStaff, "requestor"), + (UserPortfolioPermission, "user"), ] - USER_FIELDS = ["portfolio", "portfolio_roles", "portfolio_additional_permissions"] + # Future-proofing in case joined fields get added on the user model side + # This was tested in the first portfolio model iteration and works + USER_FIELDS: List[Any] = [] def get(self, request, user_id): """current_user referes to the 'source' user where the button that redirects to this view was clicked. @@ -51,6 +56,7 @@ class TransferUserView(View): **admin_context, # Include the admin context "current_user_domains": self.get_domains(current_user), "current_user_domain_requests": self.get_domain_requests(current_user), + "current_user_portfolios": self.get_portfolios(current_user), } selected_user_id = request.GET.get("selected_user") @@ -59,6 +65,7 @@ class TransferUserView(View): context["selected_user"] = selected_user context["selected_user_domains"] = self.get_domains(selected_user) context["selected_user_domain_requests"] = self.get_domain_requests(selected_user) + context["selected_user_portfolios"] = self.get_portfolios(selected_user) return render(request, "admin/transfer_user.html", context) @@ -121,8 +128,6 @@ class TransferUserView(View): """ Transfers portfolio fields from the selected_user to the current_user. Logs the changes for each transferred field. - - NOTE: This will be refactored in #2644 """ for field in cls.USER_FIELDS: field_value = getattr(selected_user, field, None) @@ -158,3 +163,10 @@ class TransferUserView(View): domain_requests = DomainRequest.objects.filter(creator=user) return domain_requests + + @classmethod + def get_portfolios(cls, user): + """Get portfolios""" + portfolios = UserPortfolioPermission.objects.filter(user=user) + + return portfolios