feat(new-form-builder): updated the regex for initial fields to not allow empty string #1888
Conversation
…llow empty string
sandeepdsvs
requested review from
lvrach,
cisse21,
debanjan97,
am6010,
ssbeefeater,
ItsSudip,
krishna2020,
1abhishekpandey,
RanjeetMishra,
saikumarrs and
a team
as code owners
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces a new Python script for validating JSON UI configuration files and makes extensive updates across multiple JSON schemas and UI configuration files for various destinations. The updates primarily tighten regex validations by enforcing a non-empty string requirement and appropriate length restrictions. Several fields, such as API keys, endpoints, tokens, and other configuration properties, now require at least one character, and in some cases, optionality has been adjusted. Test configurations have also been updated to reflect these new validation rules. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Validator as validate_all_configs
participant FileValidator as validate_ui_config_file
User->>Validator: Provide base path for config validation
Validator->>FileValidator: Iterate over each ui-config.json
FileValidator-->>Validator: Return status and error details
Validator-->>User: Present summary of valid, invalid, and old configurations
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 0
🔭 Outside diff range comments (1)
src/configurations/destinations/smartly/schema.json (1)
37-41: [Refactor Suggestion – Essential] Update "eventName" regex to disallow empty strings.
Currently, the regex for the eventName property is:"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$"This allows an empty string. If the intent is to enforce non-empty initial fields consistently, consider changing
{0,100}to{1,100}. For example:-"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$" +"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$"Please confirm if all initial fields should reject empty strings.
🧹 Nitpick comments (4)
scripts/validateRegex.py (2)
87-88: Catch narrower exceptions to aid debugging.
Catching broad exceptions can obscure underlying causes and make debugging harder. Narrowing the exception types helps ensure that unexpected errors bubble up clearly.- except Exception as e: + except (OSError, ValueError, json.JSONDecodeError) as e:
101-101: Rename unused loop variable to underscore.
Thedirsloop variable is never used, so rename it to_dirsto conform with standard Python conventions.-for root, dirs, files in os.walk(base_path): +for root, _dirs, files in os.walk(base_path):🧰 Tools
🪛 Ruff (0.8.2)
101-101: Loop control variable
dirsnot used within loop bodyRename unused
dirsto_dirs(B007)
src/configurations/destinations/smartly/schema.json (2)
56-64: [Refactor Suggestion – Good to Have] Consider consistent non-empty validation for "oneTrustCookieCategory".
The properties under oneTrustCookieCategories for various platforms are still using the pattern with{0,100}, which permits empty strings. If the same non-empty requirement applies here, update the regex accordingly.
For instance:-"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$" +"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$"
211-214: [Refactor Suggestion – Good to Have] Check if the "consent" field should also disallow empty strings.
Within the consentManagement section, the "consent" property currently uses:"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$"If initial field validation requires a non-empty value, consider updating it to use
{1,100}.-"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$" +"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
scripts/validateRegex.py(1 hunks)src/configurations/destinations/active_campaign/schema.json(1 hunks)src/configurations/destinations/active_campaign/ui-config.json(1 hunks)src/configurations/destinations/bloomreach/schema.json(1 hunks)src/configurations/destinations/bloomreach/ui-config.json(1 hunks)src/configurations/destinations/bloomreach_catalog/schema.json(1 hunks)src/configurations/destinations/bloomreach_catalog/ui-config.json(1 hunks)src/configurations/destinations/cordial/schema.json(1 hunks)src/configurations/destinations/cordial/ui-config.json(1 hunks)src/configurations/destinations/facebook_conversions/schema.json(1 hunks)src/configurations/destinations/facebook_conversions/ui-config.json(1 hunks)src/configurations/destinations/fb_custom_audience/schema.json(2 hunks)src/configurations/destinations/fb_custom_audience/ui-config.json(2 hunks)src/configurations/destinations/fullstory/schema.json(2 hunks)src/configurations/destinations/fullstory/ui-config.json(2 hunks)src/configurations/destinations/ga4/schema.json(2 hunks)src/configurations/destinations/ga4/ui-config.json(2 hunks)src/configurations/destinations/google_adwords_remarketing_lists/schema.json(2 hunks)src/configurations/destinations/google_adwords_remarketing_lists/ui-config.json(3 hunks)src/configurations/destinations/http/schema.json(1 hunks)src/configurations/destinations/http/ui-config.json(1 hunks)src/configurations/destinations/intercom/ui-config.json(1 hunks)src/configurations/destinations/movable_ink/schema.json(1 hunks)src/configurations/destinations/movable_ink/ui-config.json(1 hunks)src/configurations/destinations/optimizely_fullstack/schema.json(1 hunks)src/configurations/destinations/optimizely_fullstack/ui-config.json(1 hunks)src/configurations/destinations/rakuten/schema.json(1 hunks)src/configurations/destinations/rakuten/ui-config.json(1 hunks)src/configurations/destinations/smartly/schema.json(1 hunks)src/configurations/destinations/smartly/ui-config.json(1 hunks)src/configurations/destinations/tune/schema.json(1 hunks)src/configurations/destinations/tune/ui-config.json(1 hunks)test/data/validation/destinations/fullstory.json(1 hunks)test/data/validation/destinations/ga4.json(1 hunks)test/data/validation/destinations/movable_ink.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/configurations/destinations/bloomreach/ui-config.json
- src/configurations/destinations/movable_ink/ui-config.json
- src/configurations/destinations/bloomreach_catalog/ui-config.json
- src/configurations/destinations/bloomreach_catalog/schema.json
🧰 Additional context used
🪛 Ruff (0.8.2)
scripts/validateRegex.py
101-101: Loop control variable dirs not used within loop body
Rename unused dirs to _dirs
(B007)
🔇 Additional comments (51)
scripts/validateRegex.py (1)
69-77: Ensure this logic aligns with all textInput fields.
This check correctly flags regexes that permit empty strings, which matches the PR objective to disallow empty values. However, confirm that disallowing empty values is indeed desired for every "textInput" field across all configurations.src/configurations/destinations/rakuten/ui-config.json (1)
19-19: Regex update ensures no empty strings.
Changing^(.{0,100})$to^(.{1,100})$prevents empty Merchant IDs, aligning with the PR goal.src/configurations/destinations/cordial/ui-config.json (1)
34-34: Regex now prohibits empty strings.
This update removes the possibility of empty strings for the "API Base URL," consistent with the PR objective.src/configurations/destinations/active_campaign/ui-config.json (2)
28-29: Regex Update for API Key Field
The API Key field’s regex has been updated to"^(.{1,100})$", which enforces that any entered API Key must be at least 1 character long and no more than 100 characters. This effectively disallows empty strings, aligning with the requirement to prevent blank inputs.
25-33: Optional Field vs. Validation Consistency
According to the PR objectives, the "API Key" field’srequiredattribute was intended to change fromtruetofalse. Combined with the new regex which disallows empty strings, please verify that this design meets your intended use case. In other words, if the field is optional, ensure that an empty submission bypasses regex validation, or that the intended behavior is correctly communicated to users.src/configurations/destinations/google_adwords_remarketing_lists/schema.json (2)
11-14: Customer ID Regex Enhancement
ThecustomerIdproperty now uses the regex"(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$". This change ensures that the field cannot be an empty string and restricts the length to 1–100 characters, which enhances data integrity.
268-313: Conditional Validation with allOf Block
A newallOfblock has been added that introduces conditional validation:
- If
connectionModeis set to"cloud", anaudienceIdmust be provided and conform to the updated regex.- Similarly, if
subAccountistrue, thenloginCustomerIdmust be supplied, matching the new pattern.
This logic effectively enforces the desired field dependencies. Please review that these conditions cover all necessary scenarios and that downstream components (such as UI configurations) are consistent with these requirements.src/configurations/destinations/smartly/ui-config.json (2)
20-21: Api Token Regex Upgrade
The regex for the "Api Token" field has been updated to"(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$". This change ensures that the token, if provided, is not empty and does not exceed 100 characters. This enhancement is consistent with our new non-empty string validation policy.
187-203: Consistent Validation for Event Mapping Fields
The mapping fields for "RudderStack Event Name" and "Smartly Event Name" now use the same updated regex, ensuring that any event name entered must be between 1 and 100 characters. This consistency across the configuration helps maintain uniform validation rules.test/data/validation/destinations/movable_ink.json (1)
18-19: Updated Endpoint Validation Error Message
The error message for the invalidendpointvalue now reflects the updated regex pattern accurately. This ensures that when an endpoint like"abc"is provided, the feedback clearly indicates the required format. Please verify that all test cases cover the new validation behavior and that error messaging remains clear.src/configurations/destinations/google_adwords_remarketing_lists/ui-config.json (3)
16-22: Customer ID Regex Consistency
The regex for the "Customer ID" field has been updated to"(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$", ensuring that the field cannot be empty and is limited to 100 characters maximum. This change aligns with the new non-empty input requirement.
25-32: List ID (audienceId) Regex Update
The "List Id" field now uses the updated regex to prevent empty strings and limit input length. This adjustment ensures that the Google remarketing list ID conforms to the same validation rules as the Customer ID, thereby enhancing data consistency.
47-53: Login Customer ID Regex Enhancement
For users operating in sub-account mode, the "Login Customer ID" field has been updated to enforce the regex"(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{1,100})$". This ensures that when provided, the manager account’s customer ID meets the non-empty and length criteria.src/configurations/destinations/fullstory/ui-config.json (2)
19-21: Enforce non-empty API Key.
The regex for the API Key has been updated to require at least one character (changed to^(.{1,200})$), which prevents empty string inputs and strengthens data validation.
75-80: Enforce non-empty FS ORG.
The updated regex (^(.{1,100})$) now mandates a minimum of one character for the FS ORG field. This change ensures that users cannot leave the field empty and provides more robust validation.src/configurations/destinations/fb_custom_audience/schema.json (2)
388-391: Require non-empty Audience Id.
The regex for theaudienceIdproperty has been updated to enforce a minimum of one character. This change prevents empty string inputs and aligns with the new validation standards.
412-415: Require non-empty Ad Account Id.
The regex update for theadAccountIdfield (now^(.{1,100})$) ensures that this field cannot be empty, improving the integrity of the configuration data.src/configurations/destinations/tune/ui-config.json (1)
20-22: Enforce non-empty Subdomain.
The regex for the Subdomain field has been updated to require at least one character (^(.{1,100})$), along with a clear error message ("Invalid Subdomain"), to ensure that the input is not left empty.test/data/validation/destinations/fullstory.json (2)
532-563: Add test case: Cloud connection with non-native SDK.
A new test case has been introduced where bothuseNativeSDK.webanduseNativeSDK.androidare set to false and the connection mode is set to "cloud". This configuration validates correctly and increases coverage for scenarios using cloud settings.
564-595: Add test case: Device connection with native SDK enabled.
This test case checks that when bothwebandandroidconnection modes are set to "device" anduseNativeSDKis true, the configuration passes validation. This ensures that alternative connection setups are properly supported.src/configurations/destinations/intercom/ui-config.json (1)
92-96: Enforce non-empty App Id.
The regex for the App Id field has been updated to^(.{1,100})$, which now requires at least one character. This ensures that an invalid (empty) App Id cannot be submitted and aligns with the overall stricter validation rules.src/configurations/destinations/facebook_conversions/ui-config.json (1)
29-29: Regex Update for Business Access Token: Enforcing non-empty value.
The regex pattern has been modified from allowing an empty string (.{0,500}) to requiring at least one character (.{1,500}), ensuring that the Business Access Token cannot be empty.src/configurations/destinations/ga4/ui-config.json (2)
20-20: Regex Update for API Secret: Minimum character requirement set.
The regex for the API Secret field now enforces a minimum of one character (changed from allowing empty strings), which improves input validation.
53-53: Regex Update for Measurement Id: Require non-empty value.
The regex for the Measurement Id has been updated to require at least one character (G-.{1,100}) instead of allowing zero characters. This change prevents empty inputs for this critical field.test/data/validation/destinations/ga4.json (1)
80-81: Test Case Update: Measurement Id validation error message.
The expected error message now reflects the updated regex pattern that enforces a non-empty Measurement Id. Please verify that this test case correctly captures the stricter validation behavior.src/configurations/destinations/http/ui-config.json (3)
53-53: Regex Update for Username: Non-empty string enforced.
The regex has been updated to require at least one character (.{1,100}), ensuring that the Username field is not submitted empty.
86-86: Regex Update for Token: Enforcing non-empty value.
The updated regex for the Token field now demands a minimum of one character instead of allowing an empty token.
117-117: Regex Update for Value Field: Minimum character requirement enforced.
The regex for the API Key Value has been changed to require at least one character (.{1,100}) so that empty inputs are rejected.src/configurations/destinations/fb_custom_audience/ui-config.json (2)
30-30: Regex Update for Ad Account id: Non-empty requirement enforced.
The regex for the Ad Account id field is now updated to require at least one character (.{1,100}), preventing the field from being left blank.
47-48: Regex and Error Message Update for Audience Id: Enforcing non-empty validation.
The Audience Id field’s regex has been updated to require a non-empty value and a clear error message ("Invalid Audience id") has been added to improve feedback when validation fails.src/configurations/destinations/rakuten/schema.json (2)
9-9: Updated regex for the "mid" field enforces non-empty values.
The modified regex now requires the string to have at least one character (using^(.{1,100})$), which meets the PR objective of preventing empty strings.
11-24: Verify consistency across nested fields.
While the "mid" field has been updated to enforce a minimum length of 1, the nested fields under "oneTrustCookieCategories" (e.g. for "oneTrustCookieCategory") still use the pattern^(.{0,100})$, which continues to allow empty strings. Please confirm if these nested fields should be similarly tightened to require at least one character.src/configurations/destinations/cordial/schema.json (1)
13-13: Regex update for "apiBaseUrl" field.
The updated regex removes the portion that allowed empty strings (by eliminating|^$), ensuring that a valid URL or dynamic expression is required. This change aligns with the PR’s objective to prevent empty strings in initial fields.src/configurations/destinations/movable_ink/schema.json (1)
9-9: Improved regex for "endpoint" field disallows empty strings.
By removing the allowance for an empty string, the pattern now ensures that the "endpoint" must be a valid non-empty URL or dynamic value. This update meets the intended validation tightening.src/configurations/destinations/active_campaign/schema.json (1)
13-13: Tightened regex for "apiKey" field.
The new regex pattern now requires the "apiKey" to be between 1 and 100 characters (^(.{1,100})$), effectively disallowing empty string values while preserving the maximum length restriction.src/configurations/destinations/bloomreach/schema.json (1)
9-9: Refined regex for "apiBaseUrl" field prevents empty strings.
The updated regex removes the trailing empty string allowance (i.e. the|^$portion) so that an empty value is no longer considered valid. This change is consistent with the overall objective of the PR.src/configurations/destinations/smartly/schema.json (2)
7-10: [Approved] Correct update for the "apiToken" property.
The new regex now enforces a minimum of one character (using^(.{1,100})$), which meets the objective of not allowing empty strings.
16-23: [Approved] Validation for "from" and "to" in eventsMapping is on point.
The regex has been updated to require at least one character (^(.{1,100})$), aligning with the non–empty string requirement.src/configurations/destinations/tune/schema.json (2)
7-10: [Approved] "subdomain" pattern correctly updated.
The "subdomain" property now uses the regex requiring at least one character (^(.{1,100})$), which meets the PR objective.
11-14: [Verify] Consider whether the "eventName" field should disallow empty strings.
Currently, the "eventName" property remains defined with the regex:"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$"If all initial fields must be non-empty, updating this to
{1,100}might be appropriate.src/configurations/destinations/facebook_conversions/schema.json (1)
11-14: [Approved] "accessToken" regex update is correct.
The regex now requires at least one character (^(.{1,500})$), which effectively prevents empty values.src/configurations/destinations/optimizely_fullstack/schema.json (1)
7-10: [Approved] "dataFileUrl" regex tightened to disallow empty strings.
By removing the trailing allowance for an empty string (i.e. removing|^$), the validation now only accepts valid URLs (or environment/templated variables) and rejects empty strings.src/configurations/destinations/ga4/schema.json (2)
7-10: [Approved] "apiSecret" pattern update is solid.
Changing the regex to require a minimum of one character (^(.{1,100})$) is in line with the PR objective.
956-962: [Approved] "measurementId" regex now correctly disallows empty strings.
The updated pattern"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(G-.{1,100})$"ensures that the measurementId must start with “G-” and be followed by at least one character beyond that prefix, preventing empty inputs.
src/configurations/destinations/fullstory/schema.json (2)
975-979: Non-Empty API Key Validation
The regex for theapiKeyfield has been updated to require at least one character (i.e.^(.{1,200})$). This change ensures that empty strings are no longer accepted and improves overall data integrity. Please verify that this stricter pattern still correctly handles templated values (using{{ }}syntax) and environment variable prefixes.
996-1007: Strengthen fs_org Field Validation
The pattern for thefs_orgfield is now^(.{1,100})$, which enforces a minimum of one character. This update prevents empty string entries and aligns the validation with the intended requirements. Ensure that any pre-existing environment variable or templated inputs continue to match appropriately.src/configurations/destinations/http/schema.json (4)
967-969: Enforce Non-Empty Username
The regex for theusernamefield has been updated from allowing empty strings to requiring at least one character (^(.{1,100})$). This ensures that a valid username is always provided. Verify that clients using templated inputs or environment variables continue to be supported by this change.
1004-1008: Require Non-Empty Bearer Token
The updated regex for thebearerTokenfield is now^(.{1,255})$, meaning it no longer accepts empty strings. This stricter validation enhances security and data consistency. Please confirm that this adjustment does not break any existing integration flows that depend on alternative token formats.
1021-1029: Non-Empty API Key Name Requirement
TheapiKeyNamefield now uses the regex^(\\S{1,100})$, ensuring that there is at least one non-whitespace character. This update prevents accidental submissions of blank API key names and improves input validation consistency.
1041-1048: Enforce Non-Empty API Key Value
The regex for theapiKeyValuefield has been updated to^(.{1,100})$, disallowing empty string entries. Please verify that this change preserves compatibility with existing valid API key values and does not obstruct any valid templated inputs.src/configurations/destinations/optimizely_fullstack/ui-config.json (1)
26-26: Tightened Datafile URL Validation
Theregexfor the "Datafile URL" field has been modified to remove the allowance for an empty string. Now, only valid URLs (or values matching the templated/environment variable formats) are accepted. Ensure that this stricter rule is consistent with the schema and that it does not inadvertently reject legitimate values provided via templating.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## hotfix/feb.04 #1888 +/- ##
===============================================
Coverage 100.00% 100.00%
===============================================
Files 2 2
Lines 53 53
Branches 7 7
===============================================
Hits 53 53 ☔ View full report in Codecov by Sentry. |
What are the changes introduced in this PR?
Updated the regex for initial fields to not allow empty string
What is the related Linear task?
Resolves INT-3172
Please explain the objectives of your changes below
Put down any required details on the broader aspect of your changes. If there are any dependent changes, mandatorily mention them here
Any changes to existing capabilities/behaviour, mention the reason & what are the changes ?
N/A
Any new dependencies introduced with this change?
N/A
Any new checks got introduced or modified in test suites. Please explain the changes.
N/A
Developer checklist
My code follows the style guidelines of this project
No breaking changes are being introduced.
All related docs linked with the PR?
All changes manually tested?
Any documentation changes needed with this change?
I have executed schemaGenerator tests and updated schema if needed
Are sensitive fields marked as secret in definition config?
My test cases and placeholders use only masked/sample values for sensitive fields
Is the PR limited to 10 file changes & one task?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
New Features
Enhancements
Tests