-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Rahulbarwal/hotfix-v1.53/select-widget-handling #38259
Changes from all commits
462db00
524d400
5c7a468
662df2d
43b4737
7649de5
cf75cf5
b086761
23d5834
203b653
0a66486
c4e4ce3
6f7d58c
c3f8c5d
f170b92
19ffda1
67123fa
5367d23
7c3c28d
519ce9a
5811d43
ed4b37f
fc589d9
c3cb5ef
f041fcb
2613b67
9eb9e3d
455abf8
9a285aa
6c5fc61
4df3d45
8a1a287
c5efa91
a4c8a02
5ef0e94
a8e3d71
8d910dd
1edecf1
7930bec
54c8e78
5f9f0fb
299ce06
2199333
7c21b1c
ae9e006
76fc6f5
6689edd
f7b0472
16b5d20
4453206
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,6 +138,27 @@ export function labelKeyValidation( | |
messages: [], | ||
}; | ||
} else if (_.isArray(value)) { | ||
/* | ||
* Here assumption is that if evaluated array is all equal, then it is a key, | ||
* and we can return the parsed value(from source data) as the options. | ||
*/ | ||
const areAllValuesEqual = value.every((item, _, arr) => item === arr[0]); | ||
|
||
if ( | ||
areAllValuesEqual && | ||
props.sourceData[0].hasOwnProperty(String(value[0])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replace hasOwnProperty with Object.hasOwn Direct hasOwnProperty calls are unsafe. Use Also applies to: 250-250 🧰 Tools🪛 Biome (1.9.4)[error] 149-149: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). (lint/suspicious/noPrototypeBuiltins) |
||
) { | ||
const parsedValue = props.sourceData.map( | ||
(d: Record<string, unknown>) => d[String(value[0])], | ||
); | ||
|
||
return { | ||
parsed: parsedValue, | ||
isValid: true, | ||
messages: [], | ||
}; | ||
} | ||
Comment on lines
+141
to
+160
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Refactor validation logic for better safety and reusability The validation logic needs improvement in error handling and property access. Consider extracting the common logic and adding safety checks: + const validateSourceDataKey = (value: unknown[], sourceData: unknown[]) => {
+ if (!Array.isArray(sourceData) || !sourceData.length || !sourceData[0]) {
+ return { isValid: false, parsedValue: [] };
+ }
+
+ const areAllValuesEqual = value.every((item, _, arr) => item === arr[0]);
+ const firstValue = String(value[0]);
+
+ if (areAllValuesEqual && Object.hasOwn(sourceData[0], firstValue)) {
+ return {
+ isValid: true,
+ parsedValue: sourceData.map((d: Record<string, unknown>) => d[firstValue]),
+ };
+ }
+
+ return { isValid: false, parsedValue: [] };
+ }; Then use this helper in both validation functions.
🧰 Tools🪛 Biome (1.9.4)[error] 149-149: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). (lint/suspicious/noPrototypeBuiltins) |
||
|
||
const errorIndex = value.findIndex((d) => !_.isString(d)); | ||
|
||
return { | ||
|
@@ -218,6 +239,27 @@ export function valueKeyValidation( | |
|
||
options = sourceData.map((d: Record<string, unknown>) => d[value]); | ||
} else if (_.isArray(value)) { | ||
/* | ||
* Here assumption is that if evaluated array is all equal, then it is a key, | ||
* and we can return the parsed value(from source data) as the options. | ||
*/ | ||
const areAllValuesEqual = value.every((item, _, arr) => item === arr[0]); | ||
|
||
if ( | ||
areAllValuesEqual && | ||
props.sourceData[0].hasOwnProperty(String(value[0])) | ||
) { | ||
const parsedValue = props.sourceData.map( | ||
(d: Record<string, unknown>) => d[String(value[0])], | ||
); | ||
|
||
return { | ||
parsed: parsedValue, | ||
isValid: true, | ||
messages: [], | ||
}; | ||
} | ||
Comment on lines
+242
to
+261
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remove duplicated validation logic This block duplicates the logic from labelKeyValidation. Use the suggested 🧰 Tools🪛 Biome (1.9.4)[error] 250-250: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). (lint/suspicious/noPrototypeBuiltins) |
||
|
||
const errorIndex = value.findIndex( | ||
(d) => | ||
!(_.isString(d) || (_.isNumber(d) && !_.isNaN(d)) || _.isBoolean(d)), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add null checks and safer property access
The function needs defensive programming for edge cases.
Apply these improvements:
📝 Committable suggestion