Skip to content

WEB-824: Fix null handling in PrettyPrintPipe#3339

Merged
IOhacker merged 1 commit intoopenMF:devfrom
tkshsbcue:WEB-824/fix-pretty-print-null-handling
Mar 9, 2026
Merged

WEB-824: Fix null handling in PrettyPrintPipe#3339
IOhacker merged 1 commit intoopenMF:devfrom
tkshsbcue:WEB-824/fix-pretty-print-null-handling

Conversation

@tkshsbcue
Copy link
Contributor

@tkshsbcue tkshsbcue commented Mar 9, 2026

Description:
PrettyPrintPipe calls value.charAt() without validating the input. If the value is null, undefined, or not a string (which can happen with malformed datatable JSON data), the view crashes with a TypeError.

Fix:
Add a guard clause to safely handle null, undefined, and non-string values. Return an empty string when the input is invalid.

Ticket -> https://mifosforge.jira.com/browse/WEB-824?atlOrigin=eyJpIjoiZWMzYTY1MmNiYWUxNDFhMDkyYmZhNTYyYTM4ZjM0NjYiLCJwIjoiaiJ9

Summary by CodeRabbit

  • Bug Fixes
    • Improved the pretty-print feature to gracefully handle invalid inputs, returning empty content for null or non-string values.

Guard against null/undefined/non-string values; return empty string to
prevent runtime crash and objects/numbers reaching [innerHTML] consumer.

Made-with: Cursor
@coderabbitai
Copy link

coderabbitai bot commented Mar 9, 2026

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'pre_merge_checks'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

A null/non-string guard clause was added to the PrettyPrintPipe.transform method to handle invalid inputs by returning an empty string early, preventing downstream processing for non-string or null values.

Changes

Cohort / File(s) Summary
Input Validation Guard
src/app/pipes/pretty-print.pipe.ts
Added null/non-string check at the start of transform method to short-circuit execution and return empty string for invalid inputs, while preserving existing JSON-brace detection and error handling for valid strings.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main change: adding null handling to the PrettyPrintPipe, which directly addresses the guard clause addition mentioned in the summary.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/app/pipes/pretty-print.pipe.ts (1)

14-17: Consider using unknown instead of any for stricter type safety.

Per coding guidelines on strict type safety, using unknown with proper type guards (which you already have) is preferred over any. Adding an explicit return type also improves clarity.

♻️ Suggested type improvements
-  transform(value: any) {
+  transform(value: unknown): string {
     if (value == null || typeof value !== 'string') {
       return '';
     }

Based on learnings: "avoid using Observable as a project-wide pattern... introduce specific interfaces/types... and use proper typing instead of any."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/pipes/pretty-print.pipe.ts` around lines 14 - 17, The transform
method currently uses a loose any type and lacks an explicit return type; change
the parameter type to unknown and add an explicit return type (string) on the
transform signature (e.g., transform(value: unknown): string) and keep the
existing type guards (value == null || typeof value !== 'string') to safely
narrow the type before returning a string; update any callers or tests if they
relied on the previous any typing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/app/pipes/pretty-print.pipe.ts`:
- Around line 14-17: The transform method currently uses a loose any type and
lacks an explicit return type; change the parameter type to unknown and add an
explicit return type (string) on the transform signature (e.g., transform(value:
unknown): string) and keep the existing type guards (value == null || typeof
value !== 'string') to safely narrow the type before returning a string; update
any callers or tests if they relied on the previous any typing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f15a2358-63f3-488f-ab30-781993b43f10

📥 Commits

Reviewing files that changed from the base of the PR and between b1fd0f1 and 8273540.

📒 Files selected for processing (1)
  • src/app/pipes/pretty-print.pipe.ts

Copy link
Contributor

@IOhacker IOhacker left a comment

Choose a reason for hiding this comment

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

LGTM

@IOhacker IOhacker merged commit db8b4c5 into openMF:dev Mar 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants