Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling invite a deactivated user #35144

Merged
merged 4 commits into from
Sep 19, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion corehq/apps/registration/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from corehq.apps.hqwebapp import crispy as hqcrispy
from corehq.apps.programs.models import Program
from corehq.apps.users.forms import SelectUserLocationForm, BaseTableauUserForm
from corehq.apps.users.models import CouchUser
from corehq.apps.users.models import CouchUser, WebUser


class RegisterWebUserForm(forms.Form):
Expand Down Expand Up @@ -592,6 +592,10 @@ def clean_email(self):
if email.lower() in self.excluded_emails:
raise forms.ValidationError(_("A user with this email address is already in "
"this project or has a pending invitation."))
web_user = WebUser.get_by_username(email)
if web_user and not web_user.is_active:
raise forms.ValidationError(_("A user with this email address is deactivated. "
"Please reactivate this user first."))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking: it'd be a nice UX improvement to turn "reactivate this user" into a link to the edit page for the relevant web user. (Granted, then you have to make sure whatever code catches this error ultimately displays it as HTML.)

Copy link
Contributor Author

@jingcheng16 jingcheng16 Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Jenny. Usually we don't deactivate user, deactivation and reactivation can be operated by us in Admin>Look up user by email. So having that link might not be very helpful for most customers, because they won't have access to that tool. For SSO user who get auto-deactivated, they need first add the user back to their identity provider, then ask the user to relogin, I feel this is too long in a error message?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. If most users won't have access, perhaps we shouldn't tell them to reactivate the user? We have other messages that say to contact support (if that's the appropriate action), or maybe just removing the second sentence would be fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, removed in 409c134 ...

return email

def clean(self):
Expand Down
Loading