Components name validation #1785
Conversation
…ases for the same.
WalkthroughThe pull request introduces validation checks in the Changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
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
🧹 Nitpick comments (3)
tests/unit_test/data_processor/data_processor2_test.py (2)
51-580: Refactor tests to reduce code duplication and improve maintainability.Consider the following improvements:
- Extract common setup code into fixtures
- Use pytest's parametrize to combine similar test cases
Here's an example of how to refactor the tests:
import pytest from pytest import fixture @fixture def processor(): return MongoProcessor() @fixture def bot(): return 'test_bot' @fixture def user(): return 'test_user' @pytest.mark.parametrize("invalid_name", [ "test-name", "test.name", "test@name", "test#name", "test/name", "test>name", "test name", "test:name", "test=name", "test~name" ]) def test_add_intent_with_invalid_name(processor, bot, user, invalid_name): with pytest.raises(AppException, match=re.escape("Invalid name! Only letters, numbers, and underscores (_) are allowed.")): processor.add_intent(invalid_name, bot, user, is_integration=False)This approach:
- Reduces code duplication
- Makes it easier to add new test cases
- Improves maintainability
51-580: Add test cases for valid names and edge cases.The current test suite focuses on invalid names but lacks coverage for:
- Valid names (positive test cases)
- Edge cases such as empty strings or very long names
Consider adding these test cases:
@pytest.mark.parametrize("valid_name", [ "test_name", "test_name_123", "TEST_NAME", "test123", "_test_name" ]) def test_add_intent_with_valid_name(processor, bot, user, valid_name): # Test should pass without raising AppException processor.add_intent(valid_name, bot, user, is_integration=False) @pytest.mark.parametrize("edge_case_name", [ "", # Empty string "a" * 1000, # Very long name "_", # Single underscore "123", # Only numbers ]) def test_add_intent_with_edge_case_names(processor, bot, user, edge_case_name): # Define expected behavior for each edge case # Add appropriate assertions passkairon/shared/data/processor.py (1)
2531-2533: LGTM! Consider using a constant for the error message.The validation for special characters in intent names is correctly implemented. However, consider defining the error message "Invalid name! Only letters, numbers, and underscores (_) are allowed." as a constant since it's used across multiple validations.
INVALID_NAME_CHARACTER_ERROR = "Invalid name! Only letters, numbers, and underscores (_) are allowed."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kairon/shared/data/processor.py(20 hunks)tests/unit_test/data_processor/data_processor2_test.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python CI
🔇 Additional comments (1)
kairon/shared/data/processor.py (1)
4193-4195: LGTM! Consistent validation implementation across components.The validation for special characters is consistently implemented across all components (HTTP actions, pyscript actions, database actions, Google search actions, Zendesk actions, slots, synonyms, lookups, Pipedrive actions, Hubspot forms actions, callback actions, Razorpay actions, and schedule actions). This ensures uniform name validation throughout the system.
Also applies to: 4295-4297, 4421-4423, 4570-4572, 4704-4706, 4762-4764, 5615-5617, 6195-6197, 7126-7128, 7268-7270, 7387-7389, 8146-8148, 8502-8504
…ases for the same.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
kairon/shared/data/processor.py (2)
8014-8015: Consider moving regex import to top of file.The regex import should be moved to the top of the file with other imports for better organization.
-import reMove this import to the top of the file with other imports.
8037-8037: Consider extracting regex pattern to a constant.The regex substitution pattern could be extracted to a named constant for better maintainability.
+NAME_VALIDATION_PATTERN = r"[^a-zA-Z0-9_]" -intent = re.sub(r"[^a-zA-Z0-9_]", "_", intent) if Utility.special_match(intent) else intent +intent = re.sub(NAME_VALIDATION_PATTERN, "_", intent) if Utility.special_match(intent) else intent
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kairon/shared/data/processor.py(23 hunks)tests/unit_test/data_processor/data_processor2_test.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit_test/data_processor/data_processor2_test.py
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Python CI
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
kairon/shared/data/processor.py (3)
2531-2533: Consistent validation for intent names.The validation ensures intent names only contain letters, numbers and underscores.
4193-4195: Consistent validation pattern across all action types.The validation pattern is consistently applied across all action types (HTTP, Pyscript, Database, Google Search, Hubspot Forms, Razorpay, Callback and Schedule actions) to ensure names only contain letters, numbers and underscores.
Also applies to: 4295-4297, 4421-4423, 4570-4572, 7271-7273, 8151-8153, 8392-8394, 8507-8509
4762-4764: Consistent validation for slot, synonym, lookup and regex names.The validation ensures consistent naming rules across slots, synonyms, lookups and regex patterns.
Also applies to: 5615-5617, 6195-6197, 6138-6140
Added validations for all components names and added and fixed test cases for the same.
Summary by CodeRabbit
New Features
Bug Fixes
Tests