translation fix for action #1621
Conversation
WalkthroughThe changes in this pull request primarily focus on enhancing the handling of action configurations within the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MultilingualTranslator
participant ActionSerializer
User->>MultilingualTranslator: create_multilingual_bot()
MultilingualTranslator->>ActionSerializer: serialize()
ActionSerializer-->>MultilingualTranslator: serialized actions
MultilingualTranslator->>MultilingualTranslator: __save_bot_files()
MultilingualTranslator->>User: Bot created successfully
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: 1
🧹 Outside diff range and nitpick comments (2)
tests/unit_test/multilingual/multilingual_processor_test.py (2)
675-675: Remove unused variableexpectedThe variable
expectedis assigned but never used, which can be removed to clean up the code.Apply this diff:
action_config = { "http_action": [{"response": {}}], "email_action": [{}], } - expected = { - "http_action": [{"response": {}}], - "email_action": [{}], - } s_lang = "en" d_lang = "fr"🧰 Tools
🪛 Ruff (0.8.0)
675-675: Local variable
expectedis assigned to but never usedRemove assignment to unused variable
expected(F841)
683-683: Remove unused variableresultThe variable
resultis assigned but never used. Since we're only checking that an exception is raised, the assignment can be removed.Apply this diff:
with pytest.raises(KeyError): - result = MultilingualTranslator._MultilingualTranslator__translate_actions(action_config, s_lang, d_lang) + MultilingualTranslator._MultilingualTranslator__translate_actions(action_config, s_lang, d_lang)🧰 Tools
🪛 Ruff (0.8.0)
683-683: Local variable
resultis assigned to but never usedRemove assignment to unused variable
result(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
kairon/multilingual/processor.py(5 hunks)tests/unit_test/multilingual/multilingual_processor_test.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.0)
tests/unit_test/multilingual/multilingual_processor_test.py
675-675: Local variable expected is assigned to but never used
Remove assignment to unused variable expected
(F841)
682-682: pytest.raises(Exception) should be considered evil
(B017)
683-683: Local variable result is assigned to but never used
Remove assignment to unused variable result
(F841)
🔇 Additional comments (6)
kairon/multilingual/processor.py (6)
12-12: Importing ActionSerializer for action serialization
The addition of ActionSerializer improves the way actions are serialized and deserialized, enhancing code modularity and maintainability.
128-145: Enhance robustness by checking action types before translation
The added checks for the existence of specific action types in the action_config dictionary before translation prevent potential KeyError exceptions and improve code reliability.
159-164: Use ActionSerializer.deserialize for action deserialization
Replacing the direct call to save_integrated_actions with ActionSerializer.deserialize standardizes the action deserialization process and promotes reuse of serialization logic.
200-201: Serialize actions using ActionSerializer.serialize
Utilizing ActionSerializer.serialize for serializing actions and other collections ensures consistency and encapsulates serialization logic within a dedicated class.
233-233: Pass other_collections to __save_bot_files
Including other_collections when saving bot files ensures that all necessary collections are preserved in the new multilingual bot.
150-150: 🛠️ Refactor suggestion
Update method signature to include other_collections
The method __save_bot_files now accepts an additional parameter other_collections. Ensure that all calls to this method have been updated accordingly throughout the codebase.
Run the following script to verify all calls to __save_bot_files are updated:
✅ Verification successful
Note: Corrected the syntax by using -t py instead of --python flag for ripgrep to search Python files.
All calls to __save_bot_files are properly updated
The verification shows that there is only one call to __save_bot_files in the codebase, and it correctly includes the other_collections parameter. The method call matches the updated signature perfectly, so no further changes are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of __save_bot_files and check for the correct parameters
rg --python --multiline 'def __save_bot_files\(.*\):' -A3
# Find all calls to __save_bot_files
rg --python 'self\.__save_bot_files\(' -A5
Length of output: 926
Script:
#!/bin/bash
# Description: Find all usages of __save_bot_files and check for the correct parameters
# Find method definition
rg -t py 'def __save_bot_files.*:' -A3
# Find all calls to __save_bot_files
rg -t py 'self\.__save_bot_files\(' -A5
Length of output: 1028
kairon/multilingual/processor.py
Outdated
| actions = actions, | ||
| other_collections_data=other_collections, | ||
| overwrite=True) | ||
| # self.mp.save_integrated_actions(actions=actions, bot=new_bot_id, user=self.user) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
kairon/multilingual/processor.py (2)
128-145: Consider refactoring repetitive action translation logicThe current implementation has repetitive patterns for each action type. Consider refactoring this into a more maintainable structure.
Here's a suggested refactor to reduce duplication:
- if action_config.get(ActionType.http_action.value): - action_config[ActionType.http_action.value] = translate_responses_for_http_actions( - action_config[ActionType.http_action.value]) - if action_config.get(ActionType.email_action.value): - action_config[ActionType.email_action.value] = translate_responses(action_config[ActionType.email_action.value]) - # ... more similar conditions ... + action_translation_config = { + ActionType.http_action.value: {'translator': translate_responses_for_http_actions}, + ActionType.email_action.value: {'translator': translate_responses}, + ActionType.jira_action.value: {'translator': translate_responses}, + ActionType.zendesk_action.value: {'translator': translate_responses}, + ActionType.pipedrive_leads_action.value: {'translator': translate_responses}, + ActionType.google_search_action.value: { + 'translator': translate_responses, + 'fields': ['failure_response'] + }, + ActionType.form_validation_action.value: { + 'translator': translate_responses, + 'fields': ['valid_response', 'invalid_response'] + } + } + + for action_type, config in action_translation_config.items(): + if action_config.get(action_type): + translator_func = config['translator'] + fields = config.get('fields') + if fields: + action_config[action_type] = translator_func( + action_config[action_type], fields) + else: + action_config[action_type] = translator_func( + action_config[action_type])
159-163: Fix indentation in ActionSerializer.deserialize callThe indentation of the parameters in the deserialize call is inconsistent.
- ActionSerializer.deserialize(bot=new_bot_id, - user=self.user, - actions = actions, - other_collections_data=other_collections, - overwrite=True) + ActionSerializer.deserialize( + bot=new_bot_id, + user=self.user, + actions=actions, + other_collections_data=other_collections, + overwrite=True + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
kairon/multilingual/processor.py(5 hunks)
🔇 Additional comments (2)
kairon/multilingual/processor.py (2)
12-12: LGTM: Import addition is appropriate
The ActionSerializer import is correctly placed and aligns with the module's requirements.
199-199: LGTM: Improved bot creation process
The changes to use ActionSerializer and handle other collections improve the robustness of the bot creation process. The error handling and logging are comprehensive.
Also applies to: 231-231
Summary by CodeRabbit
New Features
Bug Fixes
Tests