-
Notifications
You must be signed in to change notification settings - Fork 206
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
Enhance/dokan order status sync with wc order status #2576
base: develop
Are you sure you want to change the base?
Enhance/dokan order status sync with wc order status #2576
Conversation
WalkthroughThis update refactors how order statuses are handled across several files in the system. The changes introduce a new method to dynamically retrieve a whitelist of order statuses, modify the order counting logic to use these statuses, update status-related functions to use more explicit types and array lookups, and simplify the listing filter logic by centralizing status retrieval. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant M as Manager::count_orders
participant G as Manager::get_dokan_order_statuses
participant DB as Database
C->>M: Request order count
M->>G: Retrieve whitelisted order statuses
G-->>M: Return status list
M->>DB: Execute SQL query using status list filter
DB-->>M: Return counts
M-->>C: Return aggregated order count
sequenceDiagram
participant T as Template Caller
participant F as dokan_order_listing_status_filter
participant M as Manager::get_dokan_order_statuses
T->>F: Request order listing statuses
F->>M: Retrieve current order statuses
M-->>F: Return status list
F->>T: Return merged status list (including 'All')
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
includes/Order/Manager.php (1)
959-987
: Newget_dokan_order_statuses()
method is well-structured, but finalize the doc block
Declaring an array return type is clear. Ensure you replace@since DOKAN_SINCE
with a real version number before merge to maintain accurate documentation. Also, watch out for the pipeline complaint about multiple consecutive blank lines.Apply this diff to remove extra blank lines:
- /** * Dokan whitelisted order statuses ...
includes/Order/functions.php (1)
380-416
: Array-based mapping indokan_get_order_status_class
is clear and extensible
Switching from a switch-case to an associative array and filter hooks ('dokan_order_status_class_mapping'
) is a robust refactor. One minor UI consideration:'pending'
is mapped to'danger'
, which might be better as'warning'
, but that's subjective.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
includes/Order/Manager.php
(5 hunks)includes/Order/functions.php
(1 hunks)includes/template-tags.php
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Inspections
includes/Order/functions.php
[error] 1-1: Functions must not contain multiple empty lines in a row; found 3 empty lines.
includes/template-tags.php
[error] 1-1: Functions must not contain multiple empty lines in a row; found 3 empty lines.
includes/Order/Manager.php
[error] 1-1: Functions must not contain multiple empty lines in a row; found 3 empty lines.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: merge-reports
🔇 Additional comments (6)
includes/Order/Manager.php (4)
290-297
: Refactor for dynamic order status initialization looks solid
The new logic cleanly retrieves whitelisted statuses viaget_dokan_order_statuses()
, initializes counts witharray_fill_keys
, and appends a'total'
key. This improves extensibility and clarity compared to a static array approach.
308-309
: Ensure consistent usage of non-deleted records
Filtering out'trash'
for HPOS is consistent, but confirm no custom statuses exist (e.g.,'auto-draft'
) that also need exclusion.
323-324
: Confirm identical filtering for traditional CPT
Mirroring the'trash'
check for CPT-based orders is good. Verify that no additional statuses are needed for older installations.
342-343
: Good practice applying a final filter
Exposing$counts
to a final'dokan_order_status_count'
filter is beneficial for further custom handling.includes/template-tags.php (1)
224-226
: Simplified order status retrieval improves maintainability
Callingdokan()->order->get_dokan_order_statuses()
and prepending the'all'
status is a much cleaner approach than manually filtering statuses in this function. Nicely done.includes/Order/functions.php (1)
424-460
: Refined translation strategy for order statuses
Using an associative array and'dokan_order_status_translations'
filter offers flexibility. Returning an empty string for unknown statuses is consistent with the intent to exclude them. This change is well-structured.
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Refactor