-
Notifications
You must be signed in to change notification settings - Fork 138
introduce Priority enum #447
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a Priority enum to replace integer priority values throughout the queue system, providing better type safety while maintaining backward compatibility with integer values.
Key Changes:
- Added a new
Priorityenum with 10 priority levels (Critical=1 through Idle=10) - Updated
QueuedJobentity to support bothPriorityenum and integer types for the priority property - Modified display templates to use the new
priority_intvirtual property for rendering integer values
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Model/Enum/Priority.php | New enum defining 10 priority levels with integer backing values |
| src/Model/Entity/QueuedJob.php | Added Priority enum support and priority_int virtual property accessor for display purposes |
| src/Model/Table/QueuedJobsTable.php | Configured EnumType mapping for the priority column to handle enum/int conversion |
| src/Config/JobConfig.php | Updated type annotations to accept Priority enum or int for priority methods |
| tests/TestCase/Controller/Admin/QueuedJobsControllerTest.php | Updated test assertion to verify Priority enum conversion from integer input |
| templates/Admin/QueuedJobs/view.php | Changed to use priority_int for displaying the priority value |
| templates/Admin/QueuedJobs/index.php | Changed to use priority_int for displaying the priority value |
| templates/Admin/Queue/index.php | Changed to use priority_int for displaying priority in pending and scheduled job lists |
| src/Command/JobCommand.php | Changed to use priority_int for console output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
That is indeed somewhat bc breaking enum Priority: int {
case High = 1;
case Normal = 5;
case Low = 10;
}And people can use either that for creating the options array value, or classic int for the meantime. |
|
Well I wouldn't like to have to always add |
|
Whats the use case of passing it down? Do you have some code examples? |
|
We could keep the setter for now that transforms it internally to int (until next major anyways) This way you dont have to wry about it anymore, its fully BC and you can use a speaking enum. |
|
I have no idea why that worker key is now longer than 40 chars as sha1() should always return a 40 char long string... |
|
Docs in f69cac3 |
Fixes #446
Its not really backwards compatible as several method and property types have changed. But it still allows int's being patched into it, so existing calls don't need to be adjusted.