enhance: Add additional tax fields and new order types for Dokan order stats table.#3099
Conversation
📝 WalkthroughWalkthroughThis change implements an analytics migration for version 4.2.10, introducing four new order types (advertisement product/refund, subscription/refund), extending the order stats table with vendor and admin tax columns, and adding migration logic to safely alter the database schema and regenerate analytics data. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@includes/Upgrade/Upgrades/V_4_2_10.php`:
- Around line 9-14: The docblock `@since` tags in this file are incorrect: they
say "3.15.0" while the upgrade class is V_4_2_10 and registered as version
4.2.10; update the file-level and any method-level docblocks inside this file to
use "@since 4.2.10" (search for every occurrence of "3.15.0" in this file and
replace it), ensuring the class declaration V_4_2_10 and any related methods'
docblocks reflect the correct 4.2.10 version.
- Around line 42-48: The upgrade script's $columns_to_add sets the
'admin_earning' ALTER TABLE to be added AFTER `vendor_shipping_tax`, which
misorders columns compared to Installer.php (expected order: `vendor_order_tax`
→ `admin_earning` → `admin_commission`); update the 'admin_earning' entry in the
$columns_to_add array so its ALTER TABLE uses AFTER `vendor_order_tax` (keeping
other column definitions unchanged) to match the Installer.php schema and avoid
schema drift.
- Around line 70-86: Replace the synchronous call to
ReportsSync::regenerate_report_data() inside regenerate_order_stats_data() with
background queueing using the existing DokanBackgroundProcesses flow: create a
task payload that signals ReportsSync regeneration (e.g., class/method name and
args), call DokanBackgroundProcesses::push_to_queue($payload) and then
DokanBackgroundProcesses::dispatch_process() so the heavy
ReportsSync::regenerate_report_data(null, false) work runs asynchronously during
upgrade; update regenerate_order_stats_data() to only enqueue and dispatch the
job.
🧹 Nitpick comments (2)
includes/Analytics/Reports/Orders/Stats/DataStore.php (1)
145-154:admin_earningandadmin_commissionare both sourced fromget_admin_commission()— clarify intent.Lines 148 and 149 both call
$order_commission->get_admin_commission(). Ifadmin_earningis intended to eventually diverge fromadmin_commission, this is fine as a transitional step, but a brief inline comment explaining the distinction (or planned distinction) would help future maintainers.Also, per the linked issue,
admin_earningshould be "populated viaget_admin_commission", so the current mapping is consistent with the spec — just redundant at this point.includes/Upgrade/Upgrades/V_4_2_10.php (1)
50-61: No error handling after$wpdb->query()— failed ALTERs are silently ignored.If an
ALTER TABLEfails (e.g., due to permissions or disk space),$wpdb->query()returnsfalseand no error is logged. On the next plugin load the upgrade won't re-run (version is already bumped), leaving the schema in a partial state. Consider logging a warning on failure.Proposed improvement
if ( empty( $column_exists ) ) { - $wpdb->query( $alter_query ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + $result = $wpdb->query( $alter_query ); // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared + if ( false === $result ) { + dokan_log( sprintf( 'Failed to add column %s to %s: %s', $column_name, $table_name, $wpdb->last_error ) ); + } }
| /** | ||
| * Upgrade class for version 3.15.0. | ||
| * | ||
| * @since 3.15.0 | ||
| */ | ||
| class V_4_2_10 extends DokanUpgrader { |
There was a problem hiding this comment.
Docblock version mismatch: @since 3.15.0 vs upgrade key 4.2.10.
The @since tags throughout this file reference 3.15.0, but the class is named V_4_2_10 and registered under version 4.2.10 in Upgrades.php. Align the docblocks.
🤖 Prompt for AI Agents
In `@includes/Upgrade/Upgrades/V_4_2_10.php` around lines 9 - 14, The docblock
`@since` tags in this file are incorrect: they say "3.15.0" while the upgrade
class is V_4_2_10 and registered as version 4.2.10; update the file-level and
any method-level docblocks inside this file to use "@since 4.2.10" (search for
every occurrence of "3.15.0" in this file and replace it), ensuring the class
declaration V_4_2_10 and any related methods' docblocks reflect the correct
4.2.10 version.
| $columns_to_add = [ | ||
| 'vendor_shipping_tax' => "ALTER TABLE `{$table_name}` ADD COLUMN `vendor_shipping_tax` double NOT NULL DEFAULT '0' AFTER `vendor_discount`", | ||
| 'vendor_order_tax' => "ALTER TABLE `{$table_name}` ADD COLUMN `vendor_order_tax` double NOT NULL DEFAULT '0' AFTER `vendor_shipping_tax`", | ||
| 'admin_earning' => "ALTER TABLE `{$table_name}` ADD COLUMN `admin_earning` double NOT NULL DEFAULT '0' AFTER `vendor_shipping_tax`", | ||
| 'admin_shipping_tax' => "ALTER TABLE `{$table_name}` ADD COLUMN `admin_shipping_tax` double NOT NULL DEFAULT '0' AFTER `admin_discount`", | ||
| 'admin_order_tax' => "ALTER TABLE `{$table_name}` ADD COLUMN `admin_order_tax` double NOT NULL DEFAULT '0' AFTER `admin_shipping_tax`", | ||
| ]; |
There was a problem hiding this comment.
admin_earning column placed AFTER vendor_shipping_tax — inconsistent with Installer.php schema.
In Installer.php, the column order is: vendor_order_tax → admin_earning → admin_commission. But line 45 places admin_earning after vendor_shipping_tax, which puts it before vendor_order_tax in upgraded databases.
This doesn't affect functionality (queries use column names, not ordinal positions), but it creates a schema drift between fresh installs and upgrades.
Proposed fix
- 'admin_earning' => "ALTER TABLE `{$table_name}` ADD COLUMN `admin_earning` double NOT NULL DEFAULT '0' AFTER `vendor_shipping_tax`",
+ 'admin_earning' => "ALTER TABLE `{$table_name}` ADD COLUMN `admin_earning` double NOT NULL DEFAULT '0' AFTER `vendor_order_tax`",🤖 Prompt for AI Agents
In `@includes/Upgrade/Upgrades/V_4_2_10.php` around lines 42 - 48, The upgrade
script's $columns_to_add sets the 'admin_earning' ALTER TABLE to be added AFTER
`vendor_shipping_tax`, which misorders columns compared to Installer.php
(expected order: `vendor_order_tax` → `admin_earning` → `admin_commission`);
update the 'admin_earning' entry in the $columns_to_add array so its ALTER TABLE
uses AFTER `vendor_order_tax` (keeping other column definitions unchanged) to
match the Installer.php schema and avoid schema drift.
| // Regenerate the order stats data | ||
| self::regenerate_order_stats_data(); | ||
| } | ||
|
|
||
| /** | ||
| * Regenerate order stats data by syncing WC order stats. | ||
| * | ||
| * @since 3.15.0 | ||
| * | ||
| * @return void | ||
| */ | ||
| private static function regenerate_order_stats_data() { | ||
| // Sync the WC order stats to regenerate dokan order stats | ||
| if ( class_exists( ReportsSync::class ) ) { | ||
| ReportsSync::regenerate_report_data( null, false ); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the ReportsSync class and regenerate_report_data method
find . -type f -name "*.php" | xargs grep -l "class ReportsSync" 2>/dev/null | head -5Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Search for regenerate_report_data method definition
rg -n "regenerate_report_data" --type php -B 2 -A 15 | head -100Repository: getdokan/dokan
Length of output: 832
🏁 Script executed:
# Check how regenerate_report_data is implemented in ReportsSync
fd -e php | xargs grep -l "ReportsSync" | head -10Repository: getdokan/dokan
Length of output: 135
🏁 Script executed:
# Check the top of V_4_2_10.php for imports
head -30 includes/Upgrade/Upgrades/V_4_2_10.phpRepository: getdokan/dokan
Length of output: 744
🏁 Script executed:
# Check if ReportsSync is imported or namespaced
rg -n "use.*ReportsSync|ReportsSync" includes/Upgrade/Upgrades/V_4_2_10.phpRepository: getdokan/dokan
Length of output: 227
🏁 Script executed:
# Search for any background job queuing patterns in upgrade files
rg -n "as_enqueue_async_action|schedule_single_action|ActionScheduler" includes/Upgrade/ -B 2 -A 2Repository: getdokan/dokan
Length of output: 40
🏁 Script executed:
# Search for WC Action Scheduler usage patterns in entire codebase
rg -n "as_enqueue|as_schedule|ActionScheduler" --type php | head -30Repository: getdokan/dokan
Length of output: 994
🏁 Script executed:
# Check if background processing is used in other parts of the plugin
rg -n "wp_schedule|wp_remote_post|background" --type php includes/ | head -20Repository: getdokan/dokan
Length of output: 2407
🏁 Script executed:
# Search for how other expensive operations are handled in the codebase
rg -n "async|background|queue" includes/ --type php | grep -i "job\|task\|queue" | head -20Repository: getdokan/dokan
Length of output: 1914
🏁 Script executed:
# Check other upgrade files to see if any use background processing
ls -la includes/Upgrade/Upgrades/ | head -20Repository: getdokan/dokan
Length of output: 1215
🏁 Script executed:
# Check how background processing is implemented in V_2_8_3.php
cat -n includes/Upgrade/Upgrades/V_2_8_3.phpRepository: getdokan/dokan
Length of output: 823
🏁 Script executed:
# Check the BackgroundProcess directory to understand the pattern
ls -la includes/Upgrade/BackgroundProcesses/Repository: getdokan/dokan
Length of output: 142
🏁 Script executed:
# Look at how the processor is used
rg -n "push_to_queue|dispatch_process" includes/Upgrade/Upgrades/ -B 5 -A 2Repository: getdokan/dokan
Length of output: 7023
🏁 Script executed:
# Find BackgroundProcesses directory
find . -type d -name "BackgroundProcesses" | head -5Repository: getdokan/dokan
Length of output: 105
🏁 Script executed:
# Look for background processor classes used in upgrades
rg -n "class.*Background|extends.*Background" includes/Upgrade/ --type php -B 2 -A 5Repository: getdokan/dokan
Length of output: 7153
🏁 Script executed:
# Check if there's a dedicated background processor for ReportsSync or similar analytics operations
rg -n "ReportsSync|Analytics|regenerate" includes/BackgroundProcess/ --type php 2>/dev/null | head -20Repository: getdokan/dokan
Length of output: 212
Queue ReportsSync::regenerate_report_data() as a background task during upgrade.
Calling this WooCommerce analytics regeneration synchronously will timeout on stores with large order volumes. The codebase uses DokanBackgroundProcesses with push_to_queue() and dispatch_process() throughout upgrade routines (V_2_8_3, V_2_9_4, V_3_0_10, V_3_6_2, V_3_7_19, etc.) for expensive operations. This method should follow the same pattern instead of executing inline. Note: This same issue appears in V_3_13_0.php line 15.
🤖 Prompt for AI Agents
In `@includes/Upgrade/Upgrades/V_4_2_10.php` around lines 70 - 86, Replace the
synchronous call to ReportsSync::regenerate_report_data() inside
regenerate_order_stats_data() with background queueing using the existing
DokanBackgroundProcesses flow: create a task payload that signals ReportsSync
regeneration (e.g., class/method name and args), call
DokanBackgroundProcesses::push_to_queue($payload) and then
DokanBackgroundProcesses::dispatch_process() so the heavy
ReportsSync::regenerate_report_data(null, false) work runs asynchronously during
upgrade; update regenerate_order_stats_data() to only enqueue and dispatch the
job.
1 participant
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
Chores