Skip to content

Conversation

@mrabbani
Copy link
Member

@mrabbani mrabbani commented Oct 6, 2025

All Submissions:

  • My code follow the WordPress' coding standards
  • My code satisfies feature requirements
  • My code is tested
  • My code passes the PHPCS tests
  • My code has proper inline documentation
  • I've included related pull request(s) (optional)
  • I've included developer documentation (optional)
  • I've added proper labels to this pull request

Changes proposed in this Pull Request:

Related Pull Request(s)

  • Full PR Link

Closes

  • Closes #

How to test the changes in this Pull Request:

  • Steps or issue link

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:

  • Code is not following code style guidelines
  • Bad naming: make sure you would understand your code if you read it a few months from now.
  • KISS: Keep it simple, Sweetie (not stupid!).
  • DRY: Don't Repeat Yourself.
  • Code that is not readable: too many nested 'if's are a bad sign.
  • Performance issues
  • Complicated constructions that need refactoring or comments: code should almost always be self-explanatory.
  • Grammar errors.

FOR PR REVIEWER ONLY:

As a reviewer, your feedback should be focused on the idea, not the person. Seek to understand, be respectful, and focus on constructive dialog.

As a contributor, your responsibility is to learn from suggestions and iterate your pull request should it be needed based on feedback. Seek to collaborate and produce the best possible contribution to the greater whole.

  • Correct — Does the change do what it’s supposed to? ie: code 100% fulfilling the requirements?
  • Secure — Would a nefarious party find some way to exploit this change? ie: everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities?
  • Readable — Will your future self be able to understand this change months down the road?
  • Elegant — Does the change fit aesthetically within the overall style and architecture?

Summary by CodeRabbit

  • Tests
    • Upgrades test suite to align with WooCommerce HPOS, ensuring broader table coverage and reliable installation during testing.
    • Switches to queue-based task processing in tests for more consistent execution.
    • Introduces improved test data factories and grouping, with refined namespaces for clearer organization.
    • Removes deprecated database assertion utilities and streamlines related checks.
  • Chores
    • General test stability and maintenance improvements.
  • Note
    • No user-facing functionality changes in this release.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 6, 2025

Walkthrough

Introduces HPOS-aware test bootstrap and installation, renames DokanTestCase queue helpers, replaces pending-task runner across Analytics/Product tests, adjusts several Orders/Commission tests and metadata, removes a DB assertion trait, adds a WooCommerce CouponFactory, and updates various REST/Withdrawal tests with grouping and setup tweaks.

Changes

