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

Add Custom Data to Web User Invite #35124

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
92 changes: 52 additions & 40 deletions corehq/apps/custom_data_fields/edit_entity.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ def with_prefix(string, prefix):
return "{}-{}".format(prefix, string)


def without_prefix(string, prefix):
prefix_len = len(prefix) + 1
return string[prefix_len:] if string.startswith(prefix) else string


def add_prefix(field_dict, prefix):
"""
Prefix all keys in the dict.
Expand Down Expand Up @@ -131,13 +136,45 @@ def _make_field(self, field):
else:
return forms.CharField(label=safe_label, required=field.is_required)

def make_fieldsets(self, form_fields, is_post, field_name_includes_prefix=False):
if self.ko_model:
field_names = []
for field_name, field in form_fields.items():
data_bind_field_name = (
without_prefix(field_name, self.prefix) if field_name_includes_prefix else field_name)
data_binds = [
f"value: {self.ko_model}.{data_bind_field_name}.value",
f"disable: {self.ko_model}.{data_bind_field_name}.disable",
]
if hasattr(field, 'choices') or without_prefix(field_name, self.prefix) == PROFILE_SLUG:
data_binds.append("select2: " + json.dumps([
{"id": id, "text": text} for id, text in field.widget.choices
]))
field_names.append(Field(
field_name,
data_bind=", ".join(data_binds)
))
else:
field_names = list(form_fields)

form_fieldsets = []
if field_names:
form_fieldsets.append(Fieldset(
_("Additional Information"),
*field_names,
css_class="custom-data-fieldset"
))
if not is_post:
form_fieldsets.append(self.uncategorized_form)
return form_fieldsets

@property
@memoized
def fields(self):
return list(self.model.get_fields(required_only=self.required_only))

def init_form(self, post_dict=None):
fields = OrderedDict()
form_fields = OrderedDict()

from corehq.apps.users.views.mobile import UserFieldsView
has_profile_privilege_and_is_user_fields_view = (
Expand Down Expand Up @@ -167,7 +204,7 @@ def validate_profile_slug(value):
}
if not self.ko_model:
attrs.update({'class': 'hqwebapp-select2'})
fields[PROFILE_SLUG] = forms.IntegerField(
form_fields[PROFILE_SLUG] = forms.IntegerField(
label=_('Profile'),
required=False,
widget=Select(choices=[
Expand All @@ -177,74 +214,49 @@ def validate_profile_slug(value):
validators=[validate_profile_slug],
)
for field in self.fields:
fields[field.slug] = self._make_field(field)

if self.ko_model:
field_names = []
for field_name, field in fields.items():
data_binds = [
f"value: {self.ko_model}.{field_name}.value",
f"disable: {self.ko_model}.{field_name}.disable",
]
if hasattr(field, 'choices') or field_name == PROFILE_SLUG:
data_binds.append("select2: " + json.dumps([
{"id": id, "text": text} for id, text in field.widget.choices
]))
field_names.append(Field(
field_name,
data_bind=", ".join(data_binds)
))
else:
field_names = list(fields)
form_fields[field.slug] = self._make_field(field)

CustomDataForm = type('CustomDataForm', (forms.Form,), fields)
CustomDataForm = type('CustomDataForm', (forms.Form,), form_fields.copy())
if self.ko_model:
CustomDataForm.helper = HQModalFormHelper()
else:
CustomDataForm.helper = HQFormHelper()
CustomDataForm.helper.form_tag = False

additional_fields = []
if field_names:
additional_fields.append(Fieldset(
_("Additional Information"),
*field_names,
css_class="custom-data-fieldset"
))
if post_dict is None:
additional_fields.append(self.uncategorized_form)
form_fieldsets = self.make_fieldsets(form_fields, post_dict is not None)

CustomDataForm.helper.layout = Layout(
*additional_fields
*form_fieldsets
)

CustomDataForm._has_uncategorized = bool(self.uncategorized_form) and post_dict is None

if post_dict:
fields = post_dict.copy() # make mutable
form_data = post_dict.copy() # make mutable
elif self.existing_custom_data is not None:
fields = add_prefix(self.existing_custom_data, self.prefix)
form_data = add_prefix(self.existing_custom_data, self.prefix)
else:
fields = None
form_data = None

# Add profile fields so that form validation passes
if fields and has_profile_privilege_and_is_user_fields_view:
if form_data and has_profile_privilege_and_is_user_fields_view:

# When a field is disabled via knockout, it is not included in POST so this
# adds it back
if (post_dict and (with_prefix(PROFILE_SLUG, self.prefix)) not in post_dict
and not can_edit_original_profile):
fields.update({with_prefix(PROFILE_SLUG, self.prefix): original_profile_id})
form_data.update({with_prefix(PROFILE_SLUG, self.prefix): original_profile_id})
try:
profile_fields = CustomDataFieldsProfile.objects.get(
id=int(fields.get(with_prefix(PROFILE_SLUG, self.prefix))),
id=int(form_data.get(with_prefix(PROFILE_SLUG, self.prefix))),
definition__field_type=self.field_view.field_type,
definition__domain=self.domain,
).fields
except (ValueError, TypeError, CustomDataFieldsProfile.DoesNotExist):
profile_fields = {}
fields.update(add_prefix(profile_fields, self.prefix))
form_data.update(add_prefix(profile_fields, self.prefix))

self.form = CustomDataForm(fields, prefix=self.prefix)
self.form = CustomDataForm(form_data, prefix=self.prefix)
return self.form

@property
Expand Down
49 changes: 32 additions & 17 deletions corehq/apps/registration/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@
from crispy_forms import layout as crispy
from crispy_forms.helper import FormHelper

from corehq import privileges
from corehq.apps.accounting.utils import domain_has_privilege
from corehq.apps.analytics.tasks import track_workflow
from corehq.apps.custom_data_fields.models import PROFILE_SLUG
from corehq.apps.custom_data_fields.edit_entity import add_prefix, get_prefixed, with_prefix
from corehq.apps.domain.forms import NoAutocompleteMixin, clean_password
from corehq.apps.domain.models import Domain
from corehq.apps.hqwebapp import crispy as hqcrispy
from corehq.apps.programs.models import Program
from corehq.toggles import WEB_USER_INVITE_ADDITIONAL_FIELDS
from corehq.apps.users.forms import SelectUserLocationForm, BaseTableauUserForm
from corehq.apps.users.models import CouchUser

Expand Down Expand Up @@ -490,23 +491,22 @@ class AdminInvitesUserForm(SelectUserLocationForm):

def __init__(self, data=None, excluded_emails=None, is_add_user=None,
role_choices=(), should_show_location=False, can_edit_tableau_config=False,
*, domain, **kwargs):
custom_data=None, *, domain, **kwargs):
self.custom_data = custom_data if WEB_USER_INVITE_ADDITIONAL_FIELDS.enabled(domain) else None
if data and self.custom_data:
data = data.copy()
custom_data_post_dict = self.custom_data.form.data
data.update({k: v for k, v in custom_data_post_dict.items() if k not in data})
self.request = kwargs.get('request')
super(AdminInvitesUserForm, self).__init__(domain=domain, data=data, **kwargs)
self.can_edit_tableau_config = can_edit_tableau_config
domain_obj = Domain.get_by_name(domain)
self.fields['role'].choices = [('', _("Select a role"))] + role_choices
if domain_obj:
if domain_has_privilege(domain_obj.name, privileges.APP_USER_PROFILES):
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be missing something but why is this FF not being used anymore? Is it effectively putting all this behind the new flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should still be behind the domain privilege check, but it's now done from the CustomDataEditor form here If that conditional is false, then the profile field here won't be added to the fields meaning that self.custom_data.form.fields will also not contain the profile field.

self.fields['profile'] = forms.ChoiceField(choices=(), label="Profile", required=False)
from corehq.apps.users.views.mobile import UserFieldsView
self.valid_profiles = UserFieldsView.get_user_accessible_profiles(
self.domain, self.request.couch_user
)
if len(self.valid_profiles) > 0:
self.fields['profile'].choices = [('', '')] + [
(profile.id, profile.name) for profile in self.valid_profiles
]
if self.custom_data:
prefixed_fields = []
prefixed_fields = add_prefix(self.custom_data.form.fields, self.custom_data.prefix)
self.fields.update(prefixed_fields)
if domain_obj.commtrack_enabled:
self.fields['program'] = forms.ChoiceField(label="Program", choices=(), required=False)
programs = Program.by_domain(domain_obj.name)
Expand Down Expand Up @@ -536,6 +536,10 @@ def __init__(self, data=None, excluded_emails=None, is_add_user=None,
'profile' if ('profile' in self.fields and len(self.fields['profile'].choices) > 0) else None,
)
]
if self.custom_data:
custom_data_fieldset = self.custom_data.make_fieldsets(prefixed_fields, data is not None,
field_name_includes_prefix=True)
fields.extend(custom_data_fieldset)
if should_show_location:
fields.append(
crispy.Fieldset(
Expand Down Expand Up @@ -579,13 +583,12 @@ def __init__(self, data=None, excluded_emails=None, is_add_user=None,
),
)

def clean_profile(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this not used anywhere else? I don't see any renaming where it is called.

Copy link
Contributor Author

@Jtang-1 Jtang-1 Oct 3, 2024

Choose a reason for hiding this comment

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

The clean_<fieldname>() method is a hook for performing custom validation or any cleaning that is specific to that particular attribute on a specific field in a Django form. Here, the field containing the profile will actually have the field name <prefix>_profile so clean_profile won't work. So I moved the cleaning to the general clean() and handle it via _validate_profile()

profile_id = self.cleaned_data['profile']
if profile_id and profile_id not in {str(p.id) for p in self.valid_profiles}:
def _validate_profile(self, profile_id):
valid_profile_ids = {choice[0] for choice in self.custom_data.form.fields[PROFILE_SLUG].widget.choices}
if profile_id not in valid_profile_ids:
raise forms.ValidationError(
_('Invalid profile selected. Please select a valid profile.'),
)
return profile_id

def clean_email(self):
email = self.cleaned_data['email'].strip()
Expand All @@ -611,6 +614,18 @@ def clean(self):
for field in cleaned_data:
if isinstance(cleaned_data[field], str):
cleaned_data[field] = cleaned_data[field].strip()

if self.custom_data:
prefixed_profile_key = with_prefix(PROFILE_SLUG, self.custom_data.prefix)
prefixed_field_names = add_prefix(self.custom_data.form.fields, self.custom_data.prefix).keys()
custom_user_data = {key: cleaned_data.pop(key) for key in prefixed_field_names if key in cleaned_data}

if prefixed_profile_key in custom_user_data:
profile_id = custom_user_data.pop(prefixed_profile_key)
self._validate_profile(profile_id)
cleaned_data['profile'] = profile_id
cleaned_data['custom_user_data'] = get_prefixed(custom_user_data, self.custom_data.prefix)

return cleaned_data

def _initialize_tableau_fields(self, data, domain):
Expand Down
1 change: 1 addition & 0 deletions corehq/apps/users/management/commands/accept_invite.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
def handle(self, username, domain, **options):
try:
invitation = Invitation.objects.get(domain=domain, email=username, is_accepted=False)
except:

Check failure on line 16 in corehq/apps/users/management/commands/accept_invite.py

View workflow job for this annotation

GitHub Actions / Flake8

corehq/apps/users/management/commands/accept_invite.py#L16

Do not use bare 'except' (E722)
print("No invites found for %s in Project Space (%s)" % (username, domain))
return

user = CouchUser.get_by_username(username)
if not user:
print("No existing web users active for email address %s. This command can only activate existing web users" % username)

Check failure on line 22 in corehq/apps/users/management/commands/accept_invite.py

View workflow job for this annotation

GitHub Actions / Flake8

corehq/apps/users/management/commands/accept_invite.py#L22

Line too long (132 > 115 characters) (E501)
return

print("Accepting %s's invite to Project Space(%s)" % (username, domain))
Expand All @@ -30,6 +30,7 @@
invitation.assigned_locations.all().values_list('location_id', flat=True)),
program_id=invitation.program,
profile=invitation.profile,
custom_user_data=invitation.custom_user_data,
tableau_role=invitation.tableau_role,
tableau_group_ids=invitation.tableau_group_ids)
invitation.is_accepted = True
Expand Down
7 changes: 5 additions & 2 deletions corehq/apps/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ def add_domain_membership(self, domain, timezone=None, **kwargs):

def add_as_web_user(self, domain, role, primary_location_id=None,
assigned_location_ids=None, program_id=None, profile=None,
tableau_role=None, tableau_group_ids=None):
tableau_role=None, tableau_group_ids=None, custom_user_data=None):
if assigned_location_ids is None:
assigned_location_ids = []
domain_obj = Domain.get_by_name(domain)
Expand All @@ -564,9 +564,11 @@ def add_as_web_user(self, domain, role, primary_location_id=None,
if primary_location_id:
self.set_location(domain, primary_location_id, commit=False)
self.reset_locations(domain, assigned_location_ids, commit=False)
user_data = self.get_user_data(domain_obj.name)
if domain_has_privilege(domain_obj.name, privileges.APP_USER_PROFILES) and profile:
user_data = self.get_user_data(domain_obj.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

also here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does still require the domain privilege check for profiles and I believe it does do the check. Or maybe i'm misunderstanding your comment

user_data.update({}, profile_id=profile.id)
if custom_user_data:
user_data.update(custom_user_data)
if TABLEAU_USER_SYNCING.enabled(domain) and (tableau_role or tableau_group_ids):
if tableau_group_ids is None:
tableau_group_ids = []
Expand Down Expand Up @@ -2853,6 +2855,7 @@ def accept_invitation_and_join_domain(self, web_user):
assigned_location_ids=list(self.assigned_locations.all().values_list('location_id', flat=True)),
program_id=self.program,
profile=self.profile,
custom_user_data=self.custom_user_data,
tableau_role=self.tableau_role,
tableau_group_ids=self.tableau_group_ids
)
Expand Down
17 changes: 16 additions & 1 deletion corehq/apps/users/static/users/js/invite_web_user.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@
'jquery',
'knockout',
'hqwebapp/js/initial_page_data',
'users/js/custom_data_fields',
'hqwebapp/js/toggles',
'hqwebapp/js/bootstrap3/validators.ko',
'locations/js/widgets',
], function (
$,
ko,
initialPageData
initialPageData,
customDataFields,
toggles
) {
'use strict';

Check warning on line 16 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 @@ -68,6 +72,17 @@
&& self.emailDelayed.isValid()
&& !self.emailDelayed.isValidating();
});
if (toggles.toggleEnabled('WEB_USER_INVITE_ADDITIONAL_FIELDS')) {
var $customDataFieldsForm = $(".custom-data-fieldset");
if ($customDataFieldsForm.length) {
self.custom_fields = customDataFields.customDataFieldsEditor({
profiles: initialPageData.get('custom_fields_profiles'),
profile_slug: initialPageData.get('custom_fields_profile_slug'),
slugs: initialPageData.get('custom_fields_slugs'),
can_edit_original_profile: true,
});
}
}

return self;
};
Expand Down
5 changes: 5 additions & 0 deletions corehq/apps/users/templates/users/invite_web_user.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@
{% requirejs_main 'users/js/invite_web_user' %}

{% block page_content %}
{% if request|toggle_enabled:"WEB_USER_INVITE_ADDITIONAL_FIELDS" %}
{% 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 %}
{% endif %}
{% registerurl "check_sso_trust" domain %}

<div id="invite-web-user-form"
Expand Down
Loading
Loading