User management fields

[brady-lamansky-gtt]

@cairocoder01 This resolves #2847 to replace user management fields w dt-web-components.
Please specifically look at the syntax and formatting of my changes to make sure it's what you're looking for!

[cairocoder01]

This is a great start. See the specific code comments, but there are also some functional issues:

• When using the new user form, the submit doesn't work because it returns an API error
• When changing fields of an existing user, they don't automatically save like they used to. We need to hook up the code to capture change events and trigger the API requests
• Those will probably require js changes. Try to find the js code that was specific to the old form fields and remove it as you implement the new code for the components.

[cairocoder01]

• We've lost the spacing in between the fields and the following label. See image below. Green is good, red is bad and needs to match green.

[cairocoder01]

Looking better.

• We still need the margin adjustments on the user management page for editing existing users.
• I found a weird issue when editing users. I set the gender of a user to Female (had been blank), and then when I opened up any other user, it also showed Female even though they should have been blank. I'm having trouble reproducing it again after setting those values.
• Related to that, as I was trying to reproduce it, I also had a situation where I changed another user to Male from blank, it saved, I closed to modal and then opened the same user, but the gender was blank again. If I refreshed the page, it correctly showed Male. I think whatever js is loading the modal had the original value instead of the updated one.

[brady-lamansky-gtt]

Just fixed those bugs! Let me know any other suggestions. For the Gender bug, I realize it was because I had 'select_cannot_be_empty' => true even though the field was supposed to be optional.

[cairocoder01]

I pushed one commit to reset all of the component fields before loading in new values. It was causing an issue where saved icons were still displaying on a second user modal and the gender field wasn't cleared out if the value was "none" and the previous had a value.

As I was looking in the subassigned code, I see there's a feature to check the URL for a query param and preset the subassigned field (/user-management/add-user?contact_id=691). Let's make sure that still works. It happens when you're on a contact, open the Admin Actions dropdown, and select the option to make that contact into a user.

[github-actions]

Code Review

High

1. Password field rendered as plain text — dt-users/template-new-user.php

The password field is now rendered with DT_Components::render_text('ddrowssap', ...), which outputs a <dt-text> web component (a text input). Unlike the removed <input type="password">, dt-text does not mask the characters the user types. When a user expands the hidden-fields panel and enters a password, it will be visible in clear text.

// dt-users/template-new-user.php (new code)
DT_Components::render_text( 'ddrowssap', [
    'ddrowssap' => [
        'name' => __( 'Password', 'disciple_tools' ),
        'type' => 'text',  // renders <dt-text>, not <input type="password">
        ...
    ]
], [], ... )

The original <input type="password" id="drowssap"> should either be kept as-is or replaced with a web component that supports type="password".

---

Medium

2. Optional fields silently dropped on new user creation — dt-users/template-new-user.php

The JS collects optional field values with:

const optionalFields = document.querySelectorAll('[data-optional=""]');

In the old template, fields like first_name, last_name, and site-defined fields all had data-optional directly on their <input> elements. The new DT_Components::render_text() / render_key_select() calls don't emit this attribute — shared_attributes() in dt-components.php only forwards placeholder from $params, not arbitrary attributes like data-optional. As a result, when creating a new user, first name, last name, gender, and any site-defined optional fields are silently dropped from the API payload. Only description (written out directly with data-optional in the HTML) is unaffected.

Fix: either pass data-optional through shared_attributes() for known params, or write those fields out as direct HTML elements with the attribute present.

---

3. Language names with apostrophes break the language dropdown — dt-users/user-management.js

write_language_dropdown() builds a <dt-single-select> element in a JS template literal:

return `<dt-single-select ... options='${JSON.stringify(options)}'></dt-single-select>`;

JSON.stringify does not escape single-quote characters in strings. If any native_name contains ' (e.g. the N'Ko script language), the JSON placed inside the single-quoted options='...' attribute will break HTML parsing, preventing the language dropdown from rendering.

PHP's render_key_select() avoids this correctly with esc_attr(json_encode(...)). The JS equivalent would be replacing single quotes in the serialized JSON with ' before inserting into the attribute, or switching to a value + JSON approach that doesn't rely on single-quoted HTML attribute syntax.

---

4. Dead typeahead initialization for the subassigned field — dt-users/user-management.js

