Skip to content

Add purchasable_type to clean up signature#2303

Merged
alecritson merged 1 commit into1.xfrom
add_purchasable_type_to_cart_line_signature
Sep 30, 2025
Merged

Add purchasable_type to clean up signature#2303
alecritson merged 1 commit into1.xfrom
add_purchasable_type_to_cart_line_signature

Conversation

@alecritson
Copy link
Collaborator

@alecritson alecritson commented Sep 30, 2025 •
edited by coderabbitai bot
Loading

Adds purchasable_type to signature on the order lines cleanup action.

Summary by CodeRabbit

  • Bug Fixes
    • Improved cart-to-order consistency by correctly distinguishing items with the same identifier but different types.
    • Prevents unintended merging or duplication of order lines during checkout.
    • Reduces edge cases where similar items were incorrectly combined, ensuring accurate line item handling.

@vercel
Copy link

vercel bot commented Sep 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
lunar-docs Ready Ready Preview Comment Sep 30, 2025 9:46am

Copilot AI reviewed Sep 30, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a 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 enhances the signature generation for order line cleanup by adding the purchasable type as a distinguishing factor. This ensures that order lines with the same purchasable ID but different types are properly differentiated during cleanup operations.

  • Adds purchasable_type parameter to the signature method
  • Updates all calls to include the purchasable type from both cart lines and order lines
  • Includes the purchasable type in the signature hash calculation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 30, 2025 •
edited
Loading

Walkthrough

The order line signature logic in CleanUpOrderLines.php was updated to include purchasable_type, passing this through from cart lines to order lines and embedding it in the computed signature hash. No public APIs were changed.

Changes

Cohort / File(s) Summary
Order line signature update
packages/core/src/Pipelines/Order/Creation/CleanUpOrderLines.php		 Include purchasable_type in cart and order line signature computation; update calls to pass type; signature hash now includes a "type" field.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • glennjacobs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly identifies the primary change—adding purchasable_type to the cleanup signature—directly reflecting the modifications in the CleanUpOrderLines class. It is concise, clear, and specific enough that a teammate scanning the history can immediately understand the PR’s main intent.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add_purchasable_type_to_cart_line_signature

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
🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

  • Public repositories are always opted into early access features.
  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

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

coderabbitai[bot]
coderabbitai bot reviewed Sep 30, 2025
View reviewed changes
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

🧹 Nitpick comments (1)
packages/core/src/Pipelines/Order/Creation/CleanUpOrderLines.php (1)

40-48: Add test for distinct purchasable_type in CleanUpOrderLines

Existing tests cover removal based on ID, meta and quantity but don’t assert that two lines with the same ID, meta and quantity—but different purchasable_type—remain separate. Add a unit test where you seed two order lines with identical ID/meta/qty but differing purchasable_type (e.g., a ProductVariant vs a custom line type) and assert that both persist after running the pipeline.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c6e1676 and c47b578.

📒 Files selected for processing (1)
  • packages/core/src/Pipelines/Order/Creation/CleanUpOrderLines.php (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/Pipelines/Order/Creation/CleanUpOrderLines.php (1)
packages/core/src/Models/Order.php (1)
  • productLines (118-121)
⏰ 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). (18)
  • GitHub Check: search - PHP 8.4 - L12.* ↑ E
  • GitHub Check: shipping - PHP 8.4 - L12.* ↑ E
  • GitHub Check: shipping - PHP 8.4 - L12.* ↑
  • GitHub Check: admin - PHP 8.4 - L12.* ↑
  • GitHub Check: admin - PHP 8.4 - L11.* ↑ E
  • GitHub Check: core - PHP 8.4 - L11.* ↑ E
  • GitHub Check: core - PHP 8.3 - L12.* ↑
  • GitHub Check: admin - PHP 8.4 - L11.* ↑
  • GitHub Check: core - PHP 8.4 - L11.* ↑
  • GitHub Check: search - PHP 8.3 - L11.* ↑
  • GitHub Check: admin - PHP 8.3 - L12.* ↑ E
  • GitHub Check: core - PHP 8.3 - L12.* ↑ E
  • GitHub Check: search - PHP 8.3 - L11.* ↑ E
  • GitHub Check: core - PHP 8.3 - L11.* ↑ E
  • GitHub Check: core - PHP 8.3 - L11.* ↑
  • GitHub Check: admin - PHP 8.3 - L11.* ↑
  • GitHub Check: admin - PHP 8.3 - L11.* ↑ E
  • GitHub Check: fix-code-style
🔇 Additional comments (2)
packages/core/src/Pipelines/Order/Creation/CleanUpOrderLines.php (2)

20-22: LGTM! Signature now includes purchasable_type.

The cart line signature correctly includes purchasable_type, which ensures proper identification of cart lines with different types.

24-35: LGTM! Order line signature correctly includes purchasable_type.

The order line signature generation now includes purchasable_type, maintaining consistency with the cart signature logic. This ensures that order lines are properly matched against cart lines.

@alecritson alecritson merged commit ed29627 into 1.x Sep 30, 2025
49 checks passed
@alecritson alecritson deleted the add_purchasable_type_to_cart_line_signature branch September 30, 2025 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@glennjacobs glennjacobs glennjacobs approved these changes

+1 more reviewer

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alecritson @glennjacobs