Closed SpecLad closed 2 months ago
[!IMPORTANT]
Review skipped
Auto incremental reviews are disabled on this repository.
Please check the settings in the CodeRabbit UI or the
.coderabbit.yaml
file in this repository. To trigger a single review, invoke the@coderabbitai review
command.You can disable this status message by setting the
reviews.review_status
tofalse
in the CodeRabbit configuration file.
This update modifies the request cancellation process by changing the permission requirements for users. Users can now cancel requests without needing specific permissions tied to the original action. Additionally, the permission handling logic within the create
method has been simplified, removing detailed checks and resource retrieval related to various actions and permissions.
File Path | Change Summary |
---|---|
changelog.d/20240828_053041_roman_rm_extra_checks.md, cvat/apps/engine/permissions.py | Updated request cancellation process to allow users to cancel without specific permissions. Simplified permission handling logic in the create method, removing detailed checks and resource retrieval. |
🐇 In fields of code where bunnies play,
Permissions light the user’s way.
With simpler paths, we hop along,
Cancelling requests, where we belong.
A joyful leap, a happy cheer,
For changes bright, we hold so dear! 🌼
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?
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
Motivation and context
IMO, these checks are not very useful. The permission logic for requests already checks that the request is being canceled by the same user that created it. Therefore, these additional checks can only fail if a user creates a request for some action, loses the permissions to do the same action again, and then tries to cancel the request. But cancelling a request does not do anything to the target resource (in fact, it prevents some future actions from taking place), so I really don't see why this shouldn't be allowed.
In addition, these checks create some problems:
If the creator of the request is no longer able to cancel it, we now have a request that nobody is allowed to cancel. That seems wrong.
To implement these checks,
RequestPermission
has to know which actions require which permissions. This creates code duplication between it and the other permission classes. It also causes a dependency on those classes, which could create problems if we want to use the request API for actions from the Enterprise version.How has this been tested?
Checklist
develop
branch[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changesLicense
Summary by CodeRabbit
New Features
Bug Fixes
Chores