Add Custom Data to Web User Invite #35124

Jtang-1 · 2024-09-13T22:11:30Z

Product Description

Allows the user to define "user data" when creating a web user invite.

Technical Summary

USH-4895
Editing custom user data uses the CustomDataEditor class which generates the profile and user data fields for CustomDataForm. The need here is very similar from the form UI side. However, it does not require populating existing user data.

In order to reuse the CustomDataForm, I initialized an instance of CustomDataEditor which initializes the form. Then this instance of CustomDataEditor is injected into AdminInvitesUserForm. The fields and fieldsets created as part of CustomDataForm is then pulled out to create the field and fieldsets for AdminInvitesUserForm. One issue faced is that CustomDataForm applies a form level defined prefix for each field but this prefix is applied only upon rendering the form. But since we are not using that form directly and are only extracting the fields, adding the prefix needs to be manually handled in AdminInvitesUserForm.

Additionally, when a user data field is defined and populated by the profile, the field is disabled. Because of this, posted data will not include data for this field. CustomDataForm already handles that and adds profile related fields to the post data. AdminInvitesUserForm takes advantage of that by accessing post data via self.custom_data.form.data.

Feature Flag

New feature flag: WEB_USER_INVITE_ADDITIONAL_FIELDS

Safety Assurance

Safety story

Locally tested and both backend and frontend changes are gated behind a FF which limits the blast radius.

Automated test coverage

No automated test

QA Plan

QA-7109

Rollback instructions

This PR can be reverted after deploy with no further considerations

Labels & Review

Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

- Manually add a prefix to custom data fields for consistency with the `CustomDataEditor` form since all fields for that form have the prefix applied. This is will later be relevant for populating post data for disabled fields

…field

…ields

When a field is disabled, it is not included in the form post data. This manually adds the field data back in

MartinRiese · 2024-09-16T16:27:01Z

corehq/toggles/__init__.py

@@ -2528,6 +2528,13 @@ def _commtrackify(domain_name, toggle_is_enabled):
    namespaces=[NAMESPACE_DOMAIN]
 )

+WEB_USER_INVITE_ADDITIONAL_FIELDS = StaticToggle(
+    'web_user_invite_additional_fields',
+    'Enable additional fields in web user invite form for enhanced user details',


Should this have a "USH:" in front or is the automatically GAed?

Yes it should 63273ea

MartinRiese · 2024-09-16T16:35:31Z

corehq/apps/users/models.py

        if assigned_location_ids is None:
            assigned_location_ids = []
+        if custom_user_data is None:


is this needed? It seems like the check on line 572 would take care of this and I don's see custom_user_data used anywhere else.

you're right, that's not needed. 8405864

minhaminha

May have to read through again but looks good so far.

minhaminha · 2024-09-16T19:37:13Z

corehq/apps/registration/forms.py

        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):


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

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.

minhaminha · 2024-09-16T19:37:27Z

corehq/apps/users/models.py

        if domain_has_privilege(domain_obj.name, privileges.APP_USER_PROFILES) and profile:
-            user_data = self.get_user_data(domain_obj.name)


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

… needs to be manually removed for conditional check

… the prefix within the name as defined by the user

Jtang-1 · 2024-10-02T13:47:15Z

bump for review

MartinRiese · 2024-10-03T14:47:31Z

corehq/apps/registration/forms.py

@@ -584,13 +584,12 @@ def __init__(self, data=None, excluded_emails=None, is_add_user=None,
            ),
        )

-    def clean_profile(self):


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

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()

Jtang-1 added 11 commits September 11, 2024 23:04

prevent side effects of type() modifying the original fields dictionary

deae220

rename variables for clarity

e1e4800

prefactor by extracting creation of fieldsets

f7cfae7

adds custom data fields to django form

2a17686

- Manually add a prefix to custom data fields for consistency with the `CustomDataEditor` form since all fields for that form have the prefix applied. This is will later be relevant for populating post data for disabled fields

add blank field for profile selection

9515780

feat: adds data binding between profile selection and populatin data …

5a2e552

…field

remove old profile field as it is now defined with customDataEditor f…

a5f848e

…ields

manually populates post data to form

9945897

When a field is disabled, it is not included in the form post data. This manually adds the field data back in

feat: remove prefix from field key names for custom data

6c798c6

use custom data when creating web user

8347c90

handle case where custom_data is None

307933f

dimagimon added Risk: High Change affects files that have been flagged as high risk. Risk: Medium Change affects files that have been flagged as medium risk. labels Sep 13, 2024

gate inclusion of custom user data and profile behind feature flag

e0fc01e

Jtang-1 force-pushed the jt/add-custom-data-web-user-invite branch from 58d6529 to e0fc01e Compare September 13, 2024 22:30

Jtang-1 added the awaiting QA QA in progress. Do not merge label Sep 13, 2024

Jtang-1 requested review from a team, MartinRiese, minhaminha and Robert-Costello and removed request for a team September 13, 2024 23:52

Jtang-1 marked this pull request as ready for review September 14, 2024 00:11

Jtang-1 requested a review from esoergel as a code owner September 14, 2024 00:11

Jtang-1 requested review from orangejenny and AddisonDunn September 16, 2024 16:36

MartinRiese reviewed Sep 16, 2024

View reviewed changes

Jtang-1 added 3 commits September 16, 2024 11:41

update validation for selected profile

a177ab0

specify FF is for USH

63273ea

remove redundant conditional check

8405864

minhaminha reviewed Sep 16, 2024

View reviewed changes

Jtang-1 added 2 commits September 17, 2024 09:47

registration form fields for user data includes the prefix. So prefix…

df8af4e

… needs to be manually removed for conditional check

bug: fixes potential issue where the defined user data field contains…

aec22cd

… the prefix within the name as defined by the user

Jtang-1 added QA Passed and removed awaiting QA QA in progress. Do not merge labels Oct 2, 2024

MartinRiese reviewed Oct 3, 2024

View reviewed changes

MartinRiese approved these changes Oct 3, 2024

View reviewed changes

Jtang-1 merged commit 1af7a8f into master Oct 7, 2024
11 of 13 checks passed

Jtang-1 deleted the jt/add-custom-data-web-user-invite branch October 7, 2024 14:05

Jtang-1 mentioned this pull request Oct 10, 2024

Revert "Add Custom Data to Web User Invite" #35206

Merged

Jtang-1 restored the jt/add-custom-data-web-user-invite branch October 10, 2024 21:25

Jtang-1 deleted the jt/add-custom-data-web-user-invite branch October 10, 2024 21:30

Jtang-1 mentioned this pull request Oct 10, 2024

Revert "Revert "Add Custom Data to Web User Invite"" #35207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Custom Data to Web User Invite #35124

Add Custom Data to Web User Invite #35124

Jtang-1 commented Sep 13, 2024 •

edited by jira bot

Loading

MartinRiese Sep 16, 2024

Jtang-1 Sep 16, 2024

MartinRiese Sep 16, 2024

Jtang-1 Sep 16, 2024

minhaminha left a comment

minhaminha Sep 16, 2024

Jtang-1 Sep 16, 2024

minhaminha Sep 16, 2024

Jtang-1 Sep 16, 2024

Jtang-1 commented Oct 2, 2024

MartinRiese Oct 3, 2024

Jtang-1 Oct 3, 2024 •

edited

Loading

		if domain_has_privilege(domain_obj.name, privileges.APP_USER_PROFILES) and profile:
		user_data = self.get_user_data(domain_obj.name)

Add Custom Data to Web User Invite #35124

Add Custom Data to Web User Invite #35124

Conversation

Jtang-1 commented Sep 13, 2024 • edited by jira bot Loading

Product Description

Technical Summary

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Rollback instructions

Labels & Review

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minhaminha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jtang-1 commented Oct 2, 2024

Choose a reason for hiding this comment

Jtang-1 Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

Jtang-1 commented Sep 13, 2024 •

edited by jira bot

Loading

Jtang-1 Oct 3, 2024 •

edited

Loading