Skip to content

bug: fix secret mask calculation#193

Merged
gkrajniak merged 1 commit into
mainfrom
bug/fix-null-undefined-values-in-secrets-mask-calulcation
Jul 6, 2026
Merged

bug: fix secret mask calculation#193
gkrajniak merged 1 commit into
mainfrom
bug/fix-null-undefined-values-in-secrets-mask-calulcation

Conversation

@gkrajniak

@gkrajniak gkrajniak commented Jul 6, 2026

Copy link
Copy Markdown
Member

secret value cell crashes the whole table when the configured data key is missing

Summary by CodeRabbit

  • Bug Fixes
    • Improved secret-value display handling so missing values no longer cause errors.
    • Empty or unavailable values now consistently render as an 8-character mask.
    • Added coverage to verify the masked text matches the expected fallback output.

…y is missing

Signed-off-by: gkrajniak <gkrajniak@gmail.com>
@gkrajniak gkrajniak requested review from a team as code owners July 6, 2026 09:24
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The SecretValue component's maskedValue computed property now uses optional chaining (this.value()?.length) to safely handle undefined values. Imports and decorator metadata were reformatted. A test helper was updated to accept optional values, and a new test verifies default masking behavior.

Changes

SecretValue undefined value handling

Layer / File(s) Summary
Optional chaining fix and formatting
projects/ngx/declarative-ui/resource-field/secret-value/secret-value.component.ts
maskedValue computed logic changed to use this.value()?.length instead of this.value().length to avoid errors when value is undefined; imports and decorator metadata reformatted to multi-line without changing effective values.
Test coverage for undefined value
projects/ngx/declarative-ui/resource-field/secret-value/secret-value.component.spec.ts
makeComponent helper updated to accept an optional value parameter, and a new test verifies that an undefined value produces an 8-asterisk mask in both maskedValue() and the rendered text.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Poem

A rabbit hopped through code so neat,
Found a gap where values could delete,
Added a mark, a question small,
So secrets mask when there's nothing at all.
Eight little stars now stand up tall! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the main change: fixing secret mask calculation for missing values.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bug/fix-null-undefined-values-in-secrets-mask-calulcation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gkrajniak gkrajniak self-assigned this Jul 6, 2026
@gkrajniak gkrajniak added the bug Something isn't working label Jul 6, 2026
@gkrajniak

Copy link
Copy Markdown
Member Author

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
projects/ngx/declarative-ui/resource-field/secret-value/secret-value.component.ts (1)

17-20: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Relax value to an optional input
value() is only used inside the @if (isVisible()) branch, but resource-field.component.html can still pass undefined here. input<string>() matches the actual data flow better than input.required<string>().

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@projects/ngx/declarative-ui/resource-field/secret-value/secret-value.component.ts`
around lines 17 - 20, The SecretValueComponent input contract is too strict:
value is marked required even though callers may pass undefined and it is only
consumed when isVisible() is true. Update the value field in
secret-value.component.ts from input.required<string>() to an optional input
signature using input<string>() so the component matches the actual data flow,
while keeping maskedValue and the existing isVisible logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@projects/ngx/declarative-ui/resource-field/secret-value/secret-value.component.ts`:
- Around line 17-20: The SecretValueComponent input contract is too strict:
value is marked required even though callers may pass undefined and it is only
consumed when isVisible() is true. Update the value field in
secret-value.component.ts from input.required<string>() to an optional input
signature using input<string>() so the component matches the actual data flow,
while keeping maskedValue and the existing isVisible logic unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 801f140c-c29a-4b88-a8ef-2f7bf54b6587

📥 Commits

Reviewing files that changed from the base of the PR and between 3cd4b22 and 59981e0.

📒 Files selected for processing (2)
  • projects/ngx/declarative-ui/resource-field/secret-value/secret-value.component.spec.ts
  • projects/ngx/declarative-ui/resource-field/secret-value/secret-value.component.ts

@gkrajniak gkrajniak enabled auto-merge (squash) July 6, 2026 09:32
@gkrajniak gkrajniak requested a review from Sobyt483 July 6, 2026 09:33
@gkrajniak gkrajniak moved this to In review in OpenMFP Development Jul 6, 2026
@gkrajniak gkrajniak merged commit 6689f43 into main Jul 6, 2026
9 checks passed
@gkrajniak gkrajniak deleted the bug/fix-null-undefined-values-in-secrets-mask-calulcation branch July 6, 2026 09:33
@github-project-automation github-project-automation Bot moved this from In review to Done in OpenMFP Development Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants