Skip to content

fix(edit-content): parse single-option label|value fields correctly#36319

Merged
oidacra merged 1 commit into
mainfrom
issue-36157-single-option-checkbox-radio-select-multiselect-sh
Jun 25, 2026
Merged

fix(edit-content): parse single-option label|value fields correctly#36319
oidacra merged 1 commit into
mainfrom
issue-36157-single-option-checkbox-radio-select-multiselect-sh

Conversation

@oidacra

@oidacra oidacra commented Jun 25, 2026

Copy link
Copy Markdown
Member

Summary

Single-option Checkbox, Radio, Select, and Multiselect fields in the new Edit Contentlet displayed the raw label|value string (e.g. Yes|yes) instead of just the label (Yes). The value after the | is what should be stored; only the label should be shown.

CleanShot 2026-06-25 at 13 04 01@2x

Root cause: getSingleSelectableFieldOptions used a global isPipeFormat flag that only treated input as pipe format when it contained a line break or started with |. A lone Yes|yes (no line break, not starting with |) fell through to the comma branch, where label and value were both set to the raw Yes|yes.

Fix: Switched to per-item pipe detection (item.includes('|')) so each option is parsed independently. This fixes the single-option case, also corrects comma-separated pipe options (label1|value1,label2|value2), and preserves multi-line pipe format, the |true checkbox-without-label special case, plain comma format, and data-type casting. All four field types delegate to this shared util, so one change covers them all — no component changes needed.

Closes #36157

Acceptance Criteria

  • A single-option Checkbox field displays only the label (Yes|yes -> Yes)
  • A single-option Radio field displays only the label
  • A single-option Select field displays only the label
  • A single-option Multiselect field displays only the label
  • The stored/submitted value remains the part after the | (yes)
  • Multi-option fields continue to display labels correctly (no regression)
  • An option with no | (Yes) uses the whole string as both label and value

Test Plan

