Skip to content

Conversation

@juah255
Copy link

@juah255 juah255 commented Jan 28, 2026

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)

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

  • Chores
    • Aligned commission metadata key usage across the system and test utilities for consistent handling.
  • Bug Fixes
    • Added a fallback to read legacy commission metadata when the new field is empty, ensuring existing commission values remain available.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 28, 2026

📝 Walkthrough

Walkthrough

Read/write of commission metadata moved from dokan_commission_meta to _dokan_commission_meta; a fallback filter was added to expose old dokan_commission_meta when the new meta is empty. Tests updated to reflect the new meta key.

Changes

Cohort / File(s) Summary
Commission Settings
includes/Commission/Settings/OrderItem.php
Switched meta key usages from dokan_commission_meta to _dokan_commission_meta for both get and save flows; logic for handling Flat commission and defaults unchanged.
Order Hooks (fallback)
includes/Order/Hooks.php
Added get_dokan_commission_meta($value, $item) and hooked it to woocommerce_order_item_get__dokan_commission_meta to return legacy dokan_commission_meta when the new meta is empty.
Test helpers
tests/pw/utils/helpers.ts
Updated test helper to read _dokan_commission_meta instead of dokan_commission_meta when accumulating admin/vendor commission totals.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

Needs: Testing, Dev Review Done, Dependency With Pro, Upcoming Release

Suggested reviewers

  • mrabbani
  • MdAsifHossainNadim

Poem

🐰 I hopped through meta keys, quick and spry,
Switched underscore paths so data won't hide,
A gentle filter pulls the old into view,
Orders keep dancing, commissions renew,
Hop, code, hop — a tiny fix, hooray! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title references a fatal error during user subscription renewal, which matches the core issue being fixed, though it is abbreviated and lacks context.
Description check ✅ Passed The PR description follows the template structure with all major sections present, including checklist items marked, related PR linked, issue closed referenced, and self-review checklist included.
Linked Issues check ✅ Passed The code changes address the core issue: replacing dokan_commission_meta with _dokan_commission_meta prevents arrays from being passed to trim(), fixing the fatal error occurring during subscription renewal checkout.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the commission metadata key issue that causes the fatal error; no unrelated modifications are present in the three files changed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 fix/fatal-error-during-user-subscription-renewal

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 (1)
includes/Commission/Settings/OrderItem.php (1)

43-55: Add fallback read for legacy meta key.
Existing orders likely still store dokan_commission_meta. Reading only _dokan_commission_meta can silently drop commission data, affecting historical renewals and calculations.

✅ Suggested backward-compatible read
-            $commission_meta       = $this->order_item->get_meta( '_dokan_commission_meta', true );
+            $commission_meta       = $this->order_item->get_meta( '_dokan_commission_meta', true );
+            if ( empty( $commission_meta ) ) {
+                $commission_meta = $this->order_item->get_meta( 'dokan_commission_meta', true );
+            }
🧹 Nitpick comments (1)
includes/Commission/Settings/OrderItem.php (1)

88-93: Optional: clean up legacy meta key on save.
If you keep the fallback read, consider deleting dokan_commission_meta after persisting the new key to avoid ambiguity.

@juah255 juah255 self-assigned this Jan 29, 2026
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