[Bug]: Custom ICS Form Builder Field Type Table - column type - select - form wont save

[rjonesbsink]

Which version are you running?

4.2.25

How is it installed?

Git clone on Windows (XAMPP or similar)

What page or screen were you on?

New ICS Form Builder

What went wrong?

When creating a custom ics form, using the field type as table and setting the column type as select, the box to list selection does not show up untill you add another column and when you add the items in the box for selection and try to save the form, it does not save nor does any message appear for failure or success.

What steps lead to it?

No response

Screenshot (optional)

What device / browser?

Windows, Mac, Safari, Chrome, Edge

[rjonesbsink]

Dug into this on v4.2.25. Both halves of what I have seen may come from a single asymmetry in assets/js/ics-form-type-admin.js: a table column's options are stored as a raw string, while a plain select field's are stored as an array.

handleFieldsListChange() has two separate branches, and only one of them splits on newlines.

// :443-446  table column -- stores whatever the element holds
if (colProp !== null && colIdx !== null) {
    colIdx = parseInt(colIdx, 10);
    if (!Array.isArray(f.columns) || !f.columns[colIdx]) return;
    f.columns[colIdx][colProp] = readInputValue(el);   // textarea -> STRING
    return;
}
// :448-451  plain select field -- splits correctly
if (prop === 'options') {
    f.options = (el.value || '').split('\n');          // -> ARRAY
    return;
}

readInputValue() returns el.value for a textarea, so the column ends up holding "Alpha\nBravo\nCharlie" rather than ["Alpha","Bravo","Charlie"]. Everything downstream expects the array.

Why the save dies silently

cleanFieldsForSave() calls .filter() on it (:328):

if (co.options) co.options = co.options.filter(function (o) { return String(o).trim() !== ''; });

Strings have no .filter, so this throws a TypeError. The critical detail is where it throws — inside the payload literal at :298, which is evaluated before postJson() is ever called:

fields: cleanFieldsForSave(editorFields)

So the throw escapes saveType() entirely. The .catch() at :315 is attached to a promise that never gets created, there is no try/catch anywhere in the file (0 hits), and no window.onerror on the page. The click handler just dies. No request, no toast, no error banner — exactly the "no message for failure or success" you saw. The only trace is an uncaught error in the browser console.

Ran the shipped cleanFieldsForSave verbatim under node against both shapes:

AFTER the change handler runs:
  table col .options          string           "Alpha\nBravo\nCharlie"
  plain select .options       object (array)   ["Alpha","Bravo","Charlie"]

cleanFieldsForSave([plain select field]):
  OK -> [{"key":"s1",...,"options":["Alpha","Bravo","Charlie"]}]

cleanFieldsForSave([table field w/ select column]):
  THREW -> TypeError: co.options.filter is not a function

Why the box only appears after you add another column

Separate cause, same area. The options textarea is only emitted when the column is already a select (:582), but the branch at :445 mutates the model and returns without re-rendering. renderFieldsEditor() is called from six places — open editor, add/remove/move field, add/remove column — and never from the change handler.

So switching the type to Select updates the data and nothing else. "Add Column" is simply the first thing you click afterwards that happens to trigger a re-render, which is when the box finally shows up. Any of the other structural actions would do the same.

A third symptom you may not have hit yet

Because the stored value is a string, the re-render at :583 cannot display it back:

var colOpts = Array.isArray(col.options) ? col.options.join('\n') : '';

Array.isArray(col.options) ? join : ''  ->  ""

So if you type your options and then click "Add Column" again, the list you just typed is silently wiped. Same root cause, and worth fixing in the same pass.

Possible direction

Entirely your call @ejosterberg, but the pieces look separable:

1. The string/array split at :445. Special-casing colProp === 'options' to .split('\n') the way :449 already does would fix the save and the wipe together.
2. The missing re-render. Re-rendering when colProp === 'type' changes would make the box appear when the type is selected rather than on the next unrelated click.
3. The silence. This is the part I would weigh separately from the bug itself. A try/catch around the payload build, or moving cleanFieldsForSave() to its own statement above the literal, would turn any future failure in there into a visible message. The underlying defect is a one-line type mismatch, but what made it hard to diagnose is that a throw between the click and the fetch produces no feedback at all — the same shape as the disabled-dropdown hazard in [Bug]: Per-unit Dest dropdown never saves — ReferenceError: ticketId is not defined, so the destination appears to revert to the call's receiving facility #98.

Also worth noting: col.options is never initialised when a column is switched to select (compare :381, where a new top-level select field gets ['Option 1','Option 2']). That path is benign today — an untouched column reaches the server as options: undefined and ics_form_type_validate_options() correctly returns "must define at least one option" — so that case does produce a proper error message. It is only when you actually type a list that it goes quiet.

If I'm reading this right, if not please direct in the right path

[ejosterberg]

Thanks for the exceptionally thorough diagnosis — verified every line reference against the live source before touching anything, and it was all exactly right.

Fixed all three pieces, per your proposed direction:

1. handleFieldsListChange()'s table-column branch now special-cases colProp === 'options' with .split('\n'), matching the plain-select field's own branch, instead of falling through to the raw-string readInputValue() path.
2. Changing a column's type now triggers renderFieldsEditor() immediately, so the options textarea appears as soon as you pick "Select" rather than waiting on the next unrelated action.
3. cleanFieldsForSave(editorFields) is no longer inlined directly in the payload literal — it's pulled into its own statement wrapped in try/catch, so any future throw in there surfaces via showSaveError() instead of a silent dead click. Left cleanFieldsForSave() itself untouched, per your note — the fix is at the source of the bad data, not a defensive patch downstream of it.

New regression coverage in tests/test_gh107_ics_form_type_table_select.php (7 assertions). Verified against a reverted copy of the fix that 6 of the 7 correctly fail without it, so this isn't a test that would pass regardless of the code underneath.

Fixed in 4f1619a, deployed to both training.ticketscad.com and bloomington-auxcomm.

[rjonesbsink]

Verified — closing.

Verified on v4.2.26, merged and migrated on this install (212 migrations applied, 0 failed; idempotent re-run reports Pending: 0, schema OK at 190 tables / 1228 columns). Windows 11 / IIS 10 / PHP 8.4.22 / MySQL 8.0.46.

All three pieces confirmed.

if (colProp === 'options') {

The table-column branch splits on newlines like the plain-select branch does, the type change re-renders immediately so the options box appears when you pick Select, and cleanFieldsForSave() is out of the payload literal and inside a try/catch. tests/test_gh107_ics_form_type_table_select.php: 7 passed, 0 failed.

Leaving cleanFieldsForSave() itself untouched was the right reading — the bad data was created at the input handler, and defending downstream would have hidden that rather than fixed it. The try/catch matters independently: a throw between the click and the fetch produced no console error, no toast and no request, which is what made a one-line type mismatch look like a mystery.