Допускать с методами tasks.* запуск только метода call(raw=True)#245
Допускать с методами tasks.* запуск только метода call(raw=True)#245leshchenko1979 wants to merge 1 commit intomasterfrom
Conversation
… call_raw, а не call Fixes #239
Reviewer's Guide by SourceryThis pull request addresses issue #239 by extending the method restrictions in the fast_bitrix24/user_request.py file. Specifically, it updates the get_all() method to restrict all 'tasks.*' methods, adds similar restrictions to the get_by_ID() and call() methods, and enforces the use of call(raw=True) for these methods. File-Level Changes
Tips
|
There was a problem hiding this comment.
Hey @leshchenko1979 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @icontract.require( | ||
| lambda self: not self.st_method.startswith("tasks.elapseditem."), | ||
| "get_all() shouldn't be used with 'tasks.elapseditem.*' method group. " | ||
| lambda self: not self.st_method.startswith("tasks."), |
There was a problem hiding this comment.
suggestion: Consider centralizing the 'tasks.*' method group check.
The 'tasks.*' method group check is repeated in multiple places. It might be beneficial to centralize this logic in a helper function to avoid redundancy and make future updates easier.
| lambda self: not self.st_method.startswith("tasks."), | |
| def is_not_tasks_method(st_method: str) -> bool: | |
| return not st_method.startswith("tasks.") | |
| @icontract.require( | |
| lambda self: is_not_tasks_method(self.st_method), | |
| "get_all() shouldn't be used with 'tasks.*' method group. " | |
| "Use call(raw=True) instead. Read more: " | |
| ) |
| ) | ||
| @icontract.require( | ||
| lambda self: not self.st_method.startswith("tasks."), | ||
| "get_by_ID() shouldn't be used with 'tasks.*' method group. " |
There was a problem hiding this comment.
suggestion: Clarify the error message for 'tasks.*' method group.
The error message could be more specific about why 'tasks.*' method group should not be used with 'get_by_ID()'. This can help users understand the limitation better.
| "get_by_ID() shouldn't be used with 'tasks.*' method group. " | |
| "get_by_ID() shouldn't be used with 'tasks.*' method group because it may lead to unexpected behavior. " |
Fixes #239
Summary by Sourcery
This pull request addresses issue #239 by restricting the use of 'get_all()', 'get_by_ID()', and 'call(raw=False)' methods with the 'tasks.*' method group, ensuring that only 'call(raw=True)' can be used with these methods.