Skip to content

feat: implement Config Requires field for dependent option disable#4669

Open
pliablepixels wants to merge 3 commits intoZoneMinder:masterfrom
pliablepixels:fix/options-requires-dependency
Open

feat: implement Config Requires field for dependent option disable#4669
pliablepixels wants to merge 3 commits intoZoneMinder:masterfrom
pliablepixels:fix/options-requires-dependency

Conversation

@pliablepixels
Copy link
Member

Summary

  • Wire up the long-dormant Config.Requires column (added in v0.9.13) so dependent options are disabled when their parent toggle is off
  • PHP evaluates Requires on page render to set initial disabled state
  • Inline JS (with CSP nonce) re-evaluates on any input change for live toggling without page reload
  • Supports compound semicolon-separated AND conditions (e.g. ZM_OPT_USE_AUTH=1;ZM_AUTH_RELAY=hashed)
  • Affects ~50 existing config entries across auth, images, logging, mail, upload, web, x10, and system categories

Fixes #4668

Test plan

  • Go to Options → Config tab, uncheck OPT_TRAINING → dependent fields disable immediately
  • Options → Auth tab, uncheck OPT_USE_AUTH → AUTH_TYPE, AUTH_RELAY, AUTH_HASH_* all disable
  • Options → Auth tab, check OPT_USE_AUTH but change AUTH_RELAY away from "hashed" → compound deps (AUTH_HASH_IPS, AUTH_HASH_SECRET, etc.) disable
  • Options → Images tab, uncheck OPT_FFMPEG → all FFMPEG fields disable
  • Options → Upload tab, uncheck OPT_UPLOAD → all UPLOAD fields disable
  • Save with parent off, reload page → dependent fields still disabled on fresh render
  • Verify no JS console errors, CSP not blocking the script

🤖 Generated with Claude Code

The Config table has had a Requires column since v0.9.13 but it was
never evaluated in the Options UI. This wires it up:

- PHP: on page render, parse Requires (single or compound semicolon-
  separated conditions like "ZM_OPT_USE_AUTH=1;ZM_AUTH_RELAY=hashed")
  and disable fields whose dependencies are not met
- JS: on any input change within the options form, re-evaluate all
  data-requires conditions and toggle disabled state live, so
  checking/unchecking a parent toggle immediately enables/disables
  dependent fields without a page reload

Affects ~50 existing config entries across auth, images, logging,
mail, upload, web, x10, and system categories.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 1, 2026 17:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR wires up the existing Config.Requires metadata so the Options UI can disable dependent configuration fields when prerequisite options aren’t set, with both initial server-side (PHP) evaluation and client-side (JS) live updates.

Changes:

  • Evaluate Config.Requires on render to set the initial disabled state of dependent option inputs.
  • Add data-requires attributes to option rows to expose dependency rules to the browser.
  • Add inline JS (with CSP nonce) to re-evaluate dependencies on form changes and toggle disabled state live.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

for (var i = 0; i < depRows.length; i++) {
var met = evalRequires(depRows[i].getAttribute('data-requires'));
var inputs = depRows[i].querySelectorAll('input, select, textarea');
for (var j = 0; j < inputs.length; j++) inputs[j].disabled = !met;
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

Original Copilot comment: Many option inputs are select elements enhanced by Chosen. Toggling select.disabled alone won't reliably update the Chosen UI; the widget needs a chosen:updated trigger after enabling/disabling.

Response: Fixed in 5199a22. Added chosen:updated trigger for Chosen-enhanced selects after toggling disabled state.

Copilot uses AI. Check for mistakes.
Comment on lines +362 to +363
var m = parts[i].trim().match(/^(ZM_\w+)=(.+)$/);
if (m && getVal(m[1]) !== m[2]) return false;
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

Original Copilot comment: evalRequires() currently fails open: if a requires fragment doesn't match the expected ZM_KEY=value pattern, it is silently ignored and the dependency is treated as satisfied. It would be safer to treat unparseable fragments as unmet.

Response: Fixed in 5199a22. Unparseable fragments now return false (fail closed) instead of being silently ignored. Empty fragments from trailing semicolons are skipped.

Copilot uses AI. Check for mistakes.
}
?>
<div class="form-group form-row <?php echo $name ?>">
<div class="form-group form-row <?php echo $name ?>"<?php if (!empty($value['Requires'])) echo ' data-requires="'.htmlspecialchars($value['Requires']).'"' ?>>
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

Original Copilot comment: data-requires is rendered using htmlspecialchars() without ENT_QUOTES. Should use ENT_QUOTES to avoid attribute-breaking/XSS edge cases if the DB value ever contains quotes.

Response: Fixed in 5199a22. Added ENT_QUOTES to htmlspecialchars.

Copilot uses AI. Check for mistakes.
Comment on lines +369 to +374
function refresh() {
for (var i = 0; i < depRows.length; i++) {
var met = evalRequires(depRows[i].getAttribute('data-requires'));
var inputs = depRows[i].querySelectorAll('input, select, textarea');
for (var j = 0; j < inputs.length; j++) inputs[j].disabled = !met;
}
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

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

Original Copilot comment: In refresh(), the JS sets disabled = !met for every input in a [data-requires] row. This can re-enable fields that were intentionally disabled for other reasons (e.g., System options or lack of edit permission).

Response: Fixed in 5199a22. The data-requires attribute is now only emitted on rows where $canEdit && !$value['System'] — so System-locked or permission-disabled fields never get a data-requires attribute and the JS refresh() can never re-enable them.

Copilot uses AI. Check for mistakes.
- Fail closed on unparseable Requires fragments instead of ignoring
- Trigger chosen:updated on Chosen-enhanced selects after toggling
- Use ENT_QUOTES in htmlspecialchars for data-requires attribute
- Only emit data-requires on rows that are otherwise editable (not
  System, user has permission) to prevent re-enabling locked fields

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@connortechnology
Copy link
Member

We should tell Claude to always use modern js, with const/let instead of var. Also, things like getVal function in here, we have jquery... $j(thing).val(). Let's have claude clean this up a bit first.

Replace var with const, vanilla DOM with $j() jQuery calls,
and C-style loops with for...of and .each() per review feedback.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

Options UI: implement Config.Requires field for dependent option disable

3 participants