Queue edits reset every setting the caller didn't name

[OlivierRunwolf]

The bug

fm_update_queue rebuilds a queue with queues_del + queues_add. queues_add() takes its config from two places: ~30 keys it reads out of $_REQUEST, and ~15 positional args. AddQueue::applyRequest() hardcoded the first group and AddQueue::writeQueue() the second — so any setting the tool has no parameter for was written back as a default on every edit.

Renaming a queue also reset announce frequencies, service level, member delay, autofill, ring-in-use, agent/join announcements, call confirm, monitoring and auto-pause.

fm_add_queue_member and fm_remove_queue_member call the same writer, so adding an agent to a queue did it too — that one is easy to hit in normal use.

Minimal reproduction on a stock queue:

# announce-frequency is 25
fwconsole frogman:tool fm_update_queue '{"account":"9099","name":"Renamed","confirm":true}'
# announce-frequency is now 0

The fix

applyRequest() and writeQueue() take an optional $current (the queues_get() snapshot, which already returns all ~65 keys) and resolve each field as explicit param > current value > the original literal. fm_update_queue and the two member tools pass it; fm_add_queue doesn't, so with no snapshot every field collapses to the literal and creation is unchanged.

UpdateQueue::buildMerged() and the member-tool helper now list only the queues_add positional args plus what the caller actually passed. Re-listing the rest was itself a clobber — it cast a retry of "none" to 0, and pinned an inheriting MoH class to "default".

Three FreePBX behaviours are documented in comments rather than worked around:

• ringinuse is derived from the cwignore positional (2 or 3 → no), so carrying cwignore through is what preserves it. There's no way to set it independently through this path.
• The queue-youarenext / thereare / callswaiting / thankyou prompts are derived from announceposition.
• autofill is stored via (!empty($_REQUEST['autofill'])) ? 'yes' : 'no', so the string 'no' is truthy there and has to be blanked to mean no.

The $_REQUEST key map is written out in full because the request names and the stored keyword names differ (announcefreq → announce-frequency, pannouncefreq → periodic-announce-frequency), which is what made the drops easy to miss in the first place.

No new tools, no routing changes, no version bump. Still BMO/queues_add-only — no new DB writes.

Testing

FreePBX 17.0.30 / Asterisk 18, real box. 23 assertions, all passing.

Create path is unchanged — created a queue with the old code, dumped queues_details + queues_config, then created the identical queue after the change and diffed. Both tables byte-identical, 39 keywords either way.

The bug is fixed — seeded 10 non-default values in fields fm_update_queue has no parameter for, then renamed the queue only:

fwconsole frogman:tool fm_add_queue '{"account":"9099","name":"ClobberTest","strategy":"linear","timeout":20,"retry":6,"maxwait":300,"members":["110"],"confirm":true}'
# seed announce-frequency=25, servicelevel=75, reportholdtime=yes, autofill=no,
# periodic-announce-frequency=45, penaltymemberslimit=3, timeoutrestart=yes,
# memberdelay=30, min-announce-frequency=90, retry=none
fwconsole frogman:tool fm_update_queue '{"account":"9099","name":"ClobberTest-RENAMED","confirm":true}'

39 keyword rows before, 39 after, full queues_details diff empty. retry stayed none rather than becoming 0.

Named fields still change — fwconsole frogman:tool fm_update_queue '{"account":"9099","strategy":"rrmemory","joinempty":"no","servicelevel":"120","confirm":true}' applied all three, and announce-frequency stayed at 25.

Member tools — fm_add_queue_member then fm_remove_queue_member; settings diff empty both times, membership changed correctly.

Two caveats on coverage, so the numbers aren't over-read:

• The preserveOnly branch (eventwhencalled, eventmemberstatus, announcemenu, callback) didn't execute — a freshly created queue has those at values its guard skips. Worst case they behave as they did before this change.
• ringinuse preservation follows from cwignore now being passed through, but no test exercised it directly; seeding ringinuse=no with cwignore=0 would be a state the GUI can't produce.

Happy to add tests or split this differently if you'd prefer. Per CONTRIBUTING: I used Claude while writing this, and kept it out of the commit trailers.