Inside write_add_user(), the jQuery typeahead is still initialized targeting .js-typeahead-subassigned:

// subassigned check here
['subassigned'].forEach((field_id) => {
  $.typeahead({
    input: `.js-typeahead-${field_id}`,
    ...
  });
});

The <input class="js-typeahead-subassigned"> element was removed from template-new-user.php and replaced with a <dt-connection> web component. The typeahead will silently find no matching elements. The developer left a // subassigned check here comment indicating this needs follow-up. If the dt-connection component does not emit an event that the existing corresponds_to_contact tracking captures, the "link to existing contact" option on user creation may be broken.

---

5. ComponentService initialized with stale user context in add-user flow — dt-users/user-management.js

write_add_user() now creates a ComponentService using window.current_user_lookup:

function write_add_user() {
    const componentService = new window.DtWebComponents.ComponentService(
      'users',
      window.current_user_lookup,   // may be null, or a previously-opened user's ID
      window.wpApiShare.nonce,
    );
    componentService.attachLoadEvents();

write_add_user() is called when the "Add User" button is clicked. At that point window.current_user_lookup is either undefined (first action on the page) or set to a user who was previously opened. If attachLoadEvents() wires up change-save listeners that fire API requests against that stale ID, edits in the new-user form could silently update an unrelated existing user instead of being held until after the new user is created.

---

Summary

The PR is not ready to merge. The password-masking regression is a clear security issue, and the optional-fields bug means profile data is silently dropped for every new user created through this form.

[brady-lamansky-gtt]

I originally set this up for type="password" for dt-text, but it was causing formatting errors from some native styling. So, I changed the "type" to "data_type" in my local dt-web-components file in the meantime. How do you recommend we handle this?

[cairocoder01]

@brady-lamansky-gtt Can you explain more about the formatting errors it was causing?

[brady-lamansky-gtt]

@cairocoder01 In code, I had the following:
<?php DT_Components::render_text( 'ddrowssap', [ 'ddrowssap' => [ 'name' => __( 'Password', 'disciple_tools' ), 'type' => 'text', 'default' => '', ] ], [], [ 'type' => 'password', 'placeholder' => __( 'Password', 'disciple_tools' ) ] ) ?>

So the component was still a "text" type, but when rendering I set "type = password" using custom attributes. Below is what it looked like. It looked like this, but the text would still show as dots as if it were a password. When talking to Gemini, it said the default "type = password" styling was conflicting with our custom "type = password" functionality on the component.

[cairocoder01]

I took a look and the problem was some styles from the Foundation library that's in use in the project. I added some CSS to reset those styles for dt-text elements. This fixes the styling issue and allows us to still use the type attribute that the component is actually looking for.

[github-actions]

Code Review

Issues not already raised in prior reviews.

High

1. Crash when user removes the selected contact — dt-users/user-management.js

The new #subassigned change handler assumes e.target.value always has at least one element:

} else if (\!e.target.value[0].delete) {   // throws if e.target.value is []
    contact_id = e.target.value[0].id;
} else {
    contact_id = '';
}

When the user removes the selected contact from the dt-connection component, the component fires a change event with an empty array. e.target.value[1] is undefined (falsy), so we fall through to the else if where e.target.value[0] is also undefined — accessing .delete on it throws TypeError: Cannot read properties of undefined. The submit button is left disabled and the form is stuck.

Fix: guard against an empty value array before accessing index 0:

