Skip to content

Conversation

amirnafees88
Copy link

Description

This PR updates the order processing flow to automatically mark orders containing OpenedX courses or regular virtual/downloadable products as Completed.
If an order contains physical products, it will remain in the Processing status until completed manually.

Additionally, this PR disables the Order Processing email for orders that are auto-marked as Completed (to avoid sending both "Processing" and "Completed" emails to the purchaser at the same time).

Reasoning for Email Change:
When an order is marked as Completed immediately (OpenedX courses or virtual products only), sending both the Processing and Completed emails creates unnecessary duplication for the purchaser. This PR ensures only the Completed email is sent in such cases.

Testing instructions

Single Item Combinations:

  • OpenedX Course — Order Status: Completed, Order Processing Email: Not Sent, Order Completed Email: Sent
  • Virtual — Order Status: Completed, Order Processing Email: Not Sent, Order Completed Email: Sent
  • Physical — Order Status: Processing, Order Processing Email: Sent

Two-Item Combinations:

  • OpenedX Course + Virtual — Order Status: Completed, Order Processing Email: Not Sent, Order Completed Email: Sent
  • OpenedX Course + Physical — Order Status: Processing, Order Processing Email: Sent
  • Virtual + Physical — Order Status: Processing, Order Processing Email: Sent

Three-Item Combination:

  • OpenedX Course + Virtual + Physical — Order Status: Processing, Order Processing Email: Sent

@muhammadali286
Copy link

@MaferMazu kindly review this PR, Thanks

@MaferMazu MaferMazu self-assigned this Aug 21, 2025
@MaferMazu MaferMazu self-requested a review August 21, 2025 15:44
Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amirnafees88 thanks for this contribution!

I tested it and it works. Furthermore, I think we can implement this more effectively if we treat the Open edX course product as a virtual product, which I believe is the root of the problem (with this change, the email and order status should not generate issues). Additionally, WooCommerce will treat the product as virtual, which is more accurate in our case.

@amirnafees88 @muhammadali286 I'll open an issue regarding this, but I really would like your thoughts about this.

*
* @return bool
*/
public function disable_processing_email_for_virtual_and_courses( $enabled, $order ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I saw this, my first thought was that we shouldn't modify the email behavior of other items that are not Open edX courses. So I try to simplify the logic of this function, and I obtain that the best way is to ask if $analysis['has_physical_products'] then enable the email, else: disable. However, it is not clear with this approach that I am only modifying the open edx course behavior (because here you can't see that the base behavior of the virtual and downloadable items has this email disabled).

In summary, this fix is good, but I think it is better to make improvements in this plugin so that WooCommerce treats the Open edX Course as a virtual product, which is more accurate. What do you think @amirnafees88?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we previously discussed how to implement this feature, but upon implementation, I realized that we are not specifically checking if the enrollment was successful (that could be more complicated); instead, we only verify if the enrollment exists. As a result, the behavior of the order status and email is more similar to that of a virtual product, and perhaps the answer lies in that approach.

Issue: #119

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MaferMazu, I am already handling the check for Open edX Course, Virtual, Downloadable, and Physical products in my code lines(573 to 586).

if ( $analysis['has_physical_products'] ) {
	return $enabled;
}

if ( $analysis['has_openedx_courses'] ) {
	$enrollment_metadata = $this->get_enrollment_metadata( $order_id, $analysis['items'] );
	if ( empty( $enrollment_metadata ) ) {
		return false;
	}
}

if ( $analysis['has_regular_virtual_products'] ) {
	return false;
}

About making the Open edX Course as a Virtual product, I added comments in this issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amirnafees88 thanks a lot for your answer in #119. I mixed concepts, but focusing on the email issue, the product type that doesn't pass through the "processing" state is the downloadable one. So that's the only product type that doesn't send the processing email by default.

So I suggest changing:

if ( $analysis['has_regular_virtual_products'] ) {
	return false;
}

To $enabled.

And regarding the conditions for sending or not sending the processing email for an Open edX course, it is our decision. It makes sense to me that we checked the enrollment before deciding whether to send or not the emails. But it wouldn't sound like a bad idea to me to turn it off if there are only open edx courses, regardless of whether the enrollment was successful or not, and manage failed registrations with a new feature so that if there is an error with the enrollment, an email is sent to the store owner notifying them, and that's it (out of the scope of this PR).

