updates in response to PR suggestions

This commit is contained in:
David Kennedy 2025-01-15 06:34:46 -05:00
parent 2d1eaf8b18
commit b5970ecb37
No known key found for this signature in database
GPG key ID: 6528A5386E66B96B
6 changed files with 35 additions and 35 deletions

View file

@ -1520,7 +1520,7 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
""" """
if not change: if not change:
domain = obj.domain domain = obj.domain
domain_org = domain.domain_info.portfolio domain_org = getattr(domain.domain_info, "portfolio", None)
requested_email = obj.email requested_email = obj.email
# Look up a user with that email # Look up a user with that email
requested_user = get_requested_user(requested_email) requested_user = get_requested_user(requested_email)
@ -1536,6 +1536,7 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
and not flag_is_active(request, "multiple_portfolios") and not flag_is_active(request, "multiple_portfolios")
and domain_org is not None and domain_org is not None
and not member_of_this_org and not member_of_this_org
and not member_of_a_different_org
): ):
send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=domain_org) send_portfolio_invitation_email(email=requested_email, requestor=requestor, portfolio=domain_org)
portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create( portfolio_invitation, _ = PortfolioInvitation.objects.get_or_create(

View file

@ -1,15 +1,9 @@
{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #}
{% if requested_user and requested_user.first_name %} Hi,{% if requested_user and requested_user.first_name %} {{ requested_user.first_name }}.{% endif %}
Hi, {{ requested_user.first_name }}.
{% else %}
Hi,
{% endif %}
{{ requestor_email }} has invited you to manage: {{ requestor_email }} has invited you to manage:
{% for domain in domains %} {% for domain in domains %}{{ domain.name }}
{{ domain.name }}
{% endfor %} {% endfor %}
To manage domain information, visit the .gov registrar <https://manage.get.gov>. To manage domain information, visit the .gov registrar <https://manage.get.gov>.
---------------------------------------------------------------- ----------------------------------------------------------------

View file

@ -334,7 +334,10 @@ class TestDomainInvitationAdmin(TestCase):
Should not send a out portfolio invitation. Should not send a out portfolio invitation.
Should trigger success message for the domain invitation. Should trigger success message for the domain invitation.
Should retrieve the domain invitation. Should retrieve the domain invitation.
Should not create a portfolio invitation.""" Should not create a portfolio invitation.
NOTE: This test may need to be reworked when the multiple_portfolio flag is fully fleshed out.
"""
user = User.objects.create_user(email="test@example.com", username="username") user = User.objects.create_user(email="test@example.com", username="username")

View file

@ -48,7 +48,10 @@ def normalize_domains(domains):
def get_requestor_email(requestor, domains): def get_requestor_email(requestor, domains):
"""Get the requestor's email or raise an error if it's missing.""" """Get the requestor's email or raise an error if it's missing.
If the requestor is staff, default email is returned.
"""
if requestor.is_staff: if requestor.is_staff:
return settings.DEFAULT_FROM_EMAIL return settings.DEFAULT_FROM_EMAIL

View file

@ -262,7 +262,7 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V
"A database error occurred while saving changes. If the issue persists, " "A database error occurred while saving changes. If the issue persists, "
f"please contact {DefaultUserValues.HELP_EMAIL}.", f"please contact {DefaultUserValues.HELP_EMAIL}.",
) )
logger.error("A database error occurred while saving changes.") 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": pk}))
except Exception as e: except Exception as e:
messages.error( messages.error(
@ -270,7 +270,7 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V
f"An unexpected error occurred: {str(e)}. If the issue persists, " f"An unexpected error occurred: {str(e)}. If the issue persists, "
f"please contact {DefaultUserValues.HELP_EMAIL}.", f"please contact {DefaultUserValues.HELP_EMAIL}.",
) )
logger.error(f"An unexpected error occurred: {str(e)}") 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": pk}))
else: else:
messages.info(request, "No changes detected.") messages.info(request, "No changes detected.")
@ -479,7 +479,7 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission
"A database error occurred while saving changes. If the issue persists, " "A database error occurred while saving changes. If the issue persists, "
f"please contact {DefaultUserValues.HELP_EMAIL}.", f"please contact {DefaultUserValues.HELP_EMAIL}.",
) )
logger.error("A database error occurred while saving changes.") 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": pk}))
except Exception as e: except Exception as e:
messages.error( messages.error(
@ -487,7 +487,7 @@ class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermission
f"An unexpected error occurred: {str(e)}. If the issue persists, " f"An unexpected error occurred: {str(e)}. If the issue persists, "
f"please contact {DefaultUserValues.HELP_EMAIL}.", f"please contact {DefaultUserValues.HELP_EMAIL}.",
) )
logger.error(f"An unexpected error occurred: {str(e)}.") 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": pk}))
else: else:
messages.info(request, "No changes detected.") messages.info(request, "No changes detected.")