if (\!e.target.value || e.target.value.length === 0) {
    contact_id = '';
} else if (e.target.value[1] && \!e.target.value[1].delete) {
    ...

---

Medium

2. Debug console.log left in production code — dt-users/user-management.js

Inside the getContact callback (the overwriteTypeahead branch):

if (overwriteTypeahead) {
    console.log('1');
    const safeName = ...

This fires every time the add-user form is pre-filled from the Admin Actions "Make contact a user" flow (/user-management/add-user?contact_id=…). Remove before merge.

---

3. workload_status silently forced to a non-empty value — dt-users/template-user-management.php

The old template had an empty <option> for workload_status, allowing it to be unset. The PR adds 'select_cannot_be_empty' => true, removing that empty option. For any user whose stored workload status is null, the component will display the first option (e.g. "Active") on load, and because the auto-save listener fires on change, subsequent interaction could write that default back — silently overwriting their actual state. user_status being required makes sense, but workload status is optional. Consider removing select_cannot_be_empty from workload_status or adding a genuine empty/none entry to its options list.

[github-actions]

Code Review

Issues not already raised in prior reviews.

---

High

1. Removing a selected contact does not clear window.contact_record — dt-users/user-management.js

When the user removes the selected contact from the dt-connection subassigned field, the change handler calls getContact(null, false). Inside getContact, a null/falsy id triggers an early return:

function getContact(id, isUser = false, overwriteTypeahead = false) {
    $('#contact-result').empty();
    if (\!id) {
        return;  // window.contact_record is never cleared
    }
}

window.contact_record is only set inside the async AJAX response block — never reached when id is null. The stale reference from the previously selected contact persists after the user deselects it.

On form submit:

if (typeof window.contact_record \!== 'undefined') {
    $('#confirm-user-upgrade').foundation('open');  // fires even though contact was removed
}

The "upgrading a contact" confirmation modal appears unexpectedly, and if confirmed, create_user(window.contact_record.ID) links the new account to the previously-selected (but explicitly removed) contact.

Fix: clear window.contact_record on the early-return path:

if (\!id) {
    window.contact_record = undefined;
    submit_button.prop('disabled', false);
    return;
}

---

Medium

2. attr('value', jsonString) sets an HTML attribute, not the DOM property, on dt-connection — dt-users/user-management.js

In the overwriteTypeahead branch (reached via ?contact_id= URL prefill):

$('#subassigned').attr('value', jsonString);

jQuery's .attr() calls element.setAttribute(...). Web components that hold complex (array) values typically react to the value property setter, not attribute mutations after initialization. If dt-connection does not list value in observedAttributes, the displayed state does not update and the pre-selected contact is invisible to the user. The form submission itself is unaffected (because window.contact_record is set correctly by the same AJAX response), but the visual state is wrong.

The correct approach: document.getElementById('subassigned').value = dataArray;

---

3. Email field loses type="email" browser validation — dt-users/template-new-user.php

DT_Components::render_text( 'liame', [
    'liame' => [ 'type' => 'text', ... ]
], [], [ 'placeholder' => ... ] )

No 'type' => 'email' is passed in $params, so shared_attributes() emits type="". The original <input type="email"> enforced email format in the browser and showed the correct mobile keyboard. Any non-empty string now passes the JS check if (email \!== '') and reaches the server without format validation.

Fix: pass 'type' => 'email' in the $params array and verify dt-text forwards it to its inner <input>.

---

Summary

The PR is not ready to merge — the stale contact_record reference causes incorrect user-to-contact linking whenever a user selects and then removes a contact on the add-user form.

[github-actions]

---

Code Review

Issues not already raised in prior reviews.

---

High

1. shared_attributes() always emits placeholder="", breaking render_location and render_tags globally — dt-core/utilities/dt-components.php

The change adds placeholder="' . esc_attr( $params['placeholder'] ?? '' ) . '" to shared_attributes(). When callers don't pass $params['placeholder'] (the normal case for almost every existing component), this emits placeholder="" as the first attribute on the element.

The HTML spec drops duplicate attributes, keeping only the first occurrence. Two render functions explicitly set their own placeholder after shared_attributes():

• render_location (dt-components.php ~line 147) sets placeholder="Search Locations" after echoing $shared_attributes — the HTML parser now drops it, leaving placeholder=""
• render_tags (~line 285) sets placeholder="Search [field_name]" after $shared_attributes — same result

Every dt-location and dt-tags component across the entire application (contacts, groups, every post type) silently loses its search placeholder text. This is a regression that affects all post-type forms, not just user management.

Fix: Only emit placeholder when the param is actually provided:

( isset( $params['placeholder'] ) ? 'placeholder="' . esc_attr( $params['placeholder'] ) . '"' : '' )

---

Medium

2. Submit button re-enabled synchronously before getContact async call resolves — dt-users/user-management.js

The new #subassigned change handler:

$('#subassigned').on('change', function (e) {
  submit_button.prop('disabled', true);
  // ...
  getContact(contact_id, false);          // async, NOT awaited
  submit_button.prop('disabled', false);  // re-enabled immediately
});

When a contact is selected, getContact() fires an API request and returns. The button is re-enabled on the very next synchronous line before the response arrives. If the user clicks submit in that window, window.contact_record is still undefined, causing the submit handler to skip the #confirm-user-upgrade modal and call create_user() without corresponds_to_contact. The new user is created without being linked to the contact. The old typeahead onClick callback kept the button disabled until getContact completed.

Fix: Remove submit_button.prop('disabled', false) at the end of the synchronous handler body — getContact() already re-enables the button in both its early-return path and its API success path.

---

Summary

The PR is not ready to merge. The shared_attributes() placeholder change is a system-wide regression that breaks location search and tag search fields across all post types.

[github-actions]

Code Review

Issues not already raised in prior reviews.

---

Medium

1. Orphaned Foundation toggle label produces duplicate switch UI — dt-users/user-management.js

The diff replaces the <input type="checkbox" class="switch-input app-state-switches"> (around line 542) with <dt-toggle class="app-state-switches">, but the immediately following <label class="switch-paddle"> block (lines 547–551) is not removed.

Foundation's toggle-switch pattern works by hiding the checkbox (.switch-input sets position: absolute; opacity: 0) and rendering the <label class="switch-paddle"> as the visible control. With <input type="checkbox"> gone, the Foundation CSS rule that hides the control no longer matches <dt-toggle>, so both the dt-toggle component's own UI and the now-orphaned Foundation label render side by side for every magic link app row. The result is two visible toggle switches per row.

Fix: remove the <label class="switch-paddle"> block from the template string when switching to <dt-toggle>, since dt-toggle renders its own label/state UI internally.

---

Summary

The PR is making steady progress. One new rendering regression was found: the orphaned Foundation <label class="switch-paddle"> creates a duplicate toggle UI for each magic link app. The issues flagged in earlier reviews (password masking, email type, stale ComponentService context, synchronous submit-button re-enable, console.log('1'), .attr('value') vs property setter, and single-quote escaping in the language dropdown) remain open.

[github-actions]

Code Review

Issues not already raised in prior reviews. Several previous findings have been fixed in the latest commits: the stale contact_record on deselect, the e.target.value[0] crash on empty array, workload_status empty-option suppression, shared_attributes() always emitting placeholder="", the orphaned Foundation label, and data-optional/type attribute forwarding.

---

High

1. ComponentService in open_user_modal captures the previous user's ID — dt-users/user-management.js

The new code at the top of open_user_modal constructs ComponentService before updating window.current_user_lookup:

window.open_user_modal = (user_id) => {
  const componentService = new window.DtWebComponents.ComponentService(
    'users',
    window.current_user_lookup,   // PREVIOUS user's ID, not user_id
    window.wpApiShare.nonce,
  );
  componentService.attachLoadEvents();
  // ...
  window.current_user_lookup = user_id;  // updated only here (line 63)

Every time a second user modal is opened (click User A, close, click User B), ComponentService is constructed with User A's ID. If attachLoadEvents() registers change-save listeners keyed to that constructor-time ID, field edits in User B's modal are saved to User A. The global jQuery change handlers ($('dt-text, dt-textarea').change(), $('dt-single-select').change()) correctly read window.current_user_lookup at fire time (User B), so two saves fire: one to the wrong user via ComponentService, one to the correct user via jQuery. User A's data gets silently overwritten.

The prior review flagged the same pattern in write_add_user(), but that case receives a null/undefined ID so no data is lost. Here the constructor receives a real previous user's ID on every non-first modal open.

Fix: move ComponentService construction to after window.current_user_lookup = user_id, or pass user_id directly:

window.current_user_lookup = user_id;
const componentService = new window.DtWebComponents.ComponentService(
  'users',
  user_id,
  window.wpApiShare.nonce,
);
componentService.attachLoadEvents();

---

Summary

Remaining open from prior reviews: synchronous submit-button re-enable in the #subassigned change handler, .attr('value') vs. property setter for the contact prefill path, language dropdown single-quote injection in JSON attribute, email type="email" loss, and ComponentService stale context in write_add_user(). The new finding above is the more severe variant of that last item — it causes data corruption on every non-first modal open. The PR is not ready to merge.

[cairocoder01]

@corsacca This looks good to me. I've reviewed all the Claude reviews and fixed what seemed important but have left a number of the medium issues. Ready for your review.

[corsacca]

Thanks guys!