validator: boolean #648
validator: boolean #648
Conversation
WalkthroughThe changes introduce a new Flatfile Boolean Validator Plugin that validates boolean values with options for strict and truthy validation. It supports multiple languages and allows for custom configurations, including case sensitivity and error message handling. Additionally, a Rollup configuration file is added for building the plugin, and a new TypeScript file implements the validation logic. An end-to-end test suite is also created to ensure the functionality of the Changes
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (1)
validate/boolean/rollup.config.mjs (1)
1-5: Overall, the Rollup configuration looks good, with room for minor improvements.The configuration file is concise, well-structured, and follows best practices for Rollup configuration. The use of a custom
buildConfigfunction from an internal package suggests a standardized approach across the organization, which is commendable.However, there are a couple of points to consider for improvement:
Documentation: Consider adding a brief comment at the top of the file explaining the purpose of this configuration, especially in relation to the boolean validator.
Specificity: Verify if any specific configuration options are needed for the boolean validator. If not, it might be helpful to add a comment explaining why the default configuration is sufficient.
These minor additions would enhance the maintainability and clarity of the configuration file.
Would you like assistance in drafting the suggested documentation comments?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
validate/boolean/metadata.jsonis excluded by!**/*.jsonvalidate/boolean/package.jsonis excluded by!**/*.json
📒 Files selected for processing (4)
- validate/boolean/README.MD (1 hunks)
- validate/boolean/rollup.config.mjs (1 hunks)
- validate/boolean/src/index.ts (1 hunks)
- validate/boolean/src/validateBoolean.e2e.spec.ts (1 hunks)
🧰 Additional context used
🪛 LanguageTool
validate/boolean/README.MD
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...th the following properties: -fields: An array of field names to validate - `...(UNLIKELY_OPENING_PUNCTUATION)
🪛 Biome
validate/boolean/src/index.ts
[error] 47-47: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (5)
validate/boolean/rollup.config.mjs (3)
1-1: LGTM: Import statement is correct and follows best practices.The import statement correctly uses ES6 module syntax to import the
buildConfigfunction from what appears to be an internal Flatfile package. This approach suggests a standardized configuration setup across the organization, which is a good practice for maintaining consistency.
3-3: Verify if default configuration is sufficient for boolean validator.The configuration is created using the
buildConfigfunction with an empty object as an argument, which will use default configuration options. While this approach is valid, it's worth confirming whether any specific configuration options are needed for a boolean validator.To ensure we're not missing any necessary configuration, could you verify if there are any boolean-specific options that should be included? You might want to check the documentation for
@flatfile/rollup-configor consult with the team about standard practices for validator configurations.
5-5: LGTM: Default export is correct.The configuration object is correctly exported as the default export using ES6 module syntax. This is the standard approach for Rollup configuration files and will allow easy import in other parts of the build process.
validate/boolean/src/index.ts (1)
90-91: Verify the conversion of numeric values to booleanWhen
normalizedValueis a number, it is converted to a boolean usingBoolean(normalizedValue). This means any non-zero number becomestrue, and zero becomesfalse. Ensure that this conversion aligns with the intended validation logic and doesn't introduce unexpected behavior.Would you like assistance in reviewing this conversion to ensure it meets the validation requirements?
validate/boolean/src/validateBoolean.e2e.spec.ts (1)
52-79:⚠️ Potential issueVerify the acceptance of string values in 'strict' validation
In the 'strict' validation type test, strings
'true'and'false'are being accepted as valid boolean values. If 'strict' validation is intended to accept only booleantrueandfalse, then string representations should be considered invalid. Please confirm whether accepting strings in 'strict' mode is the intended behavior.
validate/boolean/README.MD
Outdated
| import { validateBoolean } from '@flatfile/plugin-boolean-validator'; | ||
|
|
||
| const booleanValidator = validateBoolean({ |
There was a problem hiding this comment.
Inconsistent Function Naming: Use validateBoolean Consistently
The example code uses validateBoolean, but the configuration section refers to the BooleanValidator function. For clarity and consistency, please use the same function name throughout the documentation.
Suggestion: Replace BooleanValidator with validateBoolean in the configuration section to match the example usage.
| The `BooleanValidator` function accepts a configuration object with the following properties: | ||
|
|
||
| - `fields`: An array of field names to validate |
There was a problem hiding this comment.
Formatting Issue: Remove Extra Blank Line Before List
An extra blank line after line 42 may disrupt the list formatting in Markdown.
Suggestion: Remove the blank line at line 43 so that the list follows immediately after the introductory sentence.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...th the following properties: -fields: An array of field names to validate - `...(UNLIKELY_OPENING_PUNCTUATION)
validate/boolean/src/index.ts
Outdated
|
|
||
| if (normalizedValue === true || normalizedValue === false) { | ||
| record.set(field, normalizedValue) | ||
| } else if (mapping.hasOwnProperty(normalizedValue)) { |
There was a problem hiding this comment.
Use Object.hasOwn() instead of hasOwnProperty
Directly accessing hasOwnProperty on an object can lead to unexpected behavior if the object has a property named hasOwnProperty. It's safer to use Object.hasOwn() for property checks to avoid potential prototype pollution issues.
Apply this diff to fix the issue:
- } else if (mapping.hasOwnProperty(normalizedValue)) {
+ } else if (Object.hasOwn(mapping, normalizedValue)) {📝 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.
| } else if (mapping.hasOwnProperty(normalizedValue)) { | |
| } else if (Object.hasOwn(mapping, normalizedValue)) { |
🧰 Tools
🪛 Biome
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
validate/boolean/src/index.ts
Outdated
| case 'skip': | ||
| default: | ||
| // Do nothing, leave the field as is | ||
| break |
There was a problem hiding this comment.
Remove redundant case 'skip': in switch statement
In the handleNullValue function, the case 'skip': clause is unnecessary because the default: clause already handles all unspecified cases, including 'skip'. Removing this redundant case simplifies the code without changing its behavior.
Apply this diff to fix the issue:
switch (config.handleNull) {
case 'error':
// ...
break
case 'false':
// ...
break
case 'true':
// ...
break
- case 'skip':
default:
// Do nothing, leave the field as is
break
}📝 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.
| case 'skip': | |
| default: | |
| // Do nothing, leave the field as is | |
| break | |
| default: | |
| // Do nothing, leave the field as is | |
| break |
🧰 Tools
🪛 Biome
[error] 47-47: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
validate/boolean/src/index.ts
Outdated
| const defaultMapping = config.language | ||
| ? languageMappings[config.language] | ||
| : languageMappings.en | ||
| const mapping = config.customMapping || defaultMapping |
There was a problem hiding this comment.
Handle undefined language mappings to prevent runtime errors
When config.language is set to a language not defined in languageMappings, languageMappings[config.language] will be undefined. This can cause errors when mapping is used later in the code. Consider providing a fallback to the English mapping or adding a check to ensure the mapping exists.
Apply this diff to fix the issue:
const defaultMapping = config.language
- ? languageMappings[config.language]
+ ? languageMappings[config.language] || languageMappings.en
: languageMappings.en📝 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.
| const defaultMapping = config.language | |
| ? languageMappings[config.language] | |
| : languageMappings.en | |
| const mapping = config.customMapping || defaultMapping | |
| const defaultMapping = config.language | |
| ? languageMappings[config.language] || languageMappings.en | |
| : languageMappings.en | |
| const mapping = config.customMapping || defaultMapping |
validate/boolean/src/index.ts
Outdated
| if (normalizedValue === true || normalizedValue === false) { | ||
| record.set(field, normalizedValue) | ||
| } else if (mapping.hasOwnProperty(normalizedValue)) { |
There was a problem hiding this comment.
Correct type checking for boolean values
In the validateTruthyBoolean function, the condition normalizedValue === true || normalizedValue === false will almost always evaluate to false because normalizedValue is typically a string after normalization. This condition should check the type of value directly to accurately identify boolean values.
Apply this diff to fix the issue:
- if (normalizedValue === true || normalizedValue === false) {
+ if (typeof value === 'boolean') {
record.set(field, value)📝 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 (normalizedValue === true || normalizedValue === false) { | |
| record.set(field, normalizedValue) | |
| } else if (mapping.hasOwnProperty(normalizedValue)) { | |
| if (typeof value === 'boolean') { | |
| record.set(field, value) | |
| } else if (mapping.hasOwnProperty(normalizedValue)) { |
🧰 Tools
🪛 Biome
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
| ) | ||
|
|
||
| await createRecords(sheetId, [ | ||
| { isActive: true }, | ||
| { isActive: false }, | ||
| { isActive: 'true' }, | ||
| { isActive: 'false' }, | ||
| { isActive: 1 }, | ||
| { isActive: 0 }, | ||
| ]) | ||
| await listener.waitFor('commit:created') | ||
|
|
||
| const records = await getRecords(sheetId) | ||
|
|
||
| expect(records[0].values['isActive'].value).toBe(true) | ||
| expect(records[1].values['isActive'].value).toBe(false) | ||
| expect(records[2].values['isActive'].value).toBe(true) | ||
| expect(records[3].values['isActive'].value).toBe(false) | ||
| expect(records[4].values['isActive'].messages[0].message).toContain('Invalid boolean value') | ||
| expect(records[5].values['isActive'].messages[0].message).toContain('Invalid boolean value') | ||
|
|
||
| }) | ||
|
|
||
| it('validates truthy boolean values', async () => { | ||
| listener.use( | ||
| validateBoolean({ | ||
| fields: ['hasSubscription'], | ||
| validationType: 'truthy', | ||
| }) | ||
| ) | ||
|
|
||
| await createRecords(sheetId, [ | ||
| { hasSubscription: true }, | ||
| { hasSubscription: false }, | ||
| { hasSubscription: 'true' }, | ||
| { hasSubscription: 'false' }, | ||
| { hasSubscription: 1 }, | ||
| { hasSubscription: 0 }, | ||
| { hasSubscription: 'yes' }, | ||
| { hasSubscription: 'no' }, | ||
| ]) | ||
| await listener.waitFor('commit:created') | ||
|
|
||
| const records = await getRecords(sheetId) | ||
|
|
||
| expect(records[0].values['hasSubscription'].value).toBeTruthy() | ||
| expect(records[1].values['hasSubscription'].value).toBeFalsy() | ||
| expect(records[2].values['hasSubscription'].value).toBeTruthy() | ||
| expect(records[3].values['hasSubscription'].value).toBeFalsy() | ||
| expect(records[4].values['hasSubscription'].value).toBeTruthy() | ||
| expect(records[5].values['hasSubscription'].value).toBeFalsy() | ||
| expect(records[6].values['hasSubscription'].value).toBeTruthy() | ||
| expect(records[7].values['hasSubscription'].value).toBeFalsy() | ||
| }) | ||
|
|
||
| it('handles custom mapping', async () => { | ||
| listener.use( | ||
| validateBoolean({ | ||
| fields: ['agreeToTerms'], | ||
| validationType: 'truthy', | ||
| customMapping: { 'agreed': true, 'disagreed': false }, | ||
| }) | ||
| ) | ||
|
|
||
| await createRecords(sheetId, [ | ||
| { agreeToTerms: 'agreed' }, | ||
| { agreeToTerms: 'disagreed' }, | ||
| { agreeToTerms: 'yes' }, | ||
| ]) | ||
| await listener.waitFor('commit:created') | ||
|
|
||
| const records = await getRecords(sheetId) | ||
|
|
||
| expect(records[0].values['agreeToTerms'].value).toBeTruthy() | ||
| expect(records[1].values['agreeToTerms'].value).toBeFalsy() | ||
| expect(records[2].values['agreeToTerms'].value).toBeTruthy() | ||
| }) | ||
|
|
||
| it('handles null values', async () => { | ||
| listener.use( | ||
| validateBoolean({ | ||
| fields: ['optIn'], | ||
| validationType: 'truthy', | ||
| handleNull: 'false', | ||
| }) | ||
| ) | ||
|
|
||
| await createRecords(sheetId, [ | ||
| { optIn: null }, | ||
| { optIn: undefined }, | ||
| { optIn: '' }, | ||
| ]) | ||
| await listener.waitFor('commit:created') | ||
|
|
||
| const records = await getRecords(sheetId) | ||
|
|
||
| expect(records[0].values['optIn'].value).toBe(false) | ||
| expect(records[1].values['optIn'].value).toBe(false) | ||
| expect(records[2].values['optIn'].value).toBe(false) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor tests using parameterized testing to reduce duplication
The test cases within validateBoolean() have similar structures with repeated code blocks. Refactoring them using parameterized tests can improve maintainability and readability.
Consider refactoring using Jest's test.each or forEach to parameterize the tests:
describe('validateBoolean()', () => {
const testCases = [
{
description: 'validates strict boolean values',
listenerConfig: {
fields: ['isActive'],
validationType: 'strict',
},
records: [
{ input: { isActive: true }, expectedValue: true },
{ input: { isActive: false }, expectedValue: false },
{ input: { isActive: 'true' }, expectedValue: true },
{ input: { isActive: 'false' }, expectedValue: false },
{ input: { isActive: 1 }, expectedError: 'Invalid boolean value' },
{ input: { isActive: 0 }, expectedError: 'Invalid boolean value' },
],
},
// Add other test scenarios here
];
testCases.forEach(({ description, listenerConfig, records }) => {
it(description, async () => {
listener.use(validateBoolean(listenerConfig));
await createRecords(sheetId, records.map((r) => r.input));
await listener.waitFor('commit:created');
const resultRecords = await getRecords(sheetId);
resultRecords.forEach((record, index) => {
const expectedValue = records[index].expectedValue;
const expectedError = records[index].expectedError;
const field = listenerConfig.fields[0];
if (expectedValue !== undefined) {
expect(record.values[field].value).toBe(expectedValue);
}
if (expectedError) {
expect(record.values[field].messages[0].message).toContain(expectedError);
}
});
});
});
});| const logSpy = jest.spyOn(global.console, 'log') | ||
| const logErrorSpy = jest.spyOn(global.console, 'error') |
There was a problem hiding this comment.
Remove unused console spies
The logSpy and logErrorSpy variables are declared but never used in the test suite. Removing them will clean up the code.
Apply this diff to remove the unused variables:
- const logSpy = jest.spyOn(global.console, 'log')
- const logErrorSpy = jest.spyOn(global.console, 'error')📝 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.
| const logSpy = jest.spyOn(global.console, 'log') | |
| const logErrorSpy = jest.spyOn(global.console, 'error') |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
validate/boolean/src/index.ts (3)
4-19: Consider adding JSDoc comments for better documentation.The
BooleanValidatorConfiginterface is well-structured and comprehensive. To improve clarity and maintainability, consider adding JSDoc comments for each field, especially for fields with multiple options or non-obvious usage.For example:
interface BooleanValidatorConfig { /** List of fields to validate */ fields: string[] /** Validation type: 'strict' for exact boolean values, 'truthy' for flexible parsing */ validationType: 'strict' | 'truthy' // ... (add comments for other fields) /** Specify the sheet slug for targeted validation */ sheetSlug?: string }This will provide better context for developers using this configuration interface.
98-116: Enhance error and info messages in handleInvalidValue function.The
handleInvalidValuefunction correctly handles invalid boolean values. However, the error and info messages could be more descriptive to provide better context. Consider the following improvements:
- Include the field name in the error message.
- Add the original invalid value to the info message.
Apply this diff to implement these suggestions:
function handleInvalidValue( record: FlatfileRecord, field: string, config: BooleanValidatorConfig ) { if (config.defaultValue === undefined || config.defaultValue === 'skip') { record.addError( field, config.customErrorMessages?.invalidBoolean || - 'Invalid boolean value' + `Invalid boolean value for field '${field}'` ) } else { record.set(field, config.defaultValue) record.addInfo( field, - `Invalid value converted to default: ${config.defaultValue}` + `Invalid value '${record.get(field)}' converted to default: ${config.defaultValue}` ) } }These changes provide more context in the error and info messages, making it easier for users to understand and debug issues.
119-141: Remove unnecessary async keyword and LGTM.The
validateBooleanfunction is well-implemented and correctly integrates with the Flatfile Listener. However, theasynckeyword on the callback function is unnecessary as there are no asynchronous operations inside it.Apply this small change:
export const validateBoolean = (config: BooleanValidatorConfig) => { return (listener: FlatfileListener) => { listener.use( - recordHook(config.sheetSlug || '**', async (record: FlatfileRecord) => { + recordHook(config.sheetSlug || '**', (record: FlatfileRecord) => { config.fields.forEach((field) => { // ... (existing code) }) return record }) ) } }Other than this minor adjustment, the implementation looks good and covers both strict and truthy validation types as specified in the configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
validate/boolean/package.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
- validate/boolean/src/index.ts (1 hunks)
🧰 Additional context used
🪛 Biome
validate/boolean/src/index.ts
[error] 47-47: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (2)
validate/boolean/src/index.ts (2)
1-2: LGTM: Imports are correct and follow best practices.The current import syntax is appropriate and follows modern TypeScript practices. It correctly imports the necessary types and functions from the Flatfile packages.
1-141: Overall, a well-implemented and flexible boolean validation system.This implementation provides a comprehensive and flexible boolean validation system for Flatfile. The code is well-structured, follows TypeScript best practices, and offers a wide range of configuration options to handle various scenarios.
Key strengths:
- Flexible configuration options through the
BooleanValidatorConfiginterface.- Support for multiple languages and custom mappings.
- Handling of both strict and truthy boolean validations.
- Integration with the Flatfile Listener system.
Areas for improvement have been noted in previous comments, including:
- Adding JSDoc comments for better documentation.
- Making language mappings extensible.
- Improving error and info messages for better context.
- Minor optimizations and safety improvements in some functions.
These suggestions, if implemented, will further enhance the robustness and usability of the validation system. Great job overall!
🧰 Tools
🪛 Biome
[error] 47-47: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
validate/boolean/src/index.ts
Outdated
| const languageMappings: Record<string, Record<string, boolean>> = { | ||
| en: { yes: true, no: false, y: true, n: false }, | ||
| es: { sí: true, si: true, no: false, s: true, n: false }, | ||
| fr: { oui: true, non: false, o: true, n: false }, | ||
| de: { ja: true, nein: false, j: true, n: false }, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making languageMappings extensible.
The languageMappings object is well-defined and covers common languages. To improve flexibility and allow for easy addition of new languages, consider:
- Exporting the
languageMappingsobject. - Providing a function to add new language mappings.
Example implementation:
export const languageMappings: Record<string, Record<string, boolean>> = {
// ... existing mappings
}
export function addLanguageMapping(lang: string, mapping: Record<string, boolean>) {
languageMappings[lang] = { ...languageMappings[lang], ...mapping }
}This approach would allow users to easily extend the language support without modifying the original code.
validate/boolean/src/index.ts
Outdated
| function handleNullValue( | ||
| record: FlatfileRecord, | ||
| field: string, | ||
| config: BooleanValidatorConfig | ||
| ) { | ||
| switch (config.handleNull) { | ||
| case 'error': | ||
| record.addError( | ||
| field, | ||
| config.customErrorMessages?.nullValue || | ||
| 'Value cannot be null or undefined' | ||
| ) | ||
| break | ||
| case 'false': | ||
| record.set(field, false) | ||
| break | ||
| case 'true': | ||
| record.set(field, true) | ||
| break | ||
| case 'skip': | ||
| default: | ||
| // Do nothing, leave the field as is | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove redundant case and improve error message.
-
As noted in a previous review, the
case 'skip':is redundant and can be removed since it's handled by thedefaultcase. -
Consider making the error message more descriptive. For example:
record.addError(
field,
config.customErrorMessages?.nullValue ||
`Field '${field}' cannot be null or undefined`
)This change provides more context about which field caused the error.
Apply this diff to implement these suggestions:
function handleNullValue(
record: FlatfileRecord,
field: string,
config: BooleanValidatorConfig
) {
switch (config.handleNull) {
case 'error':
record.addError(
field,
config.customErrorMessages?.nullValue ||
- 'Value cannot be null or undefined'
+ `Field '${field}' cannot be null or undefined`
)
break
case 'false':
record.set(field, false)
break
case 'true':
record.set(field, true)
break
- case 'skip':
default:
// Do nothing, leave the field as is
break
}
}🧰 Tools
🪛 Biome
[error] 47-47: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
validate/boolean/src/index.ts
Outdated
| function validateStrictBoolean( | ||
| record: FlatfileRecord, | ||
| field: string, | ||
| value: any, | ||
| config: BooleanValidatorConfig | ||
| ) { | ||
| if ( value === true || value === false) { | ||
| record.set(field, value) | ||
| } else if (config.convertNonBoolean) { | ||
| record.set(field, Boolean(value)) | ||
| } else { | ||
| handleInvalidValue(record, field, config) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider handling string boolean values in strict mode.
The validateStrictBoolean function correctly handles boolean values and non-boolean conversion. However, in some cases, users might expect string 'true' or 'false' to be treated as valid boolean values even in strict mode. Consider adding an optional configuration flag to handle this case:
interface BooleanValidatorConfig {
// ... existing fields
strictStringBooleans?: boolean
}
function validateStrictBoolean(
record: FlatfileRecord,
field: string,
value: any,
config: BooleanValidatorConfig
) {
if (value === true || value === false) {
record.set(field, value)
} else if (config.strictStringBooleans && (value === 'true' || value === 'false')) {
record.set(field, value === 'true')
} else if (config.convertNonBoolean) {
record.set(field, Boolean(value))
} else {
handleInvalidValue(record, field, config)
}
}This enhancement would provide more flexibility while maintaining the strict validation when needed.
validate/boolean/src/index.ts
Outdated
| function validateTruthyBoolean( | ||
| record: FlatfileRecord, | ||
| field: string, | ||
| value: any, | ||
| config: BooleanValidatorConfig | ||
| ) { | ||
| const defaultMapping = config.language | ||
| ? languageMappings[config.language] | ||
| : languageMappings.en | ||
| const mapping = config.customMapping || defaultMapping | ||
|
|
||
| let normalizedValue = value | ||
| if (typeof value === 'string' && !config.caseSensitive) { | ||
| normalizedValue = value.toLowerCase() | ||
| } | ||
|
|
||
| if (normalizedValue === true || normalizedValue === false) { | ||
| record.set(field, normalizedValue) | ||
| } else if (mapping.hasOwnProperty(normalizedValue)) { | ||
| record.set(field, mapping[normalizedValue]) | ||
| } else if (typeof normalizedValue === 'number') { | ||
| record.set(field, Boolean(normalizedValue)) | ||
| } else if (config.convertNonBoolean) { | ||
| record.set(field, Boolean(value)) | ||
| } else { | ||
| handleInvalidValue(record, field, config) | ||
| } | ||
| } |
There was a problem hiding this comment.
Improve validateTruthyBoolean function logic and safety.
-
Remove the unnecessary condition for boolean check:
ThenormalizedValue === true || normalizedValue === falsecheck is redundant becausenormalizedValueis always a string at this point. -
Use
Object.hasOwn()instead ofhasOwnProperty:
This change improves safety against prototype pollution. -
Consider using a type guard for the
valueparameter to improve type safety.
Apply this diff to implement these suggestions:
function validateTruthyBoolean(
record: FlatfileRecord,
field: string,
- value: any,
+ value: unknown,
config: BooleanValidatorConfig
) {
const defaultMapping = config.language
? languageMappings[config.language]
: languageMappings.en
const mapping = config.customMapping || defaultMapping
let normalizedValue = value
if (typeof value === 'string' && !config.caseSensitive) {
normalizedValue = value.toLowerCase()
}
- if (normalizedValue === true || normalizedValue === false) {
+ if (typeof value === 'boolean') {
record.set(field, value)
- } else if (mapping.hasOwnProperty(normalizedValue)) {
+ } else if (typeof normalizedValue === 'string' && Object.hasOwn(mapping, normalizedValue)) {
record.set(field, mapping[normalizedValue])
} else if (typeof normalizedValue === 'number') {
record.set(field, Boolean(normalizedValue))
} else if (config.convertNonBoolean) {
record.set(field, Boolean(value))
} else {
handleInvalidValue(record, field, config)
}
}These changes improve the function's type safety and robustness.
📝 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.
| function validateTruthyBoolean( | |
| record: FlatfileRecord, | |
| field: string, | |
| value: any, | |
| config: BooleanValidatorConfig | |
| ) { | |
| const defaultMapping = config.language | |
| ? languageMappings[config.language] | |
| : languageMappings.en | |
| const mapping = config.customMapping || defaultMapping | |
| let normalizedValue = value | |
| if (typeof value === 'string' && !config.caseSensitive) { | |
| normalizedValue = value.toLowerCase() | |
| } | |
| if (normalizedValue === true || normalizedValue === false) { | |
| record.set(field, normalizedValue) | |
| } else if (mapping.hasOwnProperty(normalizedValue)) { | |
| record.set(field, mapping[normalizedValue]) | |
| } else if (typeof normalizedValue === 'number') { | |
| record.set(field, Boolean(normalizedValue)) | |
| } else if (config.convertNonBoolean) { | |
| record.set(field, Boolean(value)) | |
| } else { | |
| handleInvalidValue(record, field, config) | |
| } | |
| } | |
| function validateTruthyBoolean( | |
| record: FlatfileRecord, | |
| field: string, | |
| value: unknown, | |
| config: BooleanValidatorConfig | |
| ) { | |
| const defaultMapping = config.language | |
| ? languageMappings[config.language] | |
| : languageMappings.en | |
| const mapping = config.customMapping || defaultMapping | |
| let normalizedValue = value | |
| if (typeof value === 'string' && !config.caseSensitive) { | |
| normalizedValue = value.toLowerCase() | |
| } | |
| if (typeof value === 'boolean') { | |
| record.set(field, value) | |
| } else if (typeof normalizedValue === 'string' && Object.hasOwn(mapping, normalizedValue)) { | |
| record.set(field, mapping[normalizedValue]) | |
| } else if (typeof normalizedValue === 'number') { | |
| record.set(field, Boolean(normalizedValue)) | |
| } else if (config.convertNonBoolean) { | |
| record.set(field, Boolean(value)) | |
| } else { | |
| handleInvalidValue(record, field, config) | |
| } | |
| } |
🧰 Tools
🪛 Biome
[error] 88-88: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
validate/boolean/README.MD
Outdated
| @@ -0,0 +1,63 @@ | |||
| # Flatfile Boolean Validator Plugin | |||
There was a problem hiding this comment.
This doesn't follow our README structure
bangarang
force-pushed
the
feat/BooleanValidator
branch
from
19498b1 to
8250fcc
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (2)
validate/boolean/README.MD (2)
63-63: Consider reorganizing integration informationThe information about using the plugin as a RecordHook or an external constraint is valuable. However, it might be more beneficial to users if this information was placed in a dedicated "Integration" or "Usage" section earlier in the README.
Consider creating a new section, such as "Integration Options" or "Usage Methods", and moving this information there. This would make it more visible and easier for users to find when looking for ways to implement the plugin in their projects.
1-63: README provides comprehensive documentation with room for minor improvementsOverall, this README offers clear and detailed documentation for the Flatfile Boolean Validator Plugin. It covers essential aspects such as features, installation, usage, configuration, and behavior. The content is informative and well-presented.
To further enhance the document:
- Consider adjusting the overall structure to align with project guidelines, including adding sections like a table of contents, prerequisites, and contributing information.
- Reorganize the integration information into a dedicated section for better visibility.
- Review the document for any remaining inconsistencies in terminology or formatting.
These minor improvements will make the README even more user-friendly and aligned with project standards.
Would you like assistance in implementing any of these suggested improvements?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...th the following properties: -fields: An array of field names to validate - `...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
validate/boolean/metadata.jsonis excluded by!**/*.jsonvalidate/boolean/package.jsonis excluded by!**/*.json
📒 Files selected for processing (5)
- validate/boolean/README.MD (1 hunks)
- validate/boolean/rollup.config.mjs (1 hunks)
- validate/boolean/src/index.ts (1 hunks)
- validate/boolean/src/validate.boolean.plugin.e2e.spec.ts (1 hunks)
- validate/boolean/src/validate.boolean.plugin.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- validate/boolean/rollup.config.mjs
- validate/boolean/src/index.ts
🧰 Additional context used
🪛 LanguageTool
validate/boolean/README.MD
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...th the following properties: -fields: An array of field names to validate - `...(UNLIKELY_OPENING_PUNCTUATION)
🪛 Biome
validate/boolean/src/validate.boolean.plugin.ts
[error] 47-47: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
[error] 87-87: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
🔇 Additional comments (8)
validate/boolean/README.MD (6)
5-15: Features section looks goodThe Features section provides a comprehensive and clear list of the plugin's capabilities. It effectively communicates the key functionalities to potential users.
16-23: Installation section is clear and conciseThe Installation section provides a straightforward npm command for installing the plugin. This is sufficient for most users familiar with npm.
24-38: Example Usage section is clear and consistentThe Example Usage section provides a clear demonstration of how to use the plugin. The code snippet is easy to understand and shows key configuration options.
Note: The previously mentioned inconsistency in function naming (
validateBooleanvsBooleanValidator) has been resolved. The example now consistently usesvalidateBoolean.
40-52: Configuration section is well-structuredThe Configuration section clearly lists all the properties of the configuration object. Each property is briefly described, providing users with a good understanding of the available options.
Note: The previously mentioned formatting issue (extra blank line before the list) has been resolved. The list now follows immediately after the introductory sentence, improving the Markdown formatting.
The LanguageTool hint about a loose punctuation mark on line 44 appears to be a false positive. The Markdown formatting for the list is correct and consistent.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...th the following properties: -fields: An array of field names to validate - `...(UNLIKELY_OPENING_PUNCTUATION)
54-61: Behavior section provides clear operational detailsThe Behavior section effectively outlines the key operational aspects of the plugin. It covers important topics such as validation types, null handling, and error management, giving users a good understanding of how the plugin will function in different scenarios.
1-4:⚠️ Potential issueAdjust README structure to follow project guidelines
The current structure of the README doesn't seem to follow the project's standard README format. Consider adjusting the structure to align with the project's guidelines.
Suggestions for improvement:
- Add a badges section (if applicable) after the title.
- Include a table of contents for easy navigation.
- Consider adding sections like "Prerequisites" and "Contributing" if they're part of the standard structure.
Would you like assistance in restructuring the README to match the project's standard format?
validate/boolean/src/validate.boolean.plugin.e2e.spec.ts (2)
141-141: Verify the type of 'handleNull' optionThe
handleNulloption is set as a string'false'. Ensure that thevalidateBooleanfunction expects a string value forhandleNull. If it expects a boolean, you should passfalsewithout quotes.Confirm whether
handleNullshould be a string or boolean. If it should be a boolean, update the code:- handleNull: 'false', + handleNull: false,
172-172: Check the expected value for invalid inputsThe test asserts that the
valueremains'invalid'when an invalid input is provided. Verify whether thevalidateBooleanfunction should transform invalid inputs or leave them unchanged.If invalid inputs should be transformed or set to a default value, adjust the expectation accordingly.
| const defaultMapping = config.language | ||
| ? languageMappings[config.language] | ||
| : languageMappings.en | ||
| const mapping = config.customMapping || defaultMapping |
There was a problem hiding this comment.
Handle unsupported languages in 'languageMappings'
If config.language is set to a language code that does not exist in languageMappings, defaultMapping will be undefined, leading to errors when accessing properties on mapping. To prevent this, ensure that defaultMapping falls back to a supported language or handle the unsupported language appropriately.
Apply this diff to fix the issue:
const defaultMapping = config.language
- ? languageMappings[config.language]
+ ? languageMappings[config.language] || languageMappings.en
: languageMappings.enThis change ensures that if the specified language is not supported, the default English mapping is used instead.
Alternatively, you could add an error handling mechanism for unsupported languages:
const defaultMapping = config.language
? languageMappings[config.language]
: languageMappings.en
+ if (!defaultMapping) {
+ record.addError(
+ field,
+ `Unsupported language specified: ${config.language}`
+ )
+ return
+ }
const mapping = config.customMapping || defaultMappingThis approach notifies the user that an unsupported language has been specified and prevents further processing with an undefined mapping.
📝 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.
| const defaultMapping = config.language | |
| ? languageMappings[config.language] | |
| : languageMappings.en | |
| const mapping = config.customMapping || defaultMapping | |
| const defaultMapping = config.language | |
| ? languageMappings[config.language] || languageMappings.en | |
| : languageMappings.en | |
| const mapping = config.customMapping || defaultMapping |
|
|
||
| if (normalizedValue === true || normalizedValue === false) { | ||
| record.set(field, normalizedValue) | ||
| } else if (mapping.hasOwnProperty(normalizedValue)) { |
There was a problem hiding this comment.
Use 'Object.hasOwn()' instead of 'hasOwnProperty'
To prevent potential issues when hasOwnProperty is overridden or not present in the object's prototype chain, it's recommended to use Object.hasOwn() when checking for property existence.
Apply this diff to fix the issue:
if (normalizedValue === true || normalizedValue === false) {
record.set(field, normalizedValue)
- } else if (mapping.hasOwnProperty(normalizedValue)) {
+ } else if (Object.hasOwn(mapping, normalizedValue)) {
record.set(field, mapping[normalizedValue])📝 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.
| } else if (mapping.hasOwnProperty(normalizedValue)) { | |
| } else if (Object.hasOwn(mapping, normalizedValue)) { |
🧰 Tools
🪛 Biome
[error] 87-87: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
| case 'skip': | ||
| default: |
There was a problem hiding this comment.
Remove unnecessary 'case' clause in switch statement
The 'case 'skip':' clause is unnecessary because the default case will handle any unmatched cases, making 'case 'skip':' redundant. Removing it will simplify the switch statement and clean up the code.
Apply this diff to fix the issue:
record.set(field, true)
break
- case 'skip':
default:
// Do nothing, leave the field as is
break📝 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.
| case 'skip': | |
| default: | |
| default: | |
| // Do nothing, leave the field as is | |
| break |
🧰 Tools
🪛 Biome
[error] 47-47: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
| { key: 'agreeToTerms', type: 'boolean' }, | ||
| { key: 'optIn', type: 'boolean' }, | ||
| ]) | ||
| sheetId = workbook.sheets![0].id |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using non-null assertion operator
Using the non-null assertion operator ! on workbook.sheets![0].id can lead to runtime errors if sheets is null or undefined. Consider adding a null check to ensure safety.
Update the code to include a null check:
if (workbook.sheets && workbook.sheets.length > 0) {
sheetId = workbook.sheets[0].id
} else {
throw new Error('No sheets found in the workbook')
}| expect(records[3].values['isActive'].value).toBe(false) | ||
| expect(records[4].values['isActive'].messages[0].message).toContain('Invalid boolean value') | ||
| expect(records[5].values['isActive'].messages[0].message).toContain('Invalid boolean value') | ||
|
|
||
| }) | ||
|
|
||
| it('validates truthy boolean values', async () => { | ||
| listener.use( | ||
| validateBoolean({ | ||
| fields: ['hasSubscription'], | ||
| validationType: 'truthy', | ||
| }) | ||
| ) | ||
|
|
||
| await createRecords(sheetId, [ | ||
| { hasSubscription: true }, | ||
| { hasSubscription: false }, | ||
| { hasSubscription: 'true' }, | ||
| { hasSubscription: 'false' }, | ||
| { hasSubscription: 1 }, | ||
| { hasSubscription: 0 }, | ||
| { hasSubscription: 'yes' }, | ||
| { hasSubscription: 'no' }, | ||
| ]) | ||
| await listener.waitFor('commit:created') | ||
|
|
||
| const records = await getRecords(sheetId) | ||
|
|
||
| expect(records[0].values['hasSubscription'].value).toBeTruthy() | ||
| expect(records[1].values['hasSubscription'].value).toBeFalsy() | ||
| expect(records[2].values['hasSubscription'].value).toBeTruthy() | ||
| expect(records[3].values['hasSubscription'].value).toBeFalsy() | ||
| expect(records[4].values['hasSubscription'].value).toBeTruthy() | ||
| expect(records[5].values['hasSubscription'].value).toBeFalsy() | ||
| expect(records[6].values['hasSubscription'].value).toBeTruthy() | ||
| expect(records[7].values['hasSubscription'].value).toBeFalsy() | ||
| }) | ||
|
|
||
| it('handles custom mapping', async () => { | ||
| listener.use( | ||
| validateBoolean({ | ||
| fields: ['agreeToTerms'], | ||
| validationType: 'truthy', | ||
| customMapping: { 'agreed': true, 'disagreed': false }, | ||
| }) | ||
| ) | ||
|
|
||
| await createRecords(sheetId, [ | ||
| { agreeToTerms: 'agreed' }, | ||
| { agreeToTerms: 'disagreed' }, | ||
| { agreeToTerms: 'yes' }, | ||
| ]) | ||
| await listener.waitFor('commit:created') | ||
|
|
||
| const records = await getRecords(sheetId) | ||
|
|
||
| expect(records[0].values['agreeToTerms'].value).toBeTruthy() | ||
| expect(records[1].values['agreeToTerms'].value).toBeFalsy() | ||
| expect(records[2].values['agreeToTerms'].value).toBeTruthy() | ||
| }) | ||
|
|
||
| it('handles null values', async () => { | ||
| listener.use( | ||
| validateBoolean({ | ||
| fields: ['optIn'], | ||
| validationType: 'truthy', | ||
| handleNull: 'false', | ||
| }) | ||
| ) | ||
|
|
||
| await createRecords(sheetId, [ | ||
| { optIn: null }, | ||
| { optIn: undefined }, | ||
| { optIn: '' }, | ||
| ]) | ||
| await listener.waitFor('commit:created') | ||
|
|
||
| const records = await getRecords(sheetId) | ||
|
|
||
| expect(records[0].values['optIn'].value).toBe(false) | ||
| expect(records[1].values['optIn'].value).toBe(false) | ||
| expect(records[2].values['optIn'].value).toBe(false) | ||
| }) | ||
|
|
||
| it('handles errors', async () => { | ||
| listener.use( | ||
| validateBoolean({ | ||
| fields: ['isActive'], | ||
| validationType: 'truthy', | ||
| }) | ||
| ) | ||
|
|
||
| await createRecords(sheetId, [{ isActive: 'invalid' }]) | ||
| await listener.waitFor('commit:created') | ||
|
|
||
| const records = await getRecords(sheetId) | ||
|
|
||
| expect(records[0].values['isActive'].value).toBe('invalid') | ||
| expect(records[0].values['isActive'].messages[0].message).toContain('Invalid boolean value') | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor repetitive test setup code
There is repetition in setting up the listener and creating records across the test cases. Consider abstracting common patterns into helper functions to adhere to the DRY (Don't Repeat Yourself) principle.
Create helper functions to streamline your tests:
function applyValidation(options) {
listener.use(validateBoolean(options))
}
async function createAndFetchRecords(data) {
await createRecords(sheetId, data)
await listener.waitFor('commit:created')
return await getRecords(sheetId)
}You can then refactor your tests:
it('validates strict boolean values', async () => {
applyValidation({
fields: ['isActive'],
validationType: 'strict',
})
const records = await createAndFetchRecords([
{ isActive: true },
// ...other test data
])
// assertions...
})| logSpy.mockReset() | ||
| logErrorSpy.mockReset() |
There was a problem hiding this comment.
Eliminate unnecessary mock resets
Since the console spies are not used, resetting them is unnecessary. You can remove these lines to simplify the afterEach hook.
Apply this diff to remove the unnecessary resets:
- logSpy.mockReset()
- logErrorSpy.mockReset()📝 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.
| logSpy.mockReset() | |
| logErrorSpy.mockReset() |
| const logSpy = jest.spyOn(global.console, 'log') | ||
| const logErrorSpy = jest.spyOn(global.console, 'error') |
There was a problem hiding this comment.
Remove unused console spies
The console spies logSpy and logErrorSpy are declared but not utilized in the tests. Consider removing them to clean up the code.
Apply this diff to remove the unused declarations:
- const logSpy = jest.spyOn(global.console, 'log')
- const logErrorSpy = jest.spyOn(global.console, 'error')📝 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.
| const logSpy = jest.spyOn(global.console, 'log') | |
| const logErrorSpy = jest.spyOn(global.console, 'error') |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- validate/boolean/README.MD (1 hunks)
🧰 Additional context used
🪛 LanguageTool
validate/boolean/README.MD
[uncategorized] ~50-~50: Loose punctuation mark.
Context: ...th the following properties: -fields: An array of field names to validate - `...(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (6)
validate/boolean/README.MD (6)
1-10: LGTM: Clear and concise header and infocardThe header and infocard provide a clear introduction to the plugin, including its name, a brief description, and the correct event type. This section effectively sets the context for the rest of the documentation.
11-20: LGTM: Comprehensive features listThe Features section provides a clear and concise list of the plugin's capabilities. This helps users quickly understand what the plugin offers and how it can be useful in their projects.
22-28: LGTM: Clear installation instructionsThe Installation section provides clear and concise instructions for installing the plugin using npm. The use of a code block for the command enhances readability and allows for easy copy-pasting.
30-44: LGTM: Clear and consistent example usageThe Example Usage section provides a clear demonstration of how to use the plugin with various configuration options. The use of
validateBooleanis consistent with the function name mentioned in the AI-generated summary, resolving the previous naming inconsistency issue.
60-69: LGTM: Clear and comprehensive behavior descriptionThe Behavior section provides a well-organized and easy-to-understand description of the plugin's functionality in various scenarios. This information helps users understand how the plugin will handle different inputs and configurations.
46-58:⚠️ Potential issueFix formatting and function naming inconsistency
- Remove the extra blank line at line 49 to ensure proper Markdown list formatting.
- Replace
BooleanValidatorwithvalidateBooleanto maintain consistency with the example usage and the exported function name.Apply this diff to fix the issues:
## Configuration -The `BooleanValidator` function accepts a configuration object with the following properties: +The `validateBoolean` function accepts a configuration object with the following properties: - `fields`: An array of field names to validate - `validationType`: 'strict' or 'truthy'🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: Loose punctuation mark.
Context: ...th the following properties: -fields: An array of field names to validate - `...(UNLIKELY_OPENING_PUNCTUATION)
| <!-- START_INFOCARD --> | ||
| # @flatfile/plugin-validate-boolean | ||
|
|
||
| The `@flatfile/plugin-validate-boolean` plugin provides comprehensive boolean validation capabilities, offering flexible configuration options to handle various boolean representations across different languages and use cases. | ||
|
|
||
| **Event Type:** | ||
| `listener.on('commit:created')` | ||
|
|
||
| <!-- END_INFOCARD --> | ||
|
|
||
| ## Features | ||
|
|
||
| - Supports both strict and truthy boolean validation | ||
| - Multi-language support (English, Spanish, French, German) | ||
| - Custom mapping for boolean values | ||
| - Case-sensitive and case-insensitive options | ||
| - Configurable null value handling | ||
| - Option to convert non-boolean values | ||
| - Custom error messages | ||
| - Default value setting | ||
|
|
||
| ## Installation | ||
|
|
||
| To install the plugin, run the following command: | ||
|
|
||
| ```bash | ||
| npm install @flatfile/plugin-validate-boolean | ||
| ``` | ||
|
|
||
| ## Example Usage | ||
|
|
||
| ```javascript | ||
| import { validateBoolean } from '@flatfile/plugin-validate-boolean'; | ||
|
|
||
| const booleanValidator = validateBoolean({ | ||
| fields: ['isActive', 'hasSubscription'], | ||
| validationType: 'truthy', | ||
| language: 'en', | ||
| handleNull: 'false', | ||
| convertNonBoolean: true | ||
| }); | ||
|
|
||
| listener.use(booleanValidator); | ||
| ``` | ||
|
|
||
| ## Configuration | ||
|
|
||
| The `BooleanValidator` function accepts a configuration object with the following properties: | ||
|
|
||
| - `fields`: An array of field names to validate | ||
| - `validationType`: 'strict' or 'truthy' | ||
| - `customMapping`: A custom mapping of string values to boolean | ||
| - `caseSensitive`: Whether the validation should be case-sensitive | ||
| - `handleNull`: How to handle null values ('error', 'false', 'true', or 'skip') | ||
| - `convertNonBoolean`: Whether to convert non-boolean values to boolean | ||
| - `language`: The language for predefined mappings ('en', 'es', 'fr', 'de') | ||
| - `customErrorMessages`: Custom error messages for different scenarios | ||
| - `defaultValue`: A default value to use for invalid inputs | ||
|
|
||
| ## Behavior | ||
|
|
||
| 1. **Strict Validation**: Only accepts 'true', 'false', true, or false as valid inputs. | ||
| 2. **Truthy Validation**: Accepts various representations of true/false, including language-specific terms. | ||
| 3. **Null Handling**: Configurable behavior for null or undefined values. | ||
| 4. **Non-Boolean Conversion**: Option to convert non-boolean values to boolean. | ||
| 5. **Error Handling**: Adds errors or info messages to the record for invalid inputs. | ||
| 6. **Default Value**: Option to set a default value for invalid inputs instead of raising an error. | ||
|
|
||
| The plugin can be used either as a RecordHook or as an external constraint, providing flexibility in integration with your Flatfile setup. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Suggestion: Restructure README to follow standard format
While the content of the README is comprehensive and well-organized, it doesn't follow the standard README structure used in the project. Consider restructuring the document to align with the project's conventions, which might include sections like:
- Introduction
- Installation
- Usage
- API Reference
- Configuration
- Examples
- Contributing
- License
This restructuring will improve consistency across the project's documentation and make it easier for users to find information.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: Loose punctuation mark.
Context: ...th the following properties: -fields: An array of field names to validate - `...(UNLIKELY_OPENING_PUNCTUATION)
Please explain how to summarize this PR for the Changelog:
Tell code reviewer how and what to test: