replace pipe-value sniffing with explicit list field options

[MattBudz]

Summary

replace pipe-value sniffing with explicit list field options, keeping the pipe-value check as a fallback

Testing steps

1. Login and open a project, then open any Node
2. Add a new Note
3. In the Note editor, click "Fields" to switch to the Fields tab
4. Click "Add field", enter a field name and a value, then switch to the "Source" tab
5. Assert the source text shows #[Name]# followed by the value you typed
6. Switch back to the "Fields" tab
7. Assert the field shows as a plain text value field (not a dropdown)
8. Save the Note
9. Assert the Note saves successfully and the field/value are shown correctly
10. Edit the Note again and confirm the Fields tab still renders correctly (add/remove a field, save again)
11. Assert no errors occur and the Note updates correctly
12. Go to Admin > Templates > Notes and create a new Note Template whose content includes a field with a pipe-separated value, e.g. #[OS]#\nLinux | OSX | Windows
13. Create a new Note from that template (?template=<name>)
14. Assert the OS field renders as a <select> with Linux/OSX/Windows as options
15. Save the Note, then edit it again
16. Assert the OS field now renders as a plain textarea (unchanged from current behavior, since the value collapsed to a single string after save)

Other Information

This adds a generic, explicit field_options lookup (unused by CE itself, since CE has no List-field concept) as the primary mechanism, and keeps the original pipe-separated-value heuristic as a fallback when no explicit options are set for a field. The fallback still only applies to new records, exactly as before.

The fallback is required because Note Templates (config/templates/notes/*.txt, managed under Admin > Templates > Notes) are a separate, RTP-independent CE/Pro feature that relies on hand-authored pipe-separated values to get a dropdown when creating a new record from a template. An earlier version of this PR removed the pipe-value check entirely, which broke this (caught by CI: spec/features/issues_spec.rb "preloads the editor with the template").

Confirmed via automated specs and manual browser testing that both paths (Note Templates and the new explicit lookup) work as expected.

Check List

• Added a CHANGELOG entry (no user-visible CE change)
• Commit message has a detailed description of what changed and why.

[etdsoft]

Automated peer review -- PR #1660

Reviewers: Claude (sub-agent 1), Codex (sub-agent 2)
Verdict: request-changes
Findings: 1 High, 0 Medium, 2 Low

What I checked

This is an automated peer code review by two independent LLM sub-agents reading the same rubric. Findings flagged by both agents are higher confidence; findings flagged by only one are still worth your attention. The verdict is computed from severity counts -- there is no editorial judgment in the merge step.

Both reviewers agreed

(no shared findings)

Claude only

F1 [Low|consistency] app/helpers/fields_helper.rb:12 -- helper methods not alphabetized within sections

What: In the public section, render_dropdown? (line 2) is followed by dropdown_options (line 12). In the private section, list_field_options (line 18) is followed by has_liquid_filters? (line 22). Both sections are reverse-alphabetical.

Why: .claude/CLAUDE.md → "Code style" → "Alphabetical order within sections" says method definitions must be sorted alphabetically within their logical section. The pre-existing has_liquid_filters? was previously the only private method, so ordering wasn't visible; this PR introduces dropdown_options and list_field_options and both land out of order.

How to fix: Swap the two public methods so dropdown_options is defined before render_dropdown?, and swap the two private methods so has_liquid_filters? is defined before list_field_options.

---

F2 [Low|tests] spec/helpers/fields_helper_spec.rb:21 -- spec passes for the wrong reason

What: The example "returns false for a field with no matching options" sets @field_options = { 'Risk' => ... } and asserts render_dropdown?('Status', 'Open') is falsey. Since @field_options['Status'] is nil, execution falls to the else-branch, which returns false only because @allow_dropdown was never assigned (nil). The assertion is not testing the "field_options entry missing" branch independently.

Why: If a future change defaulted @allow_dropdown to truthy, this test would silently start passing for a different reason (or failing without clearly indicating which branch broke).

How to fix: Either (a) add assign(:allow_dropdown, true) so the assertion is clearly depending on the field_options lookup (not the unset @allow_dropdown), or (b) rename the example to "falls back to the pipe-value check when the field has no explicit options" and add assign(:allow_dropdown, true) + a pipe-separated value to make the fallback path explicitly exercised.

---

Codex only

F3 [High|correctness] app/views/fields/_field.html.erb:24 -- blank list values silently submit as the first option

What: When render_dropdown? returns true for a blank explicit list-field value (CE's new path: value.blank? → true), the select renders dropdown_options(field_name, value) with { selected: value } but no blank option. A browser submits the first <option> when no option matches the selected value.

Why: For a new record with an explicit list field (e.g. Risk = High/Medium/Low), opening the Fields tab and saving without touching the Risk field submits "High" -- silently writing a non-empty value into a field the user never touched. This is data-loss behavior: user intent (leave blank) is overwritten by browser default.

How to fix: Add include_blank: value.blank? to the options hash so a blank option is rendered (and pre-selected) when the current value is empty:

<%= select :item_form, "field_value_#{index}",
      dropdown_options(field_name, value),
      { selected: value, include_blank: value.blank? },
      { class: 'form-select', data: { behavior: 'preview-enabled' }, tabindex: index + 1 } %>

Add a helper/view spec that renders a blank explicit list field and asserts the submitted value is blank (not the first option) when the user does not interact.

---

Notes

• CE never populates data-field-options on any textarea today (grep confirms), so _fieldOptions is undefined client-side and omitted from the POST -- the new High finding only manifests when Pro wires real RTP List field options. Still worth fixing here before the Pro follow-up ships.
• select ... { selected: value } for the legacy pipe path passes the full pipe string as the selected value, which matches none of the split options; the browser falls back to the first <option>, matching previous behavior. Not a new regression.

---

Reviewed automatically. Raw outputs in the shared product reviews directory.