View file

@ -1,8 +1,6 @@
from django.contrib import messages from django.contrib import messages
from django.db import IntegrityError from django.db import IntegrityError
from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models import PortfolioInvitation, User, UserPortfolioPermission
from registrar.models.user import User
from registrar.models.user_portfolio_permission import UserPortfolioPermission
from registrar.utility.email import EmailSendingError from registrar.utility.email import EmailSendingError
import logging import logging
@ -20,32 +18,33 @@ logger = logging.getLogger(__name__)
# any view, and were initially developed for domain.py, portfolios.py and admin.py # any view, and were initially developed for domain.py, portfolios.py and admin.py
def get_org_membership(requestor_org, requested_email, requested_user): def get_org_membership(org, email, user):
""" """
Verifies if an email belongs to a different organization as a member or invited member. Determines if an email/user belongs to a different organization or this organization
Verifies if an email belongs to this organization as a member or invited member. as either a member or an invited member.
User does not belong to any org can be deduced from the tuple returned.
Returns a tuple (member_of_a_different_org, member_of_this_org). This function returns a tuple (member_of_a_different_org, member_of_this_org),
which provides:
- member_of_a_different_org: True if the user/email is associated with an organization other than the given org.
- member_of_this_org: True if the user/email is associated with the given org.
Note: This implementation assumes single portfolio ownership for a user.
If the "multiple portfolios" feature is enabled, this logic may not account for
situations where a user or email belongs to multiple organizations.
""" """
# COMMENT: this code does not take into account when multiple portfolios flag is set to true # Check for existing permissions or invitations for the user
existing_org_permission = UserPortfolioPermission.objects.filter(user=user).first()
# COMMENT: shouldn't this code be based on the organization of the domain, not the org existing_org_invitation = PortfolioInvitation.objects.filter(email=email).first()
# of the requestor? requestor could have multiple portfolios
# Check for existing permissions or invitations for the requested user
existing_org_permission = UserPortfolioPermission.objects.filter(user=requested_user).first()
existing_org_invitation = PortfolioInvitation.objects.filter(email=requested_email).first()
# Determine membership in a different organization # Determine membership in a different organization
member_of_a_different_org = (existing_org_permission and existing_org_permission.portfolio != requestor_org) or ( member_of_a_different_org = (existing_org_permission and existing_org_permission.portfolio != org) or (
existing_org_invitation and existing_org_invitation.portfolio != requestor_org existing_org_invitation and existing_org_invitation.portfolio != org
) )
# Determine membership in the same organization # Determine membership in the same organization
member_of_this_org = (existing_org_permission and existing_org_permission.portfolio == requestor_org) or ( member_of_this_org = (existing_org_permission and existing_org_permission.portfolio == org) or (
existing_org_invitation and existing_org_invitation.portfolio == requestor_org existing_org_invitation and existing_org_invitation.portfolio == org
) )
return member_of_a_different_org, member_of_this_org return member_of_a_different_org, member_of_this_org