Skip to content

remove WorkPackagePolicy #18614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 16, 2025
Merged

Conversation

ulferts
Copy link
Contributor

@ulferts ulferts commented Apr 11, 2025

Ticket

https://community.openproject.org/wp/63458

What are you trying to accomplish?

This started off as a code maintenance ticket. But while on it, a missing permission check was discovered which is now added in this PR as well. That permission check is for add_comments_with_restricted_visibility in the WorkPackages::CreateNoteContract.

The WorkPackagePolicy class no longer offers an advantage over simply calling user.allowed_to_in_project? other than providing an abstraction layer that hides the implementation details. Since the rest of the implementation has by now moved to contracts, this however requires the dev to still look into the class to understand what it is doing as it is an outlier in the code.

There exist two other policies that are not touched by this. They can be removed independently.

@ulferts ulferts force-pushed the code_maintenance/remove_work_package_policy branch 5 times, most recently from 3d077de to 9e01f62 Compare April 14, 2025 14:05
@ulferts ulferts force-pushed the code_maintenance/remove_work_package_policy branch from 9e01f62 to da2efde Compare April 14, 2025 14:16
@ulferts ulferts marked this pull request as ready for review April 16, 2025 06:50
@ulferts ulferts merged commit 56d706d into dev Apr 16, 2025
15 of 19 checks passed
@ulferts ulferts deleted the code_maintenance/remove_work_package_policy branch April 16, 2025 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants