From 39b05a8fe1d0835e655f72550f91001bca35a382 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 5 Mar 2025 12:36:11 -0800 Subject: [PATCH 01/24] Add required fix for state --- src/registrar/forms/domain_request_wizard.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index d7a02b124..c76a3d856 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -348,7 +348,7 @@ class OrganizationContactForm(RegistrarForm): error_messages={ "required": ("Select the state, territory, or military post where your organization is located.") }, - widget=ComboboxWidget, + widget=ComboboxWidget(attrs={"required": True}), ) zipcode = forms.CharField( label="Zip code", From b067904086f1e1bc6b908bf69040ec38df48874a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 5 Mar 2025 13:55:33 -0700 Subject: [PATCH 02/24] Fix bug --- src/registrar/decorators.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index 7cb2792f4..829011817 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -98,6 +98,7 @@ def _user_has_permission(user, request, rules, **kwargs): if not user.is_authenticated or user.is_restricted(): return False + portfolio = request.session.get("portfolio") # Define permission checks permission_checks = [ (IS_STAFF, lambda: user.is_staff), @@ -111,7 +112,7 @@ def _user_has_permission(user, request, rules, **kwargs): ( HAS_PORTFOLIO_DOMAINS_ANY_PERM, lambda: user.is_org_user(request) - and user.has_any_domains_portfolio_permission(request.session.get("portfolio")), + and user.has_any_domains_portfolio_permission(portfolio), ), ( IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER, @@ -129,34 +130,35 @@ def _user_has_permission(user, request, rules, **kwargs): ( HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM, lambda: user.is_org_user(request) - and user.has_any_requests_portfolio_permission(request.session.get("portfolio")), + and user.has_any_requests_portfolio_permission(portfolio) ), ( HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL, lambda: user.is_org_user(request) - and user.has_view_all_domain_requests_portfolio_permission(request.session.get("portfolio")), + and user.has_view_all_domain_requests_portfolio_permission(portfolio) + and _domain_request_exists_under_portfolio(portfolio, kwargs.get("domain_request_pk")), ), ( HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, - lambda: _has_portfolio_domain_requests_edit(user, request, kwargs.get("domain_request_pk")), + lambda: _has_portfolio_domain_requests_edit(user, request, kwargs.get("domain_request_pk")) ), ( HAS_PORTFOLIO_MEMBERS_ANY_PERM, lambda: user.is_org_user(request) and ( - user.has_view_members_portfolio_permission(request.session.get("portfolio")) - or user.has_edit_members_portfolio_permission(request.session.get("portfolio")) + user.has_view_members_portfolio_permission(portfolio) + or user.has_edit_members_portfolio_permission(portfolio) ), ), ( HAS_PORTFOLIO_MEMBERS_EDIT, lambda: user.is_org_user(request) - and user.has_edit_members_portfolio_permission(request.session.get("portfolio")), + and user.has_edit_members_portfolio_permission(portfolio), ), ( HAS_PORTFOLIO_MEMBERS_VIEW, lambda: user.is_org_user(request) - and user.has_view_members_portfolio_permission(request.session.get("portfolio")), + and user.has_view_members_portfolio_permission(portfolio), ), ] @@ -198,6 +200,10 @@ def _is_domain_request_creator(user, domain_request_pk): return DomainRequest.objects.filter(creator=user, id=domain_request_pk).exists() return True +def _domain_request_exists_under_portfolio(portfolio, domain_request_pk): + """Checks to see if the given domain request exists under the provided portfolio""" + return DomainRequest.objects.filter(portfolio=portfolio, id=domain_request_pk).exists() + def _is_portfolio_member(request): """Checks to see if the user in the request is a member of the From 8b76cde13d1b435f697ad1e05811edb953becbc2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 5 Mar 2025 14:33:07 -0700 Subject: [PATCH 03/24] Start correcting additional perms --- src/registrar/decorators.py | 33 ++++++++++++++++++++++++------- src/registrar/views/portfolios.py | 23 ++++++++++----------- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index 829011817..aa16e91f5 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -2,6 +2,8 @@ import functools from django.core.exceptions import PermissionDenied from django.utils.decorators import method_decorator from registrar.models import Domain, DomainInformation, DomainInvitation, DomainRequest, UserDomainRole +from registrar.models.portfolio_invitation import PortfolioInvitation +from registrar.models.user_portfolio_permission import UserPortfolioPermission # Constants for clarity ALL = "all" @@ -116,7 +118,9 @@ def _user_has_permission(user, request, rules, **kwargs): ), ( IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER, - lambda: _is_domain_manager(user, **kwargs) and _is_portfolio_member(request), + lambda: _is_domain_manager(user, **kwargs) + and _is_portfolio_member(request) + and _domain_exists_under_portfolio(portfolio, kwargs.get("domain_pk")), ), ( IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER, @@ -141,6 +145,7 @@ def _user_has_permission(user, request, rules, **kwargs): ( HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, lambda: _has_portfolio_domain_requests_edit(user, request, kwargs.get("domain_request_pk")) + and _domain_request_exists_under_portfolio(portfolio, kwargs.get("domain_request_pk")), ), ( HAS_PORTFOLIO_MEMBERS_ANY_PERM, @@ -153,12 +158,17 @@ def _user_has_permission(user, request, rules, **kwargs): ( HAS_PORTFOLIO_MEMBERS_EDIT, lambda: user.is_org_user(request) - and user.has_edit_members_portfolio_permission(portfolio), + and user.has_edit_members_portfolio_permission(portfolio) ), ( HAS_PORTFOLIO_MEMBERS_VIEW, lambda: user.is_org_user(request) - and user.has_view_members_portfolio_permission(portfolio), + and user.has_view_members_portfolio_permission(portfolio) + # TODO -- fix this on all related URLS :( + and ( + _member_exists_under_portfolio(portfolio, kwargs.get("member_pk")) + or _member_invitation_exists_under_portfolio(portfolio, kwargs.get("memberinvitation_pk")) + ), ), ] @@ -192,6 +202,19 @@ def _is_domain_manager(user, **kwargs): return DomainInvitation.objects.filter(id=domain_invitation_id, domain__permissions__user=user).exists() return False +def _domain_exists_under_portfolio(portfolio, domain_pk): + """Checks to see if the given domain exists under the provided portfolio""" + return Domain.objects.filter(domain_info__portfolio=portfolio, id=domain_pk).exists() + +def _domain_request_exists_under_portfolio(portfolio, domain_request_pk): + """Checks to see if the given domain request exists under the provided portfolio""" + return DomainRequest.objects.filter(portfolio=portfolio, id=domain_request_pk).exists() + +def _member_exists_under_portfolio(portfolio, member_pk): + return UserPortfolioPermission.objects.filter(portfolio=portfolio, id=member_pk).exists() + +def _member_invitation_exists_under_portfolio(portfolio, memberinvitation_pk): + return PortfolioInvitation.objects.filter(portfolio=portfolio, id=memberinvitation_pk).exists() def _is_domain_request_creator(user, domain_request_pk): """Checks to see if the user is the creator of a domain request @@ -200,10 +223,6 @@ def _is_domain_request_creator(user, domain_request_pk): return DomainRequest.objects.filter(creator=user, id=domain_request_pk).exists() return True -def _domain_request_exists_under_portfolio(portfolio, domain_request_pk): - """Checks to see if the given domain request exists under the provided portfolio""" - return DomainRequest.objects.filter(portfolio=portfolio, id=domain_request_pk).exists() - def _is_portfolio_member(request): """Checks to see if the user in the request is a member of the diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 1882cc11b..420ef5acf 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -12,6 +12,7 @@ from registrar.decorators import ( HAS_PORTFOLIO_DOMAINS_ANY_PERM, HAS_PORTFOLIO_MEMBERS_ANY_PERM, HAS_PORTFOLIO_MEMBERS_EDIT, + HAS_PORTFOLIO_MEMBERS_VIEW, IS_PORTFOLIO_MEMBER, grant_access, ) @@ -71,7 +72,7 @@ class PortfolioDomainRequestsView(View): return render(request, "portfolio_requests.html") -@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW) class PortfolioMemberView(DetailView, View): model = Portfolio context_object_name = "portfolio" @@ -115,7 +116,7 @@ class PortfolioMemberView(DetailView, View): ) -@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioMemberDeleteView(View): def post(self, request, pk): @@ -217,7 +218,7 @@ class PortfolioMemberDeleteView(View): messages.warning(self.request, "Could not send email notification to existing organization admins.") -@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioMemberEditView(DetailView, View): model = Portfolio context_object_name = "portfolio" @@ -300,7 +301,7 @@ class PortfolioMemberEditView(DetailView, View): messages.warning(self.request, "Could not send email notification to existing organization admins.") -@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW) class PortfolioMemberDomainsView(View): template_name = "portfolio_member_domains.html" @@ -319,7 +320,7 @@ class PortfolioMemberDomainsView(View): ) -@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioMemberDomainsEditView(DetailView, View): model = Portfolio context_object_name = "portfolio" @@ -431,7 +432,7 @@ class PortfolioMemberDomainsEditView(DetailView, View): UserDomainRole.objects.filter(domain_id__in=removed_domain_ids, user=member).delete() -@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW) class PortfolioInvitedMemberView(DetailView, View): model = Portfolio context_object_name = "portfolio" @@ -475,7 +476,7 @@ class PortfolioInvitedMemberView(DetailView, View): ) -@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioInvitedMemberDeleteView(View): def post(self, request, pk): @@ -521,7 +522,7 @@ class PortfolioInvitedMemberDeleteView(View): messages.warning(self.request, "Could not send email notification to existing organization admins.") -@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioInvitedMemberEditView(DetailView, View): model = Portfolio context_object_name = "portfolio" @@ -592,7 +593,7 @@ class PortfolioInvitedMemberEditView(DetailView, View): messages.warning(self.request, "Could not send email notification to existing organization admins.") -@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW) class PortfolioInvitedMemberDomainsView(View): template_name = "portfolio_member_domains.html" @@ -609,7 +610,7 @@ class PortfolioInvitedMemberDomainsView(View): ) -@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioInvitedMemberDomainsEditView(DetailView, View): model = Portfolio @@ -903,7 +904,7 @@ class PortfolioMembersView(View): return render(request, "portfolio_members.html") -@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioAddMemberView(DetailView, FormMixin): template_name = "portfolio_members_add_new.html" From 949424854eecadbdf0e70d77fdbfcf7d85e26990 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 5 Mar 2025 14:56:28 -0700 Subject: [PATCH 04/24] refactor ids part 1 --- src/registrar/config/urls.py | 20 +++---- src/registrar/views/portfolios.py | 98 +++++++++++++++---------------- 2 files changed, 59 insertions(+), 59 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 12efe5a9f..56f0cfd0f 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -89,52 +89,52 @@ urlpatterns = [ name="members", ), path( - "member/", + "member/", views.PortfolioMemberView.as_view(), name="member", ), path( - "member//delete", + "member//delete", views.PortfolioMemberDeleteView.as_view(), name="member-delete", ), path( - "member//permissions", + "member//permissions", views.PortfolioMemberEditView.as_view(), name="member-permissions", ), path( - "member//domains", + "member//domains", views.PortfolioMemberDomainsView.as_view(), name="member-domains", ), path( - "member//domains/edit", + "member//domains/edit", views.PortfolioMemberDomainsEditView.as_view(), name="member-domains-edit", ), path( - "invitedmember/", + "invitedmember/", views.PortfolioInvitedMemberView.as_view(), name="invitedmember", ), path( - "invitedmember//delete", + "invitedmember//delete", views.PortfolioInvitedMemberDeleteView.as_view(), name="invitedmember-delete", ), path( - "invitedmember//permissions", + "invitedmember//permissions", views.PortfolioInvitedMemberEditView.as_view(), name="invitedmember-permissions", ), path( - "invitedmember//domains", + "invitedmember//domains", views.PortfolioInvitedMemberDomainsView.as_view(), name="invitedmember-domains", ), path( - "invitedmember//domains/edit", + "invitedmember//domains/edit", views.PortfolioInvitedMemberDomainsEditView.as_view(), name="invitedmember-domains-edit", ), diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 420ef5acf..d2a54ea51 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -78,8 +78,8 @@ class PortfolioMemberView(DetailView, View): context_object_name = "portfolio" template_name = "portfolio_member.html" - def get(self, request, pk): - portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + def get(self, request, member_pk): + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=member_pk) member = portfolio_permission.user # We have to explicitely name these with member_ otherwise we'll have conflicts with context preprocessors @@ -103,8 +103,8 @@ class PortfolioMemberView(DetailView, View): request, self.template_name, { - "edit_url": reverse("member-permissions", args=[pk]), - "domains_url": reverse("member-domains", args=[pk]), + "edit_url": reverse("member-permissions", args=[member_pk]), + "domains_url": reverse("member-domains", args=[member_pk]), "portfolio_permission": portfolio_permission, "member": member, "member_has_view_all_requests_portfolio_permission": member_has_view_all_requests_portfolio_permission, @@ -119,19 +119,19 @@ class PortfolioMemberView(DetailView, View): @grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioMemberDeleteView(View): - def post(self, request, pk): + def post(self, request, member_pk): """ Find and delete the portfolio member using the provided primary key (pk). Redirect to a success page after deletion (or any other appropriate page). """ - portfolio_member_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + portfolio_member_permission = get_object_or_404(UserPortfolioPermission, pk=member_pk) member = portfolio_member_permission.user portfolio = portfolio_member_permission.portfolio # Validate if the member can be removed error_message = self._validate_member_removal(request, member, portfolio) if error_message: - return self._handle_error_response(request, error_message, pk) + return self._handle_error_response(request, error_message, member_pk) # Attempt to send notification emails self._send_removal_notifications(request, portfolio_member_permission) @@ -162,14 +162,14 @@ class PortfolioMemberDeleteView(View): ) return None - def _handle_error_response(self, request, error_message, pk): + def _handle_error_response(self, request, error_message, member_pk): """ Return an error response (JSON or redirect with messages). """ if request.headers.get("X-Requested-With") == "XMLHttpRequest": return JsonResponse({"error": error_message}, status=400) messages.error(request, error_message) - return redirect(reverse("member", kwargs={"pk": pk})) + return redirect(reverse("member", kwargs={"pk": member_pk})) def _send_removal_notifications(self, request, portfolio_member_permission): """ @@ -225,8 +225,8 @@ class PortfolioMemberEditView(DetailView, View): template_name = "portfolio_member_permissions.html" form_class = portfolioForms.PortfolioMemberForm - def get(self, request, pk): - portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + def get(self, request, member_pk): + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=member_pk) user = portfolio_permission.user form = self.form_class(instance=portfolio_permission) @@ -241,8 +241,8 @@ class PortfolioMemberEditView(DetailView, View): }, ) - def post(self, request, pk): - portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + def post(self, request, member_pk): + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=member_pk) user = portfolio_permission.user form = self.form_class(request.POST, instance=portfolio_permission) removing_admin_role_on_self = False @@ -277,7 +277,7 @@ class PortfolioMemberEditView(DetailView, View): self._handle_exceptions(e) form.save() messages.success(self.request, "The member access and permission changes have been saved.") - return redirect("member", pk=pk) if not removing_admin_role_on_self else redirect("home") + return redirect("member", pk=member_pk) if not removing_admin_role_on_self else redirect("home") return render( request, @@ -306,8 +306,8 @@ class PortfolioMemberDomainsView(View): template_name = "portfolio_member_domains.html" - def get(self, request, pk): - portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + def get(self, request, member_pk): + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=member_pk) member = portfolio_permission.user return render( @@ -326,8 +326,8 @@ class PortfolioMemberDomainsEditView(DetailView, View): context_object_name = "portfolio" template_name = "portfolio_member_domains_edit.html" - def get(self, request, pk): - portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + def get(self, request, member_pk): + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=member_pk) member = portfolio_permission.user return render( @@ -339,33 +339,33 @@ class PortfolioMemberDomainsEditView(DetailView, View): }, ) - def post(self, request, pk): + def post(self, request, member_pk): """ Handles adding and removing domains for a portfolio member. """ added_domains = request.POST.get("added_domains") removed_domains = request.POST.get("removed_domains") - portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=member_pk) member = portfolio_permission.user portfolio = portfolio_permission.portfolio added_domain_ids = self._parse_domain_ids(added_domains, "added domains") if added_domain_ids is None: - return redirect(reverse("member-domains", kwargs={"pk": pk})) + return redirect(reverse("member-domains", kwargs={"pk": member_pk})) removed_domain_ids = self._parse_domain_ids(removed_domains, "removed domains") if removed_domain_ids is None: - return redirect(reverse("member-domains", kwargs={"pk": pk})) + return redirect(reverse("member-domains", kwargs={"pk": member_pk})) if not (added_domain_ids or removed_domain_ids): messages.success(request, "The domain assignment changes have been saved.") - return redirect(reverse("member-domains", kwargs={"pk": pk})) + return redirect(reverse("member-domains", kwargs={"pk": member_pk})) try: self._process_added_domains(added_domain_ids, member, request.user, portfolio) self._process_removed_domains(removed_domain_ids, member) messages.success(request, "The domain assignment changes have been saved.") - return redirect(reverse("member-domains", kwargs={"pk": pk})) + return redirect(reverse("member-domains", kwargs={"pk": member_pk})) except IntegrityError: messages.error( request, @@ -373,7 +373,7 @@ class PortfolioMemberDomainsEditView(DetailView, View): f"please contact {DefaultUserValues.HELP_EMAIL}.", ) logger.error("A database error occurred while saving changes.", exc_info=True) - return redirect(reverse("member-domains-edit", kwargs={"pk": pk})) + return redirect(reverse("member-domains-edit", kwargs={"pk": member_pk})) except Exception as e: messages.error( request, @@ -381,7 +381,7 @@ class PortfolioMemberDomainsEditView(DetailView, View): f"please contact {DefaultUserValues.HELP_EMAIL}.", ) logger.error(f"An unexpected error occurred: {str(e)}", exc_info=True) - return redirect(reverse("member-domains-edit", kwargs={"pk": pk})) + return redirect(reverse("member-domains-edit", kwargs={"pk": member_pk})) def _parse_domain_ids(self, domain_data, domain_type): """ @@ -439,8 +439,8 @@ class PortfolioInvitedMemberView(DetailView, View): template_name = "portfolio_member.html" # form_class = PortfolioInvitedMemberForm - def get(self, request, pk): - portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + def get(self, request, invitedmember_pk): + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=invitedmember_pk) # form = self.form_class(instance=portfolio_invitation) # We have to explicitely name these with member_ otherwise we'll have conflicts with context preprocessors @@ -464,8 +464,8 @@ class PortfolioInvitedMemberView(DetailView, View): request, self.template_name, { - "edit_url": reverse("invitedmember-permissions", args=[pk]), - "domains_url": reverse("invitedmember-domains", args=[pk]), + "edit_url": reverse("invitedmember-permissions", args=[invitedmember_pk]), + "domains_url": reverse("invitedmember-domains", args=[invitedmember_pk]), "portfolio_invitation": portfolio_invitation, "member_has_view_all_requests_portfolio_permission": member_has_view_all_requests_portfolio_permission, "member_has_edit_request_portfolio_permission": member_has_edit_request_portfolio_permission, @@ -479,12 +479,12 @@ class PortfolioInvitedMemberView(DetailView, View): @grant_access(HAS_PORTFOLIO_MEMBERS_VIEW, HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioInvitedMemberDeleteView(View): - def post(self, request, pk): + def post(self, request, invitedmember_pk): """ Find and delete the portfolio invited member using the provided primary key (pk). Redirect to a success page after deletion (or any other appropriate page). """ - portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=invitedmember_pk) try: # if invitation being removed is an admin @@ -529,8 +529,8 @@ class PortfolioInvitedMemberEditView(DetailView, View): template_name = "portfolio_member_permissions.html" form_class = portfolioForms.PortfolioInvitedMemberForm - def get(self, request, pk): - portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + def get(self, request, invitedmember_pk): + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=invitedmember_pk) form = self.form_class(instance=portfolio_invitation) return render( @@ -542,8 +542,8 @@ class PortfolioInvitedMemberEditView(DetailView, View): }, ) - def post(self, request, pk): - portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + def post(self, request, invitedmember_pk): + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=invitedmember_pk) form = self.form_class(request.POST, instance=portfolio_invitation) if form.is_valid(): try: @@ -569,7 +569,7 @@ class PortfolioInvitedMemberEditView(DetailView, View): self._handle_exceptions(e) form.save() messages.success(self.request, "The member access and permission changes have been saved.") - return redirect("invitedmember", pk=pk) + return redirect("invitedmember", pk=invitedmember_pk) return render( request, @@ -598,8 +598,8 @@ class PortfolioInvitedMemberDomainsView(View): template_name = "portfolio_member_domains.html" - def get(self, request, pk): - portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + def get(self, request, invitedmember_pk): + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=invitedmember_pk) return render( request, @@ -617,8 +617,8 @@ class PortfolioInvitedMemberDomainsEditView(DetailView, View): context_object_name = "portfolio" template_name = "portfolio_member_domains_edit.html" - def get(self, request, pk): - portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + def get(self, request, invitedmember_pk): + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=invitedmember_pk) return render( request, @@ -628,33 +628,33 @@ class PortfolioInvitedMemberDomainsEditView(DetailView, View): }, ) - def post(self, request, pk): + def post(self, request, invitedmember_pk): """ Handles adding and removing domains for a portfolio invitee. """ added_domains = request.POST.get("added_domains") removed_domains = request.POST.get("removed_domains") - portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=pk) + portfolio_invitation = get_object_or_404(PortfolioInvitation, pk=invitedmember_pk) email = portfolio_invitation.email portfolio = portfolio_invitation.portfolio added_domain_ids = self._parse_domain_ids(added_domains, "added domains") if added_domain_ids is None: - return redirect(reverse("invitedmember-domains", kwargs={"pk": pk})) + return redirect(reverse("invitedmember-domains", kwargs={"pk": invitedmember_pk})) removed_domain_ids = self._parse_domain_ids(removed_domains, "removed domains") if removed_domain_ids is None: - return redirect(reverse("invitedmember-domains", kwargs={"pk": pk})) + return redirect(reverse("invitedmember-domains", kwargs={"pk": invitedmember_pk})) if not (added_domain_ids or removed_domain_ids): messages.success(request, "The domain assignment changes have been saved.") - return redirect(reverse("invitedmember-domains", kwargs={"pk": pk})) + return redirect(reverse("invitedmember-domains", kwargs={"pk": invitedmember_pk})) try: self._process_added_domains(added_domain_ids, email, request.user, portfolio) self._process_removed_domains(removed_domain_ids, email) messages.success(request, "The domain assignment changes have been saved.") - return redirect(reverse("invitedmember-domains", kwargs={"pk": pk})) + return redirect(reverse("invitedmember-domains", kwargs={"pk": invitedmember_pk})) except IntegrityError: messages.error( request, @@ -662,7 +662,7 @@ class PortfolioInvitedMemberDomainsEditView(DetailView, View): f"please contact {DefaultUserValues.HELP_EMAIL}.", ) logger.error("A database error occurred while saving changes.", exc_info=True) - return redirect(reverse("invitedmember-domains-edit", kwargs={"pk": pk})) + return redirect(reverse("invitedmember-domains-edit", kwargs={"pk": invitedmember_pk})) except Exception as e: messages.error( request, @@ -670,7 +670,7 @@ class PortfolioInvitedMemberDomainsEditView(DetailView, View): f"please contact {DefaultUserValues.HELP_EMAIL}.", ) logger.error(f"An unexpected error occurred: {str(e)}.", exc_info=True) - return redirect(reverse("invitedmember-domains-edit", kwargs={"pk": pk})) + return redirect(reverse("invitedmember-domains-edit", kwargs={"pk": invitedmember_pk})) def _parse_domain_ids(self, domain_data, domain_type): """ From 5107fcb25211cfd622068f9ba4fd8f81830e66de Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 5 Mar 2025 15:22:42 -0700 Subject: [PATCH 05/24] refactor ids part 2 --- src/registrar/decorators.py | 6 +++--- .../includes/member_domains_edit_table.html | 4 ++-- .../templates/portfolio_member_domains.html | 8 ++++---- .../templates/portfolio_member_domains_edit.html | 8 ++++---- .../templates/portfolio_member_permissions.html | 4 ++-- src/registrar/views/portfolio_members_json.py | 5 ++++- src/registrar/views/portfolios.py | 15 ++++++++++++--- 7 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index aa16e91f5..714e375c4 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -167,7 +167,7 @@ def _user_has_permission(user, request, rules, **kwargs): # TODO -- fix this on all related URLS :( and ( _member_exists_under_portfolio(portfolio, kwargs.get("member_pk")) - or _member_invitation_exists_under_portfolio(portfolio, kwargs.get("memberinvitation_pk")) + or _member_invitation_exists_under_portfolio(portfolio, kwargs.get("invitedmember_pk")) ), ), ] @@ -213,8 +213,8 @@ def _domain_request_exists_under_portfolio(portfolio, domain_request_pk): def _member_exists_under_portfolio(portfolio, member_pk): return UserPortfolioPermission.objects.filter(portfolio=portfolio, id=member_pk).exists() -def _member_invitation_exists_under_portfolio(portfolio, memberinvitation_pk): - return PortfolioInvitation.objects.filter(portfolio=portfolio, id=memberinvitation_pk).exists() +def _member_invitation_exists_under_portfolio(portfolio, invitedmember_pk): + return PortfolioInvitation.objects.filter(portfolio=portfolio, id=invitedmember_pk).exists() def _is_domain_request_creator(user, domain_request_pk): """Checks to see if the user is the creator of a domain request diff --git a/src/registrar/templates/includes/member_domains_edit_table.html b/src/registrar/templates/includes/member_domains_edit_table.html index a76f711cc..1d706f89a 100644 --- a/src/registrar/templates/includes/member_domains_edit_table.html +++ b/src/registrar/templates/includes/member_domains_edit_table.html @@ -91,9 +91,9 @@ aria-describedby="You have unsaved changes that will be lost." > {% if portfolio_permission %} - {% url 'member-domains' pk=portfolio_permission.id as url %} + {% url 'member-domains' member_pk=portfolio_permission.id as url %} {% else %} - {% url 'invitedmember-domains' pk=portfolio_invitation.id as url %} + {% url 'invitedmember-domains' invitedmember_pk=portfolio_invitation.id as url %} {% endif %} {% include 'includes/modal.html' with modal_heading="Are you sure you want to continue?" modal_description="You have unsaved changes that will be lost." modal_button_url=url modal_button_text="Continue without saving" %} diff --git a/src/registrar/templates/portfolio_member_domains.html b/src/registrar/templates/portfolio_member_domains.html index 76d48c20b..677539585 100644 --- a/src/registrar/templates/portfolio_member_domains.html +++ b/src/registrar/templates/portfolio_member_domains.html @@ -15,11 +15,11 @@ {% url 'members' as url %} {% if portfolio_permission %} - {% url 'member' pk=portfolio_permission.id as url2 %} - {% url 'member-domains-edit' pk=portfolio_permission.id as url3 %} + {% url 'member' member_pk=portfolio_permission.id as url2 %} + {% url 'member-domains-edit' member_pk=portfolio_permission.id as url3 %} {% else %} - {% url 'invitedmember' pk=portfolio_invitation.id as url2 %} - {% url 'invitedmember-domains-edit' pk=portfolio_invitation.id as url3 %} + {% url 'invitedmember' invitedmember_pk=portfolio_invitation.id as url2 %} + {% url 'invitedmember-domains-edit' invitedmember_pk=portfolio_invitation.id as url3 %} {% endif %}