However, my primary concern is modifying the behavior of products that are outside our scope.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MaferMazu Thank you. I’ll handle it for the Open edX course only, without alter the default WooCommerce behavior for other product types.

Copy link
Contributor

@MaferMazu MaferMazu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry for my late review of this.

I have been thinking a lot about how to improve this behavior, as the goal is to manage order status and email sending without altering the WC behavior for the rest of the product. We want to ensure that we scope our work to what is related to the Open edX course products, to avoid unwanted changes in behavior that are outside our scope.

Regarding the order status

  • As it is right now, we are changing the behavior of the virtual products (if we have virtual products without the downloadable check), they should stay in processing. - Issue in lines 549 and 474. Argument: #119 (comment)
  • About the enrollment check based on line 542, we are only checking that at least one of the courses has an enrollment associated with it. So, if I have three courses in my order and only one enrollment, it will be marked as complete.
  • Related to enrollments, we only check that we have the object created, not if it was successful. It is possible that I make the purchase and the enrollment request is created, but the API response is 500, or the user cannot be found and the enrollment is not performed, or the enrollment date has already passed, and I didn't have force enrollment enabled.

Something we can have is:
Marked as completed only when the order contains successful course enrollments or downloadable products. For each product, check whether it is an Open edX course, downloadable, a physical item, or a virtual product. The order will only be auto-completed if all items are either courses with successful enrollments or downloadables. If any course enrollment fails, or if the order includes products requiring manual attention (like physical or virtual products), the order remains in processing, ensuring that only fully and successfully fulfilled orders are automatically completed.

Perhaps it would be a good idea to have this process as part of the enrollment creation process, to check immediately when the enrollments are being created. Here: https://github.com/openedx/openedx-wordpress-ecommerce/blob/main/admin/class-openedx-commerce-admin.php#L432
Or at least relate them, but only if it helps.

What do you think?

Regarding the email

My only comment here is that we should change the behavior (turn off the processing email) only if it is an open edx course, because we are currently turning off emails for the virtual-only products. (line 474)

@amirnafees88
Copy link
Author

@MaferMazu

Summary

  • Scope: Updates will apply only to Open edX course products, without changing WooCommerce’s default behavior for other product types.

Current Issues:

  • Virtual products are being incorrectly altered (should remain in processing).
  • Enrollment check is too broad (an order can be marked complete if only one course enrollment succeeds).
  • Enrollment success is not verified (orders may complete even if the API fails or enrollment is invalid).

Proposed Solution:

  • Orders will be marked completed only if all Open edX course enrollments are successful.
  • Downloadable products will be excluded from this logic, since WooCommerce already manages their order status correctly.
  • If any course enrollment fails, or if the order contains physical/virtual products requiring manual handling, the order will remain processing.

Closing Note:
This approach ensures that order completion is accurate, reliable, and scoped only to Open edX course products, while leaving WooCommerce’s default handling for all other product types untouched.

Regarding emails, I’ll handle them for Open edX course products, and I agree with your approach.

Please let me know if this approach works for you so I can proceed with the implementation.

@MaferMazu
Copy link
Contributor

MaferMazu commented Sep 11, 2025

@amirnafees88 thanks for the summary. You nailed it. ✨
The approach works; I only have things to take into account.

About the proposed solution for the order status:

  • Orders will be marked completed only if all Open edX course enrollments are successful.

I would take into account that this is true, only if I have only open edx courses in my order, or if I have a combination of open edx courses + (virtual downloadable products).

  • Downloadable products will be excluded from this logic, since WooCommerce already manages their order status correctly.

Checking the documentation, WooCommerce handles the completion if the order only contains virtual downloadable products https://woocommerce.github.io/code-reference/classes/WC-Order.html#method_needs_processing (the virtual-only and downloadable-only still need to be processed)

  • If any course enrollment fails, or if the order contains physical/virtual products requiring manual handling, the order will remain processing.

Totally.
Note: downloadable-only items still need to be processed.

And regarding the email part:

Regarding emails, I’ll handle them for Open edX course products, and I agree with your approach.

Yes, we should scope our custom behavior.

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.

3 participants