Unit tests added/updated in functions.util.spec.ts:

  • New Single-option pipe format (issue #36157) block: 'Yes|yes' -> {label:'Yes', value:'yes'} (AC8); 'Yes' -> {label:'Yes', value:'Yes'} (AC9); parameterized it.each across Checkbox/Radio/Select/Multiselect (AC10)
  • Updated the previously-broken "mixed formats" test so 'label1|value1,label2|value2' now asserts correctly split options

Verification: pnpm nx lint edit-content (0 errors) and pnpm nx test edit-content --testPathPattern=functions.util (1962 passed, 1 pre-existing skip).

Changed Files

  • core-web/libs/edit-content/src/lib/utils/functions.util.ts
  • core-web/libs/edit-content/src/lib/utils/functions.util.spec.ts

This PR fixes: #36157

@claude

claude Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Claude finished @oidacra's task in 50s —— View job


Rollback Safety Analysis — Complete

  • Gather context from PR metadata
  • Read rollback-unsafe categories reference
  • Get full PR diff
  • Analyze diff against all unsafe categories
  • Apply appropriate label

Result: ✅ Safe To Rollback

The diff touches only two frontend TypeScript files:

  • core-web/libs/edit-content/src/lib/utils/functions.util.ts — pure UI logic refactor (per-item pipe detection)
  • core-web/libs/edit-content/src/lib/utils/functions.util.spec.ts — unit tests only

Checklist against all unsafe categories:

Category Match?
C-1 Structural Data Model Change ❌ No DB changes
C-2 Elasticsearch Mapping Change ❌ No ES changes
C-3 Content JSON Model Version Bump ❌ No model version changes
C-4 DROP TABLE / DROP COLUMN ❌ No schema changes
H-1 One-Way Data Migration ❌ No data migration
H-2 RENAME TABLE / COLUMN ❌ No renames
H-3 PK Restructuring ❌ No constraint changes
H-4 New Content Type Field Type ❌ No new field types
H-5 Binary Storage Provider Change ❌ No storage changes
H-6 DROP PROCEDURE / FUNCTION ❌ No procedure changes
H-7 NOT NULL Column Without Default ❌ No DB changes
H-8 VTL Viewtool Contract Change ❌ No viewtool changes
M-1 Column Type Change ❌ No DB changes
M-2 Push Publishing Bundle Format ❌ No bundle changes
M-3 REST/GraphQL API Contract ❌ No API changes
M-4 OSGi Plugin API Breakage ❌ No Java interface changes

This is a pure frontend bug fix — rolling back to the previous release leaves no persistent artifacts (no DB migrations, no ES reindexing, no data transformations). The label "AI: Safe To Rollback" has been applied.

@github-actions github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Jun 25, 2026
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🟠 High: core-web/libs/edit-content/src/lib/utils/functions.util.ts:104options.trim().startsWith('|') incorrectly treats |true as a single-item pipe format, but this logic bypasses comma-splitting entirely and assumes no other pipe formats exist. This breaks expected behavior for inputs like |true,option2|value2 — the second item is lost. Pipe detection must be per-item, not global.
  • 🟠 High: core-web/libs/edit-content/src/lib/utils/functions.util.ts:135item.split('|') can produce [''] for input like |, which sets label = '' and value = ''. This is valid per spec, but no test covers this edge case. However, the real issue: if item is "|", parts[0] is "", parts[1] is undefined, so value = "" — but this is silently accepted. No validation or warning for malformed pipe syntax. This could cause silent data loss in UIs expecting non-empty values.
  • 🟡 Medium: core-web/libs/edit-content/src/lib/utils/functions.util.ts:102hasPipes is removed but logic still relies on per-item item.includes('|'). This is correct, but the removal of hasPipes makes it harder to audit whether global pipe detection was fully deprecated. Assumption: hasPipes was redundant, but what if a future caller expects it for pre-validation? What to verify: Is hasPipes used elsewhere in the codebase? If yes, this change breaks contract.

Existing

  • 🟡 Medium: core-web/libs/edit-content/src/lib/utils/functions.util.ts:135 — Prior issue: no validation on empty value after pipe split. Still present. No guard against value === '' being saved as valid option. This has always been a risk — not introduced by this PR, but not fixed.

Resolved

  • core-web/libs/edit-content/src/lib/utils/functions.util.ts:132 — Previously, isPipeFormat was a global flag that incorrectly disabled comma-splitting when any pipe existed. Now, per-item item.includes('|') correctly handles mixed formats. This fixes the prior bug where label1|value1,label2 would be treated as one item with literal pipe.

Run: #28186387738 · tokens: in: 2430 · out: 709 · total: 3139

@oidacra oidacra marked this pull request as ready for review June 25, 2026 17:04
@oidacra oidacra enabled auto-merge June 25, 2026 17:06
Single-option Checkbox/Radio/Select/Multiselect fields rendered the raw
"label|value" string (e.g. "Yes|yes") instead of just the label. The
parser used a global isPipeFormat flag that only treated input as pipe
format when it contained a line break or started with "|", so a lone
"Yes|yes" fell through to the comma branch and was used verbatim as both
label and value.

Switch getSingleSelectableFieldOptions to per-item pipe detection so each
option is split independently. This fixes the single-option case, also
corrects comma-separated pipe options, and preserves multi-line, the
"|true" checkbox special case, plain comma, and data-type casting.

Closes #36157
@oidacra oidacra force-pushed the issue-36157-single-option-checkbox-radio-select-multiselect-sh branch from fe6f8b4 to 25dbd57 Compare June 25, 2026 17:23
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — qwen.qwen3-next-80b-a3b

New Issues

  • 🟠 High: core-web/libs/edit-content/src/lib/utils/functions.util.ts:132item.includes('|') now triggers pipe parsing on any item with a pipe, even in comma-separated mode, but the prior logic assumed pipe format only when hasPipes was true globally. This breaks backward compatibility for inputs like label1,value2|value2 — now incorrectly parses as {label: "value2", value: "value2"} instead of preserving value2|value2 as a single unsplit option. This is a logic error introduced by removing hasPipes flag and applying pipe-split per-item without context.

Existing

  • 🟠 High: core-web/libs/edit-content/src/lib/utils/functions.util.ts:104 — Bypasses comma-splitting for inputs like '|true,option2|value2', causing loss of second item — still present; the new per-item pipe parsing does not resolve this, and |true,option2|value2 is now split into ["|true", "option2|value2"] → second item becomes {label: "option2", value: "value2"} but first item |true becomes {label: "", value: ""} — still loses semantic meaning.
  • 🟠 High: core-web/libs/edit-content/src/lib/utils/functions.util.ts:135 — Silently accepts malformed pipe input like '|' producing empty label/value with no validation — still present; no validation added for empty label or value after split.
  • 🟡 Medium: core-web/libs/edit-content/src/lib/utils/functions.util.ts:102 — Removed hasPipes flag without verifying if it's used elsewhere — potential contract breakage — still present; no audit of callers or downstream consumers of the old behavior.
  • 🟡 Medium: core-web/libs/edit-content/src/lib/utils/functions.util.ts:135 — No validation on empty value after pipe split — pre-existing risk not addressed — still present; item.split('|') with [''] yields {label: "", value: ""} with no guard.

Resolved

  • core-web/libs/edit-content/src/lib/utils/functions.util.ts:104 — The test case now correctly expects label|value parsing per item, so the prior “bypasses comma-splitting” concern is obsolete — but the underlying logic flaw remains, so this is marked resolved only for the test alignment, not the bug.

Run: #28188196894 · tokens: in: 2789 · out: 783 · total: 3572

@oidacra oidacra added this pull request to the merge queue Jun 25, 2026
@mergify

mergify Bot commented Jun 25, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Merged via the queue into main with commit 8866f80 Jun 25, 2026
40 checks passed
@oidacra oidacra deleted the issue-36157-single-option-checkbox-radio-select-multiselect-sh branch June 25, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Single-option Checkbox/Radio/Select/Multiselect shows raw "label|value" instead of the label

3 participants