Skip to content

fix(DotPropertiesService): handle boolean values returned by config endpoint for feature flags#35332

Open
rjvelazco wants to merge 3 commits intomainfrom
issue-35331-style-editor-tab-hidden-in-content-type-builder-due-to-unhandled-boolean-value-from-config-endpoint
Open

fix(DotPropertiesService): handle boolean values returned by config endpoint for feature flags#35332
rjvelazco wants to merge 3 commits intomainfrom
issue-35331-style-editor-tab-hidden-in-content-type-builder-due-to-unhandled-boolean-value-from-config-endpoint

Conversation

@rjvelazco
Copy link
Copy Markdown
Member

@rjvelazco rjvelazco commented Apr 15, 2026

Summary

  • Fixes DotPropertiesService.getFeatureFlag() to correctly handle native JSON boolean values (true/false) returned by /api/v1/configuration/config for FEATURE_FLAG_* keys — previously only string "true" was handled, causing feature flags to silently evaluate as false and hiding the Style Editor tab in the Content Type Builder
  • Updates getKey() return type from Observable<string> to Observable<string | boolean> to match the actual API contract
  • Fixes downstream TypeScript narrowing errors in DotWysiwygPluginService and DotContainersService caused by the widened return type

Test plan

  • yarn nx test data-access passes including the two new boolean true/false unit test cases
  • Open the Content Type Builder and verify the Style Editor form tab is visible when FEATURE_FLAG_UVE_STYLE_EDITOR_FOR_TRADITIONAL_PAGES is enabled on the backend
  • Verify feature flags that return string "true" / "false" continue to work correctly (backward compatibility)

Fixes #35331

Video

video.mov

🤖 Generated with Claude Code

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 15, 2026

Claude finished @rjvelazco's task in 1m 16s —— View job


PR Review

  • Read changed files
  • Analyze changes
  • Post review

Fix is correct and targeted. A few things worth a second look:

1. Missing test for string "false"false in getFeatureFlag

The spec covers string "true", boolean true, boolean false, and NOT_FOUND, but there's no test for a string "false" returning false. The logic handles it (value === 'true' returns false for other strings), but since "true" and "false" are the legacy string pair, they're worth testing symmetrically. Minor gap.

2. Inconsistency between getFeatureFlag and getFeatureFlags for FEATURE_FLAG_NOT_FOUND (pre-existing)

getFeatureFlag converts NOT_FOUNDtrue (fail-open). getFeatureFlags passes NOT_FOUND through as the raw string. Callers of getFeatureFlags must handle this themselves or silently get a string where they expect a boolean. This predates the PR, but widening the type makes it slightly more visible. Not a blocker, but worth documenting or aligning if there's a follow-up.

3. getFeatureFlag fail-open semantics are preserved

dot-properties.service.ts:88 — when a key is missing from the backend, the flag defaults to true (enabled). Preserving this is intentional per the original design, but it means any newly introduced FEATURE_FLAG_* key that isn't yet deployed on the backend will be silently enabled. Worth confirming this is the desired policy for all flags.

4. DotContainersService guard looks fine

dot-containers.service.ts:62-64 — the typeof title !== 'string' early-return guard is correct. If DEFAULT_CONTAINER ever returns a non-string (e.g., a boolean), returning null (no default container) is the safe fallback.

5. DotWysiwygPluginService filter is correct

The type guard (value): value is string => typeof value === 'string' && !!value at line 38 is the right pattern.


Overall: clean fix, no design problems, no security issues. The two items above (missing string-false test, getFeatureFlags FEATURE_FLAG_NOT_FOUND inconsistency) are minor and the second is pre-existing. Approve if you're comfortable with the fail-open semantics being policy.

@rjvelazco rjvelazco changed the title fix: catch boolean values return by the FF endpoint fix(DotPropertiesService): handle boolean values returned by config endpoint for feature flags Apr 15, 2026
@rjvelazco rjvelazco enabled auto-merge April 15, 2026 21:03
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.

Style Editor tab hidden in Content Type Builder due to unhandled boolean value from config endpoint

3 participants