Cohort / File(s) Summary
Test bootstrap & harness
tests/php/bootstrap.php, tests/php/src/DokanTestCase.php
Bootstrap now enables WooCommerce HPOS, sync options, and creates HPOS/Analytics tables; truncation guards for table existence. DokanTestCase renames queue helpers to run_all_pending_queue/cancel_all_pending_queue.
Analytics tests: switch to queued runner
tests/php/src/Analytics/*
Replaces run_all_pending() with run_all_pending_queue() across Order/Product query, stats, and schedule listener tests; adds group annotations.
Orders tests: namespace, data providers, expectations
tests/php/src/Orders/*
Namespace normalized to WeDevs\Dokan\Test\Orders, adds group tags, adjusts status transition data providers, reduces mock count in vendor balance handler, renames OrderTest to OrderLogTest.
Commission tests: metadata and removal
tests/php/src/Commission/*
Adds/updates group annotations, removes test_get_earning_by_order_method and comments an assertion in CommissionTest, updates StrategyTest group.
REST tests: grouping and setup
tests/php/src/REST/*
Adds @group annotations; CustomersControllerTest introduces protected customer_data with setUp population; minor metadata in other controllers and vendor product categories (adds $categories).
Withdrawal tests: workflow and inputs
tests/php/src/Withdrawal/*
Default withdrawal method test now sets capabilities, PayPal profile, settings, robust JSON extraction, and asserts persisted default; charge provider tweaks bank percentage '' and adds groups.
Factories and related tests
tests/php/src/Factories/CouponFactory.php, tests/php/src/DokanFactoryTest.php
Adds WC CouponFactory with CRUD-like helpers; DokanFactoryTest removes a posts count assertion and adds group tags.
DB assertions
tests/php/src/DBAssertionTrait.php, tests/php/src/CustomAssertion/DBAssertionTrait.php
Removes DBAssertionTrait file entirely; deletes docblock from CustomAssertion/DBAssertionTrait (logic unchanged).
Misc metadata and renames
tests/php/src/ReactUrlGenerationTest.php, tests/php/src/VendorNavMenuCheckerTest.php, tests/php/src/Vendor/UserToVendorConversionTest.php
Adds/updates group tags; renames UserToVendorTest to UserToVendorConversionTest with updated docblock.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Tester
  participant Bootstrap as tests/php/bootstrap.php
  participant WC as WooCommerce
  participant COTC as CustomOrdersTableController
  participant DB as Database

  Tester->>Bootstrap: Load test bootstrap
  Bootstrap->>Bootstrap: Enable HPOS, sync options (update_option)
  Bootstrap->>WC: Load plugins (Woo, Dokan)
  Bootstrap->>WC: install_wc()
  WC->>DB: Create core + analytics tables
  alt COTC available
    Bootstrap->>COTC: create_tables()
    COTC->>DB: Create HPOS tables
  end
  Bootstrap->>DB: Truncate known tables (if exists)
  Bootstrap-->>Tester: Ready (HPOS enabled)
Loading
sequenceDiagram
  autonumber
  actor Test as PHPUnit Test
  participant DTC as DokanTestCase
  participant Q as WC_Helper_Queue

  Test->>DTC: run_all_pending_queue()
  DTC->>Q: run_all_pending()
  Q-->>DTC: All queued actions processed

  Test->>DTC: cancel_all_pending_queue()
  DTC->>Q: cancel_all_pending()
  Q-->>DTC: Queue cleared
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

Needs: Dev Review, Test Automation

Suggested reviewers

  • shohag121

Poem

I toggled HPOS with a twitch of my ear,
Queues hop in order, the pathways are clear.
Coupons sprout fresh from a garden of tests,
Old traits burrowed, now taking their rests.
Orders and stats do a synchronized dance—
Thump-thump! says the rabbit, “All green at a glance.” 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description includes the template structure but leaves all required sections like "Changes proposed", "How to test changes", and "Changelog entry" as empty placeholders. It provides no actual summary of the modifications, testing instructions, or related issue links, making it impossible for reviewers to understand the intent and scope. This fails to satisfy the repository’s pull request description template requirements. Populate each section of the template with specific details about the test bootstrap enhancements, HPOS enablement, and migration to queue-based pending tasks. Complete the developer checklist by marking items as verified once coding standards and PHPCS checks are satisfied and adding any missing developer documentation. Provide a changelog entry, related issue references, and detailed testing steps to give reviewers the necessary context.
Docstring Coverage ⚠️ Warning Docstring coverage is 60.71% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title indicates the PR relates to PHP test fixes but is overly broad and does not specify the primary changes such as HPOS test support and queue-based pending task processing. It does not highlight the critical bootstrap enhancements or safety checks introduced. While it acknowledges test case work, it lacks clarity on the major functional adjustments.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/fix-php-test-cases

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
tests/php/src/Withdrawal/MakeDefaultWithdrawalMethodTest.php (1)

23-27: Rename the test function to match its behavior.

The doc comment (line 23) states "set a default withdrawal method," but the function name (line 27) is test_that_we_can_remove_withdraw_schedule(). This mismatch creates confusion about the test's actual purpose.

Apply this diff to rename the function:

-    public function test_that_we_can_remove_withdraw_schedule() {
+    public function test_that_we_can_set_default_withdrawal_method() {
tests/php/src/Orders/OrderStatusChangeTest.php (1)

164-185: Remove processing→cancelled from sub-order whitelist
In includes/Order/Hooks.php, delete 'wc-cancelled' from the wc-processing entry of $default_whitelist so that sub-orders in processing can no longer transition to cancelled.

🧹 Nitpick comments (2)
tests/php/src/REST/VendorProductCategoriesApiTest.php (1)

23-24: Add a PHPDoc for $categories for clarity

Documenting the property type helps readers and static analysers. Consider:

+    /** @var array<string,int> */
     protected $categories;
tests/php/src/Factories/CouponFactory.php (1)

