Collection in Database action #1104
Conversation
2. Fixed test cases.
WalkthroughWalkthroughThe update introduces a new Changes
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? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (7)
- kairon/api/models.py (2 hunks)
- kairon/shared/actions/data_objects.py (1 hunks)
- kairon/shared/cognition/processor.py (2 hunks)
- kairon/shared/data/processor.py (3 hunks)
- tests/integration_test/action_service_test.py (12 hunks)
- tests/integration_test/services_test.py (18 hunks)
- tests/unit_test/data_processor/data_processor_test.py (22 hunks)
Additional comments: 44
kairon/shared/cognition/processor.py (2)
9-9: The addition of the
DatabaseActionimport is correct and necessary for the subsequent use within the file.247-254: The changes to the
validate_collection_namemethod correctly introduce a check forDatabaseActionobjects associated with the collection, preventing the removal of a collection that is linked to a database action.kairon/shared/actions/data_objects.py (1)
- 200-204: The removal of the
cleanmethod from theDataObjectclass alters the data cleaning logic. Verify that this change does not negatively impact data validation and cleaning processes elsewhere in the system.kairon/api/models.py (1)
- 406-411: The addition of the
collectionfield to theDatabaseActionRequestclass is consistent with the PR's objective to enhance database action functionality. Ensure that thecollectionfield is being utilized whereverDatabaseActionRequestinstances are created or processed.kairon/shared/data/processor.py (3)
97-97: Added import for
CognitionSchemafrom..cognition.data_objects.3214-3224: In the
update_db_actionmethod, ensure that theCognitionSchemawith the given collection name exists before proceeding to update thecollectionattribute of theDatabaseAction. This helps maintain data integrity by linking actions to valid collections.3239-3251: In the
add_db_actionmethod, ensure that theCognitionSchemawith the given collection name exists before proceeding to add a newDatabaseAction. This helps maintain data integrity by linking actions to valid collections.tests/integration_test/action_service_test.py (12)
2893-2893: The addition of the
collectionparameter to theDatabaseActionconstructor is consistent with the PR objectives to enhance database action functionality. Ensure that thecollectionvalue used here is valid and exists in the database.2902-2902: The update to the
http_urlvariable reflects the newcollectionparameter. Verify that the updated URL is correct and the server is configured to handle requests to this new endpoint.2985-2985: The addition of the
collectionparameter with a different value suggests testing against various collections. Verify that this collection name is also valid and exists.2994-2994: The
http_urlupdate is consistent with the newcollectionparameter. Verify the correctness of the updated URL as with the previous hunk.3061-3061: The
collectionparameter is added with yet another unique value, indicating a thorough test coverage for different collections. Verify the existence and validity of this collection as well.3070-3070: The
http_urlis updated to match the newcollectionparameter. As before, ensure the updated URL is correct and reachable.3147-3147: The addition of the
collectionparameter here is consistent with the pattern observed in other hunks. Verify the collection name for validity.3156-3156: The
http_urlupdate matches the newcollectionparameter. Verify the correctness of the updated URL.3220-3220: The
collectionparameter is added, which is in line with the PR objectives. Verify the collection name for validity.3230-3230: The
http_urlis updated to reflect the newcollectionparameter. Verify the correctness of the updated URL.3305-3305: The
collectionparameter is added, but thequery_typeis set to a string rather than using theDbActionOperationTypeenum. This could be an inconsistency or error. Verify ifquery_typeshould be an enum value for consistency with other test cases.3366-3366: The
collectionparameter is added consistently with other test cases. Verify the collection name for validity.tests/integration_test/services_test.py (20)
7259-7259: The addition of the "collection" field in the request body is consistent with the PR's objective to enhance database actions. Ensure that the test case
test_add_vectordb_action_empty_nameis updated to reflect the new field requirement.7277-7281: The test case
test_add_vectordb_action_empty_collection_namecorrectly checks for an empty collection name. However, ensure that the test case name is unique and not a duplicate as mentioned in the AI-generated summary.7294-7294: The assertion for the error message is correct and aligns with the expected validation for the "collection" field. This ensures that the error messages are descriptive and helpful for debugging.
7314-7317: The assertions for the error message in
test_add_vectordb_action_empty_operation_valueare specific and check for the correct enumeration values, which is a good practice for validation tests.7349-7349: The test case
test_add_vectordb_action_empty_payload_valueis updated to include the "collection" field. Ensure that the test case is checking for the correct behavior when the payload value is empty.7367-7392: The test case
test_add_vectordb_action_collection_does_not_existsis well-structured to check the scenario where the collection does not exist. The assertions for the error code and message are appropriate.7364-7426: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [7395-7457]
The test case
test_add_vectordb_actionincludes a mock setup and a sequence of API calls to test the addition of a vectordb action. The assertions check for both success and failure scenarios, which is a comprehensive approach.
7460-7463: The test case
test_add_vectordb_action_case_insensitivityshould verify that the collection name is case-insensitive. Ensure that the test case is designed to validate this behavior.7509-7509: The test case
test_add_vectordb_action_existingis updated to include the "collection" field. Verify that the test case is intended to check the behavior when adding an existing vectordb action.7553-7553: The test case
test_add_vectordb_action_with_slotsincludes the "collection" field. Ensure that the test case is checking for the correct behavior when using slots in the payload.7574-7574: The test case
test_update_vectordb_actionis updated to include the "collection" field. Verify that the test case is intended to check the update functionality for vectordb actions.7597-7597: The test case
test_update_vectordb_actionis updated to include the "collection" field. Verify that the test case is intended to check the update functionality for vectordb actions.7625-7625: The test case
test_update_vectordb_action_non_existingis updated to include the "collection" field. Verify that the test case is intended to check the update functionality for non-existing vectordb actions.7640-7640: The test case
test_update_vectordb_action_non_existingis updated to include the "collection" field. Verify that the test case is intended to check the update functionality for non-existing vectordb actions.7661-7661: The test case
test_update_vector_action_wrong_parameteris updated to include the "collection" field. Verify that the test case is intended to check the behavior when an incorrect parameter is used in the vectordb action.7678-7678: The test case
test_update_vector_action_wrong_parameteris updated to include the "collection" field. Verify that the test case is intended to check the behavior when an incorrect parameter is used in the vectordb action.7726-7726: The assertion in
test_list_vector_db_actionchecks for the correct name of the vectordb action. This is a good practice to ensure that the list functionality is working as expected.7730-7732: The test case
test_delete_vectordb_actionis updated to include the "collection" field. Verify that the test case is intended to check the delete functionality for vectordb actions.7765-7765: The test case
test_delete_vectordb_action_non_existingis updated to include the "collection" field. Verify that the test case is intended to check the delete functionality for non-existing vectordb actions.8646-8649: The assertion in
test_list_actionschecks for the correct names of vectordb actions. This is a good practice to ensure that the list functionality is working as expected and that the new "collection" field is being considered.tests/unit_test/data_processor/data_processor_test.py (5)
9595-9596: The introduction of both
CognitionDataProcessorandMongoProcessormay indicate a transition phase or compatibility layer. Ensure that the use of both processors is intentional and that their respective roles within the tests are clear and justified.9629-9629: The
add_db_actionmethod is called onprocessor_two, which is an instance ofMongoProcessor. Confirm that this is consistent with the intended usage ofMongoProcessorin the context of the newCognitionDataProcessorchanges.9638-9640: The test case correctly expects an
AppExceptionwhen trying to delete a schema linked to an action. Ensure that the message in thematchparameter ofpytest.raisesis consistent with the actual error message produced by the system for such cases.9679-9688: The creation of
CognitionSchemaandCognitionDatawith the newcollectionfield appears consistent across the hunks. Ensure that thecollectionfield is being used as intended and that the test cases cover all necessary scenarios related to this new field.9786-9787: The test case correctly expects an
AppExceptionwhen adding a database action to a non-existent collection. Verify that the error message in thematchparameter ofpytest.raisesaligns with the actual error message for non-existent collections.
| @validator("collection") | ||
| def validate_collection_name(cls, v, values, **kwargs): | ||
| from kairon.shared.utils import Utility | ||
|
|
||
| if Utility.check_empty_string(v): | ||
| raise ValueError("collection is required") | ||
| return v |
There was a problem hiding this comment.
The validate_collection_name method correctly checks for an empty collection string. However, consider also validating the format of the collection name if there are specific naming conventions or restrictions in the database system being used.
@validator("collection")
def validate_collection_name(cls, v, values, **kwargs):
from kairon.shared.utils import Utility
if Utility.check_empty_string(v):
raise ValueError("collection is required")
# Example additional validation for format (if applicable)
if not re.match("^[a-zA-Z0-9_]+$", v):
raise ValueError("collection name contains invalid characters")
return v| @@ -9865,6 +9965,7 @@ def test_get_vector_embedding_action_does_not_exists(self): | |||
| payload = {'type': 'from_slot', 'value': 'email'} | |||
| DatabaseAction( | |||
| name=action, | |||
| collection='test_get_vector_embedding_action_does_not_exists', | |||
There was a problem hiding this comment.
This test case seems to be missing the assertion or exception handling for the creation of a DatabaseAction with a non-existent collection. Ensure that the test case is complete and accurately reflects the expected behavior when dealing with non-existent collections.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests