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

Allow WebUser invites to be editable before the invited user accepts it #35538

Merged
merged 14 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
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
28 changes: 21 additions & 7 deletions corehq/apps/registration/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,9 @@ class AdminInvitesUserForm(SelectUserLocationForm):

def __init__(self, data=None, is_add_user=None,
role_choices=(), should_show_location=False, can_edit_tableau_config=False,
custom_data=None, *, domain, **kwargs):
custom_data=None, invitation=None, *, domain, **kwargs):
self.custom_data = custom_data
self.invite = invitation
if data and self.custom_data:
data = data.copy()
custom_data_post_dict = self.custom_data.form.data
Expand All @@ -519,16 +520,24 @@ def __init__(self, data=None, is_add_user=None,
programs = Program.by_domain(domain_obj.name)
choices = [('', '')] + list((prog.get_id, prog.name) for prog in programs)
self.fields['program'].choices = choices
if self.invite:
self.fields['program'].initial = self.invite.program

if self.can_edit_tableau_config:
self._initialize_tableau_fields(data, domain)
self._initialize_tableau_fields(data, domain, self.invite)

self.helper = FormHelper()
self.helper.form_method = 'POST'
self.helper.form_class = 'form-horizontal form-ko-validation'

self.helper.label_class = 'col-sm-3 col-md-2'
self.helper.field_class = 'col-sm-9 col-md-8 col-lg-6'

save_button_text = "Send Invite"
Copy link
Contributor

Choose a reason for hiding this comment

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

this and the alternate text should be marked for translation

if self.invite:
self.fields['email'].widget.attrs["readonly"] = True
save_button_text = "Update Invite"

fields = [
crispy.Fieldset(
gettext("Information for new Web User"),
Expand Down Expand Up @@ -575,8 +584,7 @@ def __init__(self, data=None, is_add_user=None,
),
hqcrispy.FormActions(
twbscrispy.StrictButton(
(gettext("Add User") if is_add_user
else gettext("Send Invite")),
(gettext("Add User") if is_add_user else gettext(save_button_text)),
Copy link
Contributor

Choose a reason for hiding this comment

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

passing a variable to gettext will only translate that var if it's already marked for translation elsewhere, such as with gettext_noop. Probably most straightforward to just move the gettext call to where those strings are defined in this case.

type="submit",
css_class="btn-primary",
data_bind="enable: isSubmitEnabled",
Expand All @@ -594,7 +602,7 @@ def clean_email(self):
email = self.cleaned_data['email'].strip()

from corehq.apps.registration.validation import AdminInvitesUserFormValidator
error = AdminInvitesUserFormValidator.validate_email(self.domain, email)
error = AdminInvitesUserFormValidator.validate_email(self.domain, email, bool(self.invite))
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to skip all this validation if the field is set as read only? And then add a validation that the email didn't change like

    if self.fields['email'].widget.attrs.get('readonly', False):
        if email != self.invite.email:
            raise forms.ValidationError("The email field is read-only and cannot be changed.")
            ```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think what I have currently is simpler

if error:
raise forms.ValidationError(error)
return email
Expand Down Expand Up @@ -632,8 +640,14 @@ def clean(self):
raise forms.ValidationError(error)
return cleaned_data

def _initialize_tableau_fields(self, data, domain):
self.tableau_form = BaseTableauUserForm(data, domain=domain)
def _initialize_tableau_fields(self, data, domain, invitation=None):
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
initial = {}
if invitation:
initial = {
minhaminha marked this conversation as resolved.
Show resolved Hide resolved
'tableau_role': invitation.tableau_role,
'tableau_group_indices': invitation.tableau_group_ids,
}
self.tableau_form = BaseTableauUserForm(data, domain=domain, initial=initial)
self.fields['tableau_group_indices'] = self.tableau_form.fields["groups"]
self.fields['tableau_group_indices'].label = _('Tableau Groups')
self.fields['tableau_role'] = self.tableau_form.fields['role']
Expand Down
4 changes: 2 additions & 2 deletions corehq/apps/registration/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ def validate_parameters(domain, upload_user, parameters):
return _("This domain does not have locations privileges.")

@staticmethod
def validate_email(domain, email):
def validate_email(domain, email, is_invite_edit=False):
current_users = [user.username.lower() for user in WebUser.by_domain(domain)]
pending_invites = [di.email.lower() for di in Invitation.by_domain(domain)]
current_users_and_pending_invites = current_users + pending_invites

if email.lower() in current_users_and_pending_invites:
if email.lower() in current_users_and_pending_invites and not is_invite_edit:
return _("A user with this email address is already in "
"this project or has a pending invitation.")
web_user = WebUser.get_by_username(email)
Expand Down
17 changes: 13 additions & 4 deletions corehq/apps/users/static/users/js/custom_data_fields.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* This file defines a model for editing custom data fields while creating or editing a mobile user.
*/
'use strict';

Check warning on line 4 in corehq/apps/users/static/users/js/custom_data_fields.js

View workflow job for this annotation

GitHub Actions / Lint Javascript

'use strict' is unnecessary inside of modules

hqDefine("users/js/custom_data_fields", [
'knockout',
Expand Down Expand Up @@ -32,23 +32,32 @@
self.profile_slug = options.profile_slug;
self.slugs = options.slugs;
self.can_edit_original_profile = options.can_edit_original_profile;
self.initVals = options.initial_values || {};

var originalProfileFields = {},
originalProfileId,
originalProfile;
if (options.user_data) {
if (Object.keys(options.user_data).length) {
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
originalProfileId = options.user_data[options.profile_slug];
if (originalProfileId) {
originalProfile = self.profiles[originalProfileId];
if (originalProfile) {
originalProfileFields = originalProfile.fields;
}
}
} else if (Object.keys(self.initVals).length) {
originalProfileId = self.initVals['profile_id'];
originalProfile = self.profiles[originalProfileId];
if (originalProfile) {
originalProfileFields = originalProfile.fields;
}
}
_.each(self.slugs, function (slug) {
self[slug] = fieldModel({
value: options.user_data[slug] || originalProfileFields[slug],
});
var value = options.user_data[slug] || originalProfileFields[slug];
if (!value) {
value = self.initVals[slug];
}
self[slug] = fieldModel({value: value});
});

self.serialize = function () {
Expand Down
1 change: 1 addition & 0 deletions corehq/apps/users/static/users/js/invite_web_user.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
initialPageData,
customDataFields
) {
'use strict';

Check warning on line 15 in corehq/apps/users/static/users/js/invite_web_user.js

View workflow job for this annotation

GitHub Actions / Lint Javascript

'use strict' is unnecessary inside of modules

var inviteWebUserFormHandler = function () {
var self = {},
Expand Down Expand Up @@ -78,6 +78,7 @@
profile_slug: initialPageData.get('custom_fields_profile_slug'),
slugs: initialPageData.get('custom_fields_slugs'),
can_edit_original_profile: true,
initial_values: initialPageData.get('initial_values'),
});
}

Expand Down
4 changes: 4 additions & 0 deletions corehq/apps/users/static/users/js/web_users.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ hqDefine("users/js/web_users",[
});
};

self.inviteEditUrl = ko.computed(function () {
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
return initialPageData.reverse("edit_invitation", self.uuid);
});

return self;
};

Expand Down
1 change: 1 addition & 0 deletions corehq/apps/users/templates/users/invite_web_user.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
{% initial_page_data 'custom_fields_slugs' custom_fields_slugs %}
{% initial_page_data 'custom_fields_profiles' custom_fields_profiles %}
{% initial_page_data 'custom_fields_profile_slug' custom_fields_profile_slug %}
{% initial_page_data 'initial_values' initial_values %}
{% registerurl "check_sso_trust" domain %}

<div id="invite-web-user-form"
Expand Down
9 changes: 8 additions & 1 deletion corehq/apps/users/templates/users/web_users.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
{% registerurl "delete_request" domain %}
{% registerurl "paginate_web_users" domain %}
{% registerurl "reinvite_web_user" domain %}
{% registerurl "edit_invitation" domain '---' %}

<p class="lead">
{% blocktrans with hq_name=commcare_hq_names.COMMCARE_HQ_NAME cc_name=commcare_hq_names.COMMCARE_NAME %}
Expand Down Expand Up @@ -270,10 +271,16 @@ <h3 class="panel-title" style="padding-top: 7px;">{% trans 'Pending Invitations'
<td>
{% if not request.is_view_only %}
<div data-bind="visible: !actionMessage()">
<a data-bind="attr: {href: inviteEditUrl()}">
<button type="button" class="btn btn-default" data-bind="disable: actionInProgress">
<i class="fa-regular fa-edit"></i>
{% trans "Edit" %}
</button>
</a>
<button type="button" class="resend-invite btn btn-default"
data-bind="click: resend, disable: actionInProgress">
<i class="fa-regular fa-envelope"></i>
{% trans "Resend Invitation" %}
{% trans "Resend" %}
</button>
<button type="button" class="btn btn-danger delete-invitation"
data-bind="click: $root.confirmRemoveInvitation, disable: actionInProgress">
Expand Down
1 change: 1 addition & 0 deletions corehq/apps/users/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
url(r'^web/remove/(?P<couch_user_id>[ \w-]+)/$', remove_web_user, name='remove_web_user'),
url(r'^web/undo_remove/(?P<record_id>[ \w-]+)/$', undo_remove_web_user, name='undo_remove_web_user'),
url(r'^web/invite/$', InviteWebUserView.as_view(), name=InviteWebUserView.urlname),
url(r'^web/invite/edit/(?P<invitation_id>[ \w-]+)/$', InviteWebUserView.as_view(), name='edit_invitation'),
url(r'^web/reinvite/$', reinvite_web_user, name='reinvite_web_user'),
url(r'^web/request/$', DomainRequestView.as_view(), name=DomainRequestView.urlname),
url(r'^web/delete_invitation/$', delete_invitation, name='delete_invitation'),
Expand Down
76 changes: 58 additions & 18 deletions corehq/apps/users/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1140,9 +1140,23 @@ def invite_web_user_form(self):
role_choices = get_editable_role_choices(self.domain, self.request.couch_user, allow_admin_role=True)
domain_request = DomainRequest.objects.get(id=self.request_id) if self.request_id else None
is_add_user = self.request_id is not None
initial = {
'email': domain_request.email if domain_request else None,
}
invitation = self.invitation
if invitation:
assigned_location = invitation.assigned_locations.filter()
assigned_location_ids = [loc.location_id for loc in assigned_location]
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
primary_location_id = None
if assigned_location_ids:
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this check primary_location instead of assigned_location_ids? It should work the same in either case, just seems clearer

primary_location_id = invitation.primary_location.location_id
initial = {
'email': invitation.email,
'role': invitation.role,
'assigned_locations': assigned_location_ids,
'primary_location': primary_location_id,
}
else:
initial = {
'email': domain_request.email if domain_request else None,
}
can_edit_tableau_config = (self.request.couch_user.has_permission(self.domain, 'edit_user_tableau_config')
and toggles.TABLEAU_USER_SYNCING.enabled(self.domain))
if self.request.method == 'POST':
Expand All @@ -1154,7 +1168,8 @@ def invite_web_user_form(self):
should_show_location=self.request.project.uses_locations,
can_edit_tableau_config=can_edit_tableau_config,
request=self.request,
custom_data=self.custom_data
custom_data=self.custom_data,
invitation=invitation
)
return AdminInvitesUserForm(
initial=initial,
Expand All @@ -1164,7 +1179,8 @@ def invite_web_user_form(self):
should_show_location=self.request.project.uses_locations,
can_edit_tableau_config=can_edit_tableau_config,
request=self.request,
custom_data=self.custom_data
custom_data=self.custom_data,
invitation=invitation
)

@cached_property
Expand Down Expand Up @@ -1194,7 +1210,7 @@ def page_context(self):
ctx = {
'registration_form': self.invite_web_user_form,
**self.custom_data.field_view.get_field_page_context(
self.domain, self.request.couch_user, self.custom_data, None
self.domain, self.request.couch_user, self.custom_data, None, self.invitation
)
}
return ctx
Expand All @@ -1204,6 +1220,14 @@ def _assert_user_has_permission_to_access_locations(self, assigned_location_ids)
self.domain, self.request.couch_user).values_list('location_id', flat=True))):
raise Http404()

@property
def invitation(self):
invitation_id = self.kwargs.get("invitation_id")
try:
return Invitation.objects.get(uuid=invitation_id)
except Invitation.DoesNotExist:
return None

def post(self, request, *args, **kwargs):
if self.invite_web_user_form.is_valid():
# If user exists and has already requested access, just add them to the project
Expand All @@ -1215,7 +1239,22 @@ def post(self, request, *args, **kwargs):
profile = CustomDataFieldsProfile.objects.get(
id=profile_id,
definition__domain=self.domain) if profile_id else None
if domain_request is not None:
invitation = self.invitation
if invitation:
create_invitation = False
invitation.profile = profile
invitation.primary_location, assigned_locations = self.get_sql_locations(
data.pop("primary_location", None), data.pop("assigned_locations", []))
invitation.assigned_locations.set(assigned_locations)
invitation.custom_user_data = data.get("custom_user_data", {})

invitation.role = data.get("role", None)
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
invitation.program = data.get("program", None)
invitation.tableau_role = data.get("tableau_role", None)
invitation.tableau_group_ids = data.get("tableau_group_ids", None)
invitation.save()
messages.success(request, "Invite to %s was successfully updated." % data["email"])
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
elif domain_request is not None:
domain_request.is_approved = True
domain_request.save()
user = CouchUser.get_by_username(domain_request.email)
Expand Down Expand Up @@ -1243,20 +1282,11 @@ def post(self, request, *args, **kwargs):
data["invited_by"] = request.couch_user.user_id
data["invited_on"] = datetime.utcnow()
data["domain"] = self.domain
primary_location_id = data.pop("primary_location", None)
data["primary_location"] = (SQLLocation.by_location_id(primary_location_id)
if primary_location_id else None)
assigned_location_ids = data.pop("assigned_locations", [])
if primary_location_id:
assert primary_location_id in assigned_location_ids
self._assert_user_has_permission_to_access_locations(assigned_location_ids)
data["profile"] = profile
data["primary_location"], assigned_locations = self.get_sql_locations(
data.pop("primary_location", None), data.pop("assigned_locations", []))
invite = Invitation(**data)
invite.save()

assigned_locations = [SQLLocation.by_location_id(assigned_location_id)
for assigned_location_id in assigned_location_ids
if assigned_location_id is not None]
invite.assigned_locations.set(assigned_locations)
invite.send_activation_email()

Expand All @@ -1271,6 +1301,16 @@ def post(self, request, *args, **kwargs):
))
return self.get(request, *args, **kwargs)

def get_sql_locations(self, primary_location_id, assigned_location_ids):
Jtang-1 marked this conversation as resolved.
Show resolved Hide resolved
primary_location = (SQLLocation.by_location_id(primary_location_id) if primary_location_id else None)
if primary_location_id:
assert primary_location_id in assigned_location_ids
self._assert_user_has_permission_to_access_locations(assigned_location_ids)
assigned_locations = [SQLLocation.by_location_id(assigned_location_id)
for assigned_location_id in assigned_location_ids
if assigned_location_id is not None]
return primary_location, assigned_locations


class BaseUploadUser(BaseUserSettingsView):
def post(self, request, *args, **kwargs):
Expand Down
8 changes: 7 additions & 1 deletion corehq/apps/users/views/mobile/custom_data_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,19 @@ def get_displayable_profiles_and_edit_permission(cls, original_profile_id, domai

@classmethod
def get_field_page_context(cls, domain, couch_user, custom_data_editor: CustomDataEditor,
original_profile_id=None):
original_profile_id=None, invitation=None):
profiles, can_edit_original_profile = (
cls.get_displayable_profiles_and_edit_permission(
original_profile_id, domain, couch_user
)
)
serialized_profiles = [p.to_json() for p in profiles]

initial_values = {}
if invitation:
initial_values = {f.slug: invitation.custom_user_data.get(f.slug) for f in custom_data_editor.fields}
if invitation.profile:
initial_values["profile_id"] = invitation.profile.id
return {
'can_edit_original_profile': can_edit_original_profile,
'custom_fields_slugs': [f.slug for f in custom_data_editor.fields],
Expand All @@ -118,6 +123,7 @@ def get_field_page_context(cls, domain, couch_user, custom_data_editor: CustomDa
],
'custom_fields_profiles': sorted(serialized_profiles, key=lambda x: x['name'].lower()),
'custom_fields_profile_slug': PROFILE_SLUG,
'initial_values': initial_values,
}

@classmethod
Expand Down
4 changes: 4 additions & 0 deletions corehq/tabs/tabclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -1649,6 +1649,10 @@ def _get_web_username(request=None, couch_user=None, **context):
'title': _("Invite Web User"),
'urlname': 'invite_web_user'
},
{
'title': _("Edit Web User Invite"),
'urlname': 'edit_invitation'
},
{
'title': _get_web_username,
'urlname': EditWebUserView.urlname
Expand Down
Loading