Skip to content

Commit

Permalink
Merge pull request #3606 from cisagov/za/3528-add-member-error-handling
Browse files Browse the repository at this point in the history
#3528: Fix error handling on new member form - [ZA]
  • Loading branch information
zandercymatics authored Mar 6, 2025
2 parents e3fdb4d + b694a78 commit 04603ce
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 14 deletions.
25 changes: 25 additions & 0 deletions src/registrar/forms/portfolio.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,3 +445,28 @@ class PortfolioNewMemberForm(BasePortfolioMemberForm):
class Meta:
model = PortfolioInvitation
fields = ["portfolio", "email", "roles", "additional_permissions"]

def _post_clean(self):
"""
Override _post_clean to customize model validation errors.
This runs after form clean is complete, but before the errors are displayed.
"""
try:
super()._post_clean()
self.instance.clean()
except forms.ValidationError as e:
override_error = False
if hasattr(e, "code"):
field = "email" if "email" in self.fields else None
if e.code == "has_existing_permissions":
self.add_error(field, f"{self.instance.email} is already a member of another .gov organization.")
override_error = True
elif e.code == "has_existing_invitations":
self.add_error(
field, f"{self.instance.email} has already been invited to another .gov organization."
)
override_error = True

# Errors denoted as "__all__" are special error types reserved for the model level clean function
if override_error and "__all__" in self._errors:
del self._errors["__all__"]
13 changes: 9 additions & 4 deletions src/registrar/models/utility/portfolio_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ def validate_user_portfolio_permission(user_portfolio_permission):
if existing_permissions.exists():
raise ValidationError(
"This user is already assigned to a portfolio. "
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios."
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios.",
code="has_existing_permissions",
)

existing_invitations = PortfolioInvitation.objects.filter(email=user_portfolio_permission.user.email).exclude(
Expand All @@ -302,7 +303,8 @@ def validate_user_portfolio_permission(user_portfolio_permission):
if existing_invitations.exists():
raise ValidationError(
"This user is already assigned to a portfolio invitation. "
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios."
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios.",
code="has_existing_invitations",
)


Expand Down Expand Up @@ -350,6 +352,7 @@ def validate_portfolio_invitation(portfolio_invitation):

# == Validate the multiple_porfolios flag. == #
user = User.objects.filter(email=portfolio_invitation.email).first()

# If user returns None, then we check for global assignment of multiple_portfolios.
# Otherwise we just check on the user.
if not flag_is_active_for_user(user, "multiple_portfolios"):
Expand All @@ -362,13 +365,15 @@ def validate_portfolio_invitation(portfolio_invitation):
if existing_permissions.exists():
raise ValidationError(
"This user is already assigned to a portfolio. "
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios."
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios.",
code="has_existing_permissions",
)

if existing_invitations.exists():
raise ValidationError(
"This user is already assigned to a portfolio invitation. "
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios."
"Based on current waffle flag settings, users cannot be assigned to multiple portfolios.",
code="has_existing_invitations",
)


Expand Down
12 changes: 2 additions & 10 deletions src/registrar/tests/test_views_portfolio.py
Original file line number Diff line number Diff line change
Expand Up @@ -3873,11 +3873,7 @@ def test_member_invite_for_previously_invited_member(self, mock_send_email):
# verify messages
self.assertContains(
response,
(
"This user is already assigned to a portfolio invitation. "
"Based on current waffle flag settings, users cannot be assigned "
"to multiple portfolios."
),
f"{self.invited_member_email} has already been invited to another .gov organization.",
)

# Validate Database has not changed
Expand Down Expand Up @@ -3915,11 +3911,7 @@ def test_member_invite_for_existing_member(self, mock_send_email):
# Verify messages
self.assertContains(
response,
(
"This user is already assigned to a portfolio. "
"Based on current waffle flag settings, users cannot be "
"assigned to multiple portfolios."
),
f"{self.user.email} is already a member of another .gov organization.",
)

# Validate Database has not changed
Expand Down

0 comments on commit 04603ce

Please sign in to comment.