**Enhance property field handling with transformations** #115
Conversation
- Added support for property field transformations in `FieldDefinition`, including `uppercase`, `lowercase`, `capitalize`, `encode`, and `decode`. - Updated utilities like `getResourceValueByJsonPath` to apply these transformations. - Enhanced documentation to reflect the new `propertyField` behavior. - Added unit tests to validate transformations and edge cases. - Updated dependencies in `package-lock.json` and `package.json`.
WalkthroughAdds a propertyField mechanism for selecting a sub-key from object fields and applying text transforms (uppercase, lowercase, capitalize, decode, encode). Introduces types, transform pipeline and Base64 helpers, expands resource-field extraction logic and tests, updates docs and template formatting, and tweaks a few config files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
|
||
| - `"label"`: Display name for the field | ||
| - `"property"`: JSON path to the resource property (string or array of strings for fallback values) | ||
| - `"propertyField"`: In case the property is a scalar value that represents an object, this property can be used to specify the field to be used for display within that object |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
projects/lib/utils/utils/resource-field-by-path.spec.ts (1)
86-164: Add test coverage for non-string value handling.The test suite thoroughly covers string transformations but doesn't verify behavior when
value[field.propertyField.key]is a non-string type (e.g., number, boolean, object). Consider adding a test case to ensure graceful handling of type mismatches.Example test to add:
it('handles non-string values gracefully', () => { const resource = { data: { config: { count: 42 }, // number, not string }, } as unknown as Resource; const field = { property: 'data.config', propertyField: { key: 'count', transform: ['uppercase'] }, } as unknown as FieldDefinition; const result = getResourceValueByJsonPath(resource, field); // Define expected behavior: should it return undefined, the original value, or throw? expect(result).toBeDefined(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
docs/readme-generic-ui.md(4 hunks)package.json(1 hunks)projects/lib/models/models/resource.ts(3 hunks)projects/lib/utils/utils/get-value-by-path.ts(0 hunks)projects/lib/utils/utils/resource-field-by-path.spec.ts(1 hunks)projects/lib/utils/utils/resource-field-by-path.ts(2 hunks)projects/wc/src/app/components/generic-ui/list-view/list-view.component.html(3 hunks)
💤 Files with no reviewable changes (1)
- projects/lib/utils/utils/get-value-by-path.ts
🧰 Additional context used
🧬 Code graph analysis (2)
projects/lib/utils/utils/resource-field-by-path.spec.ts (2)
projects/lib/models/models/resource.ts (2)
Resource(65-72)FieldDefinition(24-45)projects/lib/utils/utils/resource-field-by-path.ts (1)
getResourceValueByJsonPath(8-39)
projects/lib/utils/utils/resource-field-by-path.ts (1)
projects/lib/models/models/resource.ts (3)
Resource(65-72)PropertyField(19-22)TransformType(12-17)
🔇 Additional comments (1)
projects/lib/utils/utils/resource-field-by-path.ts (1)
79-101: LGTM! Base64 encoding/decoding implementation is correct.The UTF-8 handling via
TextEncoder/TextDecoderproperly supports multi-byte characters, and error handling is appropriate for the transform pipeline use case.
| if (value && field.propertyField) { | ||
| return executeTransform( | ||
| value[field.propertyField.key], | ||
| field.propertyField.transform, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add type guard for non-string values before transformation.
The value[field.propertyField.key] expression returns any from the JSONPath query result, but executeTransform expects string | undefined. If the value is a number, boolean, or object, the string transforms (uppercase, lowercase, capitalize) will throw runtime errors since they call String prototype methods without type checking.
Apply this defensive check:
if (value && field.propertyField) {
+ const fieldValue = value[field.propertyField.key];
+ if (typeof fieldValue !== 'string' && fieldValue != null) {
+ console.warn(
+ `Expected string for propertyField key "${field.propertyField.key}", got ${typeof fieldValue}. Skipping transforms.`,
+ );
+ return fieldValue;
+ }
return executeTransform(
- value[field.propertyField.key],
+ fieldValue,
field.propertyField.transform,
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (value && field.propertyField) { | |
| return executeTransform( | |
| value[field.propertyField.key], | |
| field.propertyField.transform, | |
| ); | |
| } | |
| if (value && field.propertyField) { | |
| const fieldValue = value[field.propertyField.key]; | |
| if (typeof fieldValue !== 'string' && fieldValue != null) { | |
| console.warn( | |
| `Expected string for propertyField key "${field.propertyField.key}", got ${typeof fieldValue}. Skipping transforms.`, | |
| ); | |
| return fieldValue; | |
| } | |
| return executeTransform( | |
| fieldValue, | |
| field.propertyField.transform, | |
| ); | |
| } |
🤖 Prompt for AI Agents
In projects/lib/utils/utils/resource-field-by-path.ts around lines 31 to 36, the
code calls executeTransform with value[field.propertyField.key] which can be any
JSONPath result; add a type guard so you only call executeTransform when that
extracted value is a string or undefined. Specifically, read the value into a
local (e.g. val), check if typeof val === 'string' || val === undefined, and
only then return executeTransform(val, field.propertyField.transform); otherwise
return the original val (or undefined) without attempting string transformations
to avoid runtime errors.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
projects/lib/jest.config.js (1)
13-13: Consider maintaining the previous coverage threshold.The statements threshold has been lowered from -3 to -4, allowing one additional uncovered statement. Since this PR adds new unit tests for property field transformations, could you clarify why the coverage threshold needs to be relaxed? Ideally, coverage requirements should be maintained or improved when adding new functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
package.json(1 hunks)projects/lib/jest.config.js(1 hunks)projects/lib/utils/utils/resource-field-by-path.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🧬 Code graph analysis (1)
projects/lib/utils/utils/resource-field-by-path.spec.ts (2)
projects/lib/models/models/resource.ts (2)
Resource(65-72)PropertyField(19-22)projects/lib/utils/utils/resource-field-by-path.ts (3)
getResourceValueByJsonPath(8-39)encodeBase64(79-90)decodeBase64(92-101)
🔇 Additional comments (4)
projects/lib/utils/utils/resource-field-by-path.spec.ts (4)
1-19: Excellent test setup!The imports are clean, mocking strategy is appropriate, and the beforeEach hook ensures test isolation by clearing mocks between runs.
21-254: Outstanding test coverage for getResourceValueByJsonPath!The test suite comprehensively validates:
- All transform types (uppercase, lowercase, capitalize, encode, decode)
- Transform chaining and unknown transforms
- Error resilience with graceful fallbacks
- Edge cases (null, undefined, empty arrays, missing properties)
- Integration with jsonpath queries and propertyField extraction
The tests are well-organized, clearly named, and properly handle mock lifecycle management.
256-307: Thorough coverage of Base64 encoding!The tests validate ASCII, UTF-8, emojis, empty strings, error throwing, and error logging. Mock management is clean with proper restoration. The approach of using decodeBase64 for verification in UTF-8 tests (lines 264, 269) is pragmatic given the complexity of manually specifying encoded output.
309-359: Excellent coverage of Base64 decoding with round-trip validation!The tests comprehensively cover ASCII, UTF-8, emojis, empty strings, invalid input handling, error logging, and round-trip encoding/decoding. The round-trip test (lines 353-358) is particularly valuable for ensuring encode and decode work correctly as a pair.
gkrajniak
deleted the
feat/add-property-fileds-handling-for-scalar-values
branch
FieldDefinition, includinguppercase,lowercase,capitalize,encode, anddecode.getResourceValueByJsonPathto apply these transformations.propertyFieldbehavior.package-lock.jsonandpackage.json.Summary by CodeRabbit
New Features
Documentation
Tests
Chores