49-58: Fix docblock parameter name

The docblock still references $coupon_code, but the method accepts $coupon_id; adjust the annotation to avoid confusing future readers.

-     * @param string $coupon_code
+     * @param int    $coupon_id
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe18fba and 428a15e.

📒 Files selected for processing (37)
  • tests/php/bootstrap.php (3 hunks)
  • tests/php/src/Analytics/Reports/OrderQueryFilterTest.php (9 hunks)
  • tests/php/src/Analytics/Reports/OrderStatsQueryFilterTest.php (2 hunks)
  • tests/php/src/Analytics/Reports/OrderStatsScheduleListenerTest.php (2 hunks)
  • tests/php/src/Analytics/Reports/Orders/QueryFilterTest.php (8 hunks)
  • tests/php/src/Analytics/Reports/Orders/Stats/QueryFilterTest.php (2 hunks)
  • tests/php/src/Analytics/Reports/Orders/Stats/ScheduleListenerTest.php (2 hunks)
  • tests/php/src/Analytics/Reports/ProductQueryFilterTest.php (3 hunks)
  • tests/php/src/Analytics/Reports/ProductStatsQueryFilterTest.php (4 hunks)
  • tests/php/src/Analytics/Reports/Products/QueryFilterTest.php (2 hunks)
  • tests/php/src/Analytics/Reports/Products/Stats/QueryFilterTest.php (3 hunks)
  • tests/php/src/Commission/BackwordCompatibility.php (1 hunks)
  • tests/php/src/Commission/CommissionModelTest.php (1 hunks)
  • tests/php/src/Commission/CommissionTest.php (2 hunks)
  • tests/php/src/Commission/OrderCommission.php (1 hunks)
  • tests/php/src/Commission/StrategyTest.php (1 hunks)
  • tests/php/src/CustomAssertion/DBAssertionTrait.php (0 hunks)
  • tests/php/src/DBAssertionTrait.php (0 hunks)
  • tests/php/src/DokanFactoryTest.php (1 hunks)
  • tests/php/src/DokanTestCase.php (2 hunks)
  • tests/php/src/Factories/CouponFactory.php (1 hunks)
  • tests/php/src/Orders/OrderEventListenerTest.php (1 hunks)
  • tests/php/src/Orders/OrderLogTest.php (1 hunks)
  • tests/php/src/Orders/OrderRestoreFromTrashTest.php (1 hunks)
  • tests/php/src/Orders/OrderStatusChangeTest.php (3 hunks)
  • tests/php/src/Orders/VendorBalanceUpdateHandlerTest.php (2 hunks)
  • tests/php/src/REST/AdminOnboardingControllerTest.php (1 hunks)
  • tests/php/src/REST/CustomersControllerTest.php (2 hunks)
  • tests/php/src/REST/DokanDataContinentsControllerTest.php (1 hunks)
  • tests/php/src/REST/DokanDataCountriesControllerTest.php (1 hunks)
  • tests/php/src/REST/VendorProductCategoriesApiTest.php (2 hunks)
  • tests/php/src/ReactUrlGenerationTest.php (1 hunks)
  • tests/php/src/Vendor/UserToVendorConversionTest.php (1 hunks)
  • tests/php/src/VendorNavMenuCheckerTest.php (1 hunks)
  • tests/php/src/Withdrawal/MakeDefaultWithdrawalMethodTest.php (6 hunks)
  • tests/php/src/Withdrawal/WithdrawChargeApiTest.php (2 hunks)
  • tests/php/src/Withdrawal/WithdrawChargeTest.php (1 hunks)
💤 Files with no reviewable changes (2)
  • tests/php/src/DBAssertionTrait.php
  • tests/php/src/CustomAssertion/DBAssertionTrait.php
🧰 Additional context used
🧬 Code graph analysis (14)
tests/php/src/Analytics/Reports/Products/Stats/QueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Analytics/Reports/Orders/Stats/ScheduleListenerTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Analytics/Reports/Orders/Stats/QueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Analytics/Reports/OrderStatsQueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Analytics/Reports/Orders/QueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Orders/VendorBalanceUpdateHandlerTest.php (2)
includes/Order/VendorBalanceUpdateHandler.php (1)
  • VendorBalanceUpdateHandler (17-230)
tests/php/src/DokanTestCase.php (1)
  • DokanTestCase (23-428)
tests/php/src/Orders/OrderLogTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • DokanTestCase (23-428)
tests/php/bootstrap.php (1)
includes/Install/Installer.php (1)
  • create_tables (320-330)
tests/php/src/Analytics/Reports/ProductStatsQueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Analytics/Reports/Products/QueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Analytics/Reports/ProductQueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Orders/OrderEventListenerTest.php (1)
includes/Order/OrderEventListener.php (1)
  • OrderEventListener (8-144)
tests/php/src/Analytics/Reports/OrderStatsScheduleListenerTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
tests/php/src/Analytics/Reports/OrderQueryFilterTest.php (1)
tests/php/src/DokanTestCase.php (1)
  • run_all_pending_queue (201-203)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: e2e tests (3, 3)
  • GitHub Check: e2e tests (1, 3)
  • GitHub Check: api tests (1, 1)
  • GitHub Check: e2e tests (2, 3)
🔇 Additional comments (39)
tests/php/src/Analytics/Reports/OrderStatsScheduleListenerTest.php (1)

39-39: LGTM! Consistent queue helper usage.

The refactoring to use run_all_pending_queue() improves test code consistency across the suite while preserving the same functionality.

Also applies to: 53-53

tests/php/src/Withdrawal/WithdrawChargeTest.php (1)

8-11: LGTM! Test organization improvement.

The addition of PHPUnit group annotations enables selective test execution and aligns with test organization best practices across the test suite.

tests/php/src/Withdrawal/WithdrawChargeApiTest.php (2)

7-11: LGTM! Test organization improvement.

The addition of PHPUnit group annotations enables selective test execution and improves test organization.


163-163: LGTM! Data consistency fix.

Changing the percentage value from 0 to '' (empty string) correctly aligns the test expectation with the database setup defined in save_withdraw_data_to_database() (line 42), where the bank method's percentage is stored as an empty string.

tests/php/src/Vendor/UserToVendorConversionTest.php (2)

7-7: LGTM! Class rename improves clarity.

The rename from UserToVendorTest to UserToVendorConversionTest makes the test class name more descriptive and explicit about what is being tested.


10-12: LGTM! Docblock enhancement improves test documentation.

The expanded docblock now clearly describes what the test verifies: that shop data (store name, phone number, etc.) is properly persisted when a user is converted to a vendor. This makes the test's purpose immediately clear to anyone reading the code.

tests/php/src/Withdrawal/MakeDefaultWithdrawalMethodTest.php (5)

9-12: LGTM: Test grouping added.

The @group annotations enable selective test execution and improve test organization.


77-83: LGTM: Robust JSON extraction.

The improved parsing logic correctly handles AJAX responses that may contain HTML error messages before the JSON payload by extracting from the last occurrence of {.


89-89: LGTM: Enhanced error reporting.

Including the raw response in the error message significantly aids debugging when JSON decoding fails.


100-103: LGTM: Essential verification of persistent state.

Directly verifying the dokan_withdraw_default_method user meta confirms that the AJAX handler correctly persisted the change, not just returned a success response.


34-56: Approve test setup and formatting The code properly configures user capabilities, payment profiles, and withdrawal options, and PHPCS reports no indentation violations.

tests/php/src/Orders/OrderStatusChangeTest.php (1)

8-8: LGTM! Test grouping annotation added.

The @group orders annotation enables selective test execution and better test organization.

tests/php/src/VendorNavMenuCheckerTest.php (1)

6-8: LGTM!

The @group core-feature annotation correctly categorizes this test class for better test organization and selective execution.

tests/php/src/Commission/CommissionModelTest.php (1)

13-16: LGTM!

The group annotations properly categorize this test class within the commission test suite.

tests/php/src/Commission/StrategyTest.php (1)

15-15: LGTM!

The updated group annotation commission-strategy better aligns with the test class name and content (testing various commission strategies).

tests/php/src/Commission/BackwordCompatibility.php (1)

13-16: LGTM!

The group annotations appropriately categorize this backward compatibility test within the commission test suite.

tests/php/src/Commission/OrderCommission.php (1)

9-12: LGTM!

The group annotations properly categorize this order commission test class.

tests/php/src/Orders/OrderRestoreFromTrashTest.php (1)

7-10: LGTM!

The group annotations correctly categorize this test within the orders test suite.

tests/php/src/Commission/CommissionTest.php (2)

23-26: LGTM!

The group annotation categorizes this test class appropriately.


1017-1017: Explain the commented-out get_source assertion
The // $this->assertEquals($expected['strategy_source'], $commission->get_source()); line remains in the test without explanation. Either remove it if strategy-source assertions are no longer valid, add a TODO (with an issue link) if it’s deferred work, or restore/fix the test to match the intended behavior.

tests/php/src/Analytics/Reports/Products/QueryFilterTest.php (2)

58-58: LGTM!

The method call correctly uses run_all_pending_queue(), which aligns with the renamed helper method in DokanTestCase (line 200-202 in tests/php/src/DokanTestCase.php).


92-92: LGTM!

The method call correctly uses run_all_pending_queue(), consistent with the DokanTestCase API changes.

tests/php/src/Analytics/Reports/ProductQueryFilterTest.php (1)

58-58: Queue helper rename looks correct

run_all_pending_queue() matches the updated helper in DokanTestCase, so the tests keep their intended timing.

Also applies to: 91-91, 137-137

tests/php/src/REST/AdminOnboardingControllerTest.php (1)

11-14: Grouping metadata looks good

The added @group tags align this suite with the new REST grouping convention.

tests/php/src/Analytics/Reports/Orders/Stats/QueryFilterTest.php (1)

63-63: Queue helper rename confirmed

run_all_pending_queue() aligns with the renamed helper in the base test case, so queue draining still happens as intended.

Also applies to: 100-100

tests/php/src/REST/DokanDataContinentsControllerTest.php (1)

7-10: REST grouping annotation LGTM

Adding the @group tags keeps this test aligned with the rest of the REST suite categorization.

tests/php/src/REST/DokanDataCountriesControllerTest.php (1)

7-10: LGTM!

The group annotations improve test categorization and enable selective test execution via phpunit --group rest-api or phpunit --group rest-api-data-countries.

tests/php/src/ReactUrlGenerationTest.php (1)

8-9: LGTM!

The group annotations improve test organization and enable filtering by feature category.

tests/php/src/Analytics/Reports/OrderStatsQueryFilterTest.php (2)

66-66: LGTM!

The method rename to run_all_pending_queue() aligns with the test suite's standardization of queue-based pending task processing.


104-104: LGTM!

Consistent with the queue helper API update across the test suite.

tests/php/src/Orders/OrderEventListenerTest.php (2)

3-3: LGTM!

The namespace update to WeDevs\Dokan\Test\Orders aligns with the directory structure and standardizes test organization.


9-9: LGTM!

The group annotation enables selective test execution for order-related tests.

tests/php/src/Analytics/Reports/Orders/Stats/ScheduleListenerTest.php (2)

42-42: LGTM!

Consistent with the queue helper API standardization across the test suite.


56-56: LGTM!

Aligns with the test suite's queue-based pending task processing.

tests/php/src/Analytics/Reports/OrderQueryFilterTest.php (2)

11-11: LGTM!

The additional group annotation improves test categorization and filtering capabilities.


61-61: LGTM!

All method renames to run_all_pending_queue() are consistent with the test suite's queue-based pending task processing standardization.

Also applies to: 99-99, 156-156, 194-194, 225-225, 257-257, 285-285, 307-307

tests/php/src/Analytics/Reports/Products/Stats/QueryFilterTest.php (1)

59-59: LGTM!

The method renames align with the test suite's standardized queue-based pending task processing.

Also applies to: 93-93, 136-136

tests/php/src/Analytics/Reports/Orders/QueryFilterTest.php (1)

60-60: LGTM!

All method renames to run_all_pending_queue() are consistent with the test suite's queue-based pending task processing standardization. The changes maintain the same behavior while improving API consistency.

Also applies to: 100-100, 152-152, 190-190, 221-221, 253-253, 281-281, 303-303

tests/php/src/DokanTestCase.php (1)

201-212: All old queue method calls removed

No instances of run_all_pending() or cancel_all_pending() remain; tests reference run_all_pending_queue() (and none call cancel_all_pending_queue(), matching prior usage).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants