Skip to content

Add support to render tags as static list of options#2907

Open
cairocoder01 wants to merge 1 commit intoDiscipleTools:developfrom
cairocoder01:tags-as-static-options
Open

Add support to render tags as static list of options#2907
cairocoder01 wants to merge 1 commit intoDiscipleTools:developfrom
cairocoder01:tags-as-static-options

Conversation

@cairocoder01
Copy link
Copy Markdown
Collaborator

Better supports usage in magic links so as to not open up the API to request available tags

Better supports usage in magic links so as to not open up the API to request available tags
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

Code Review - PR 2907 (dt-components render_tags static_options)

HIGH: Silent failure in magic link context (dt-core/utilities/dt-components.php, new static_options block). DT_Posts::get_multi_select_options() begins with a can_access() check at dt-posts/posts.php:1587, which calls current_user_can('access_' . post_type). Magic link visitors typically lack this capability, causing the function to return a WP_Error. The is_array(raw_options) guard handles the error safely, but the result is that options stays null, no options attribute is rendered, and the dt-tags component silently falls back to dynamic API-fetching. The feature does nothing in the exact context the PR is designed for. Fix: either fetch options without requiring can_access (e.g. a direct query), or document that static_options only works for authenticated users.

MEDIUM 1: Dead branch. get_multi_select_options() uses wpdb->get_col() (dt-posts/posts.php:1593, 1605) which always returns a flat array of strings. The elseif (is_array(option) and isset(option['label'])) branch at dt-core/utilities/dt-components.php is unreachable.

MEDIUM 2: Options silently truncated at 20. The call uses defaults search='' and limit=20 (dt-posts/posts.php:1586). A tags field with more than 20 distinct values will produce an incomplete suggestions list with no indication to the user.

Summary: Not ready to merge. The core feature silently fails for the primary use case (magic links) due to the permission check inside get_multi_select_options().

@cairocoder01
Copy link
Copy Markdown
Collaborator Author

Code Review - PR 2907 (dt-components render_tags static_options)

HIGH: Silent failure in magic link context (dt-core/utilities/dt-components.php, new static_options block). DT_Posts::get_multi_select_options() begins with a can_access() check at dt-posts/posts.php:1587, which calls current_user_can('access_' . post_type). Magic link visitors typically lack this capability, causing the function to return a WP_Error. The is_array(raw_options) guard handles the error safely, but the result is that options stays null, no options attribute is rendered, and the dt-tags component silently falls back to dynamic API-fetching. The feature does nothing in the exact context the PR is designed for. Fix: either fetch options without requiring can_access (e.g. a direct query), or document that static_options only works for authenticated users.

MEDIUM 1: Dead branch. get_multi_select_options() uses wpdb->get_col() (dt-posts/posts.php:1593, 1605) which always returns a flat array of strings. The elseif (is_array(option) and isset(option['label'])) branch at dt-core/utilities/dt-components.php is unreachable.

MEDIUM 2: Options silently truncated at 20. The call uses defaults search='' and limit=20 (dt-posts/posts.php:1586). A tags field with more than 20 distinct values will produce an incomplete suggestions list with no indication to the user.

Summary: Not ready to merge. The core feature silently fails for the primary use case (magic links) due to the permission check inside get_multi_select_options().

This is decent thinking, but with actually testing it in a browser, it does work. From the home screen app, ML users are given the proper permissions.

@cairocoder01
Copy link
Copy Markdown
Collaborator Author

@corsacca This should be ready for your review. I think the test failures are from other recent phpunit updates.

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.

1 participant