Skip to content

Record list fields#2927

Open
brady-lamansky-gtt wants to merge 13 commits into
DiscipleTools:developfrom
brady-lamansky-gtt:record-list-fields
Open

Record list fields#2927
brady-lamansky-gtt wants to merge 13 commits into
DiscipleTools:developfrom
brady-lamansky-gtt:record-list-fields

Conversation

@brady-lamansky-gtt

@brady-lamansky-gtt brady-lamansky-gtt commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

This resolves #2845 to replace record-list fields with dt-web-components. Filtering functionality was updated to accommodate the new changes & appeared to work properly, but more testing is likely needed.

I updated these things:

  • Custom Filter Creation
  • Bulk Edit
  • Field Selection (for the displayed fields)

@cairocoder01 let me know any suggestions!

Related Components PR: DiscipleTools/disciple-tools-web-components#208

@cairocoder01

Copy link
Copy Markdown
Collaborator

After a brief review manually and bigger Claude review, here's some preliminary things to fix:

The issue may have been misleading about replacing all components in the filters, since some of the filter controls don't match the component normally used for that field.

Filters

Regressions

  • Date fields no longer support range selection
  • key_select fields no longer support selecting multiple values to search for
  • boolean fields no longer support selecting multiple values to search for
  • Filter chips display raw keys instead of labels (e.g. Contact Status: active instead of expected Contact Status: Active

Bugs

  • Connection fields show the button to add a new connection. No need to add from the filter selection

@cairocoder01

Copy link
Copy Markdown
Collaborator

Here is a full Claude review I did locally while it had access to a dev site running the code:

Code Review: Record List Fields → Web Components (record-list-fields)

Reviewed the diff against origin/develop (source files: archive-template.php,
dt-assets/js/modular-list.js, dt-assets/js/modular-list-bulk.js,
dt-core/utilities/dt-components.php, dt-assets/scss/_components.scss) and
verified the runtime behavior live on dtagent.local (deployed via wp-deploy)
with network capture and the PHP debug log.


🔴 High

1. Date filters are silently broken (return unfiltered results)

archive-template.php:994 renders a single dt-date, and in
modular-list.js:1453-1469 date/datetime fields fall through into the generic
"single select" branch, producing val = [customComponent.value].

The list query builder in dt-posts/posts.php:874-884 only applies a date
constraint when the value is an object keyed by start/end. An array key (0)
matches neither, so no WHERE clause is emitted — the filter is silently ignored
and all records are returned.

Verified live — applying a Baptism Date filter posts:

{"fields":[{"baptism_date":["2024-01-15"]}, ...]}
// expected: {"baptism_date":{"start":"2024-01-15"}}

This is also a capability regression: the old UI had Range Start / Range End;
the new one renders one date input, so date-range filtering is gone entirely.

Fix: add a dedicated date branch that emits { [field]: { start, end } } and a
start/end UI (two dt-dates).


🟠 Medium

2. render_toggle throws PHP warnings on every list load

dt-components.php:325 does checked( $post[$field_key], '1', false ), but the
new call sites pass an empty $post ([]) — archive-template.php:466 (archived
toggle) and archive-template.php:1005 (every boolean filter field). Confirmed in
debug.log:

PHP Warning: Undefined array key "archivedToggle" in .../dt-components.php on line 325
PHP Warning: Undefined array key "favorite" ...
PHP Warning: Undefined array key "requires_update" ...

Fix: checked( $post[$field_key] ?? '', '1', false ) in render_toggle.

3. key_select and boolean filters regressed to single-value

archive-template.php:964 renders key_select as dt-single-select and
boolean as dt-toggle. Previously these were multi-checkbox groups.
Consequences:

  • key_select (e.g. Contact Status): can no longer filter for multiple statuses
    at once, and the explicit "None Set" option is gone. The backend still
    supports value arrays (dt-posts/posts.php:886) — only the UI regressed.
  • boolean: the old Yes/No checkboxes are replaced by one toggle, so filtering
    for "No"/unset (0) is no longer expressible.

4. Field-selection save persists stale - delete markers

modular-list.js:890-898 reads $('#field_search_input').val() directly. When a
user removes a column, dt-multi-select marks it as -fieldkey rather than
removing it. Verified live — after removing "Name", the saved value is:

["-name","last_modified","favorite","overall_status","assigned_to","milestones","groups","seeker_path"]

That -name gets written to the fields_to_show_in_table cookie and sent as
fields_to_return. The filter-change handler already strips these
(modular-list.js:1449-1452); save_column_choices should do the same
(.filter(k => k.charAt(0) !== '-')). The now-dead try/catch around it (no JSON
parsing anymore) should also be removed.

5. Filter chips display raw keys instead of labels

For key_select/multi_select/tags, create_name_value_label
(modular-list.js:1735) interpolates the value directly. Verified live the chips
read "Contact Status: active" and "Sources: web,phone" instead of the
translated option labels ("Active", "Web, Phone"). Connections/users/locations are
fine (they use item.label). The change handler should resolve option keys to
labels before building the chip text.


🟡 Low / cleanup

(the issue explicitly asks to remove now-unused code)

  • Dead field-picker code left behind after the template rewrite: the
    .remove-field-btn handler and #selected_fields_input /
    #enabled_fields_container references (modular-list.js:866-888) and in
    reset_column_choices (modular-list.js:908) — none of those elements exist
    anymore.
  • Dead .text-comms-filter-input keyup handler (modular-list.js:1774) — that
    class is no longer rendered (fields are now dt-text), so the old "auto-select
    the default radio while typing" behavior is silently gone. Text filtering still
    works, but confirm this UX loss is intended.
  • Dead .clear-date-picker handler (modular-list.js:2323) — those buttons
    were removed from the date template.
  • Duplicate/mismatched selector in the modal-close reset
    (modular-list.js:2115): dt-location is listed twice, and dt-location-map is
    in the reset branch's tag list but not in the query selector.
  • dt-assets/scss/_components.scss has no trailing newline (line 17) —
    PHPCS/prettier nitpick.

What I verified vs. inferred

Live-confirmed on dtagent.local: date query shape (#1), PHP warnings (#2), stale
- markers (#4), raw-key chips (#5), single-select rendering (#3). The
double-labeling I initially suspected for text/comms fields turned out not to
be a bug — the radio handler correctly replaces the change-handler chip (net one
chip). Connection/user/location/multi-select/tags query building looks correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace components: record list

3 participants