CRM: Add WooSync status to avoid creating invoices and transactions #37276

gogdzl · 2024-05-08T01:50:06Z

Fixes https://github.com/Automattic/zero-bs-crm/issues/3463

Proposed changes:

This PR adds a status mapping to WooSync to avoid creating invoices and transactions in Jetpack CRM from WooCommerce

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?
Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

https://github.com/Automattic/zero-bs-crm/issues/3463

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

Initial Setup and Verification of WooSync:

Verify WooSync Functionality:
- Navigate to the WooSync hub.
- Confirm that the status displays Status: Sync Completed.
Enable Invoice Generation:
- If invoice generation is not active, switch to the trunk branch and activate the feature at /wp-admin/admin.php?page=zerobscrm-plugin-settings&tab=woosync.
Create a WooCommerce Order:
- Place a new order in WooCommerce.
Verify Transaction and Invoice Creation:
- Ensure both the transaction and invoice are correctly generated.

Testing the New PR Setting:

Switch to the PR Branch:
- Change to the branch update/crm/3463-woosync-avoid-creating-invoices-and-transactions-for-woo-draft.
Configure WooCommerce Settings:
- Access Woo settings at /wp-admin/admin.php?page=zerobscrm-plugin-settings&tab=woosync.
- Change the status mapping from one of WooCommerce's orders for at least one Invoice and one Transaction status to Do not Create.
Add Another WooCommerce Order:
- Place a new order with the new mapping and confirm that WooSync is not generating Invoices when the mapping is set to Do not Create.
- Place a new order with the new mapping and confirm that WooSync is not generating Transactions when the mapping is set to Do not Create.

Documentantion changes

We will need to update our CRM Knowledge Base to reflect this change. It is necessary to update our WooSync page to inform users that we now offer a new setting and explain how it works.

Pinging @StefMattana

…ions

… not create'

github-actions · 2024-05-08T01:51:00Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

cleacos · 2024-05-09T16:31:43Z

projects/plugins/crm/modules/woo-sync/includes/class-woo-sync-background-sync-job.php

-		// Add/update invoice (if enabled) (previously `add_or_update_invoice`)
-		if ( $settings['wcinv'] == 1 ) {
+		// Add/update invoice (if enabled) (previously `add_or_update_invoice`), while checking for a 'Do not create' status to avoid creating this invoice if the mapping doesn't allow it
+		if ( $settings['wcinv'] == 1 && isset( $crm_object_data['invoice'] ) && isset( $crm_object_data['invoice']['status'] ) && $crm_object_data['invoice']['status'] !== WOOSYNC_DO_NOT_CREATE['id'] ) { // phpcs:ignore Universal.Operators.StrictComparisons.LooseEqual


GH is complaining about something here...

cleacos · 2024-05-09T16:37:14Z

projects/plugins/crm/modules/woo-sync/jpcrm-woo-sync-init.php

+// If we ever have more WooSync constants we should create a separate php file.
+if ( ! defined( 'WOOSYNC_DO_NOT_CREATE' ) ) {
+	define(
+		'WOOSYNC_DO_NOT_CREATE',


For this global definition I'd add the JPCRM_ prefix => JPCRM_WOOSYNC_DO_NOT_CREATE

cleacos

It works well for me. I'd add the JPCRM_ prefix to the WOOSYNC_DO_NOT_CREATE definition => JPCRM_WOOSYNC_DO_NOT_CREATE

Other than that, LGTM!

cleacos · 2024-05-09T16:51:44Z

As a side note, if an invoice is created because the order is different to Draft, if we set the order a Draft, the invoice/transaction doesn't change its status.

For me that behavior is OK, once the invoice is created, you cannot go back automatically if the order is set as Draft later. But maybe I'd say that, if the invoice/transaction is already created and the new status should be Draft, we have to switch to that status.

gogdzl · 2024-05-09T17:05:40Z

As a side note, if an invoice is created because the order is different to Draft, if we set the order a Draft, the invoice/transaction doesn't change its status.

That is a known problem, we sync only existing transactions, if a transactions gets deleted we don't record that.

cleacos · 2024-05-10T08:59:13Z

projects/plugins/crm/modules/woo-sync/includes/class-woo-sync-background-sync-job.php

-		// Add/update invoice (if enabled) (previously `add_or_update_invoice`)
-		if ( $settings['wcinv'] == 1 ) {
+		// Add/update invoice (if enabled) (previously `add_or_update_invoice`), while checking for a 'Do not create' status to avoid creating this invoice if the mapping doesn't allow it
+		if ( $settings['wcinv'] == 1 && isset( $crm_object_data['invoice'] ) && isset( $crm_object_data['invoice']['status'] ) && $crm_object_data['invoice']['status'] !== JPCRM_WOOSYNC_DO_NOT_CREATE['id'] ) { // phpcs:ignore Universal.Operators.StrictComparisons.LooseEqual


I don't know why GH is still complaining here...

cleacos

Re-approved it...

A test is failing

cleacos · 2024-05-10T11:16:04Z

As a side note, if an invoice is created because the order is different to Draft, if we set the order a Draft, the invoice/transaction doesn't change its status.

That is a known problem, we sync only existing transactions, if a transactions gets deleted we don't record that.

I mean, the user creates an Order in a valid status, let's say completed. JPCRM will generate the invoice as paid, but the user takes that order and changes it to Draft; in JPCRM, the invoice status will not change to Draft, it will leave the invoice as Paid. But it's a side note about this edge case not covered by this PR, not a blocker.

cleacos

👍

gogdzl added 4 commits May 7, 2024 22:37

Add 'Do not create' option for WooSync imported invoices and transact…

4fcf2d2

…ions

Add 'Do not create' option as a valid mapping

221d5f9

Do not create invoices and transactions when state mapping equals 'Do…

ac86bc6

… not create'

changelog

07fcab7

gogdzl requested a review from cleacos May 8, 2024 01:50

gogdzl self-assigned this May 8, 2024

gogdzl added [Status] Needs Team Review [Plugin] CRM Issues about the Jetpack CRM plugin labels May 8, 2024

gogdzl added this to the crm/6.4.3 milestone May 8, 2024

github-actions bot added the [CRM] WooSync Module label May 8, 2024

gogdzl added 2 commits May 7, 2024 22:53

Change Significance to patch

3e7725e

Check if array index is valid

5e85fb0

cleacos reviewed May 9, 2024

View reviewed changes

cleacos previously approved these changes May 9, 2024

View reviewed changes

Improve constant naming

43cfe7d

gogdzl dismissed cleacos’s stale review via 43cfe7d May 10, 2024 04:44

cleacos reviewed May 10, 2024

View reviewed changes

cleacos previously approved these changes May 10, 2024

View reviewed changes

Suppress warning

d546d6f

gogdzl dismissed cleacos’s stale review via d546d6f May 10, 2024 13:40

gogdzl added 2 commits May 10, 2024 10:50

Suppress warning

c34b31e

Suppress warning

376dc88

cleacos approved these changes May 10, 2024

View reviewed changes

gogdzl merged commit 8477a12 into trunk May 10, 2024
53 checks passed

gogdzl deleted the update/crm/3463-woosync-avoid-creating-invoices-and-transactions-for-woo-draft branch May 10, 2024 16:50

github-actions bot removed the [Status] Needs Team Review label May 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CRM: Add WooSync status to avoid creating invoices and transactions #37276

CRM: Add WooSync status to avoid creating invoices and transactions #37276

gogdzl commented May 8, 2024

github-actions bot commented May 8, 2024

cleacos May 9, 2024

cleacos May 9, 2024 •

edited

Loading

cleacos left a comment

cleacos commented May 9, 2024

gogdzl commented May 9, 2024

cleacos May 10, 2024

cleacos left a comment

cleacos commented May 10, 2024

cleacos left a comment

CRM: Add WooSync status to avoid creating invoices and transactions #37276

CRM: Add WooSync status to avoid creating invoices and transactions #37276

Conversation

gogdzl commented May 8, 2024

Proposed changes:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Documentantion changes

github-actions bot commented May 8, 2024

cleacos May 9, 2024

Choose a reason for hiding this comment

cleacos May 9, 2024 • edited Loading

Choose a reason for hiding this comment

cleacos left a comment

Choose a reason for hiding this comment

cleacos commented May 9, 2024

gogdzl commented May 9, 2024

cleacos May 10, 2024

Choose a reason for hiding this comment

cleacos left a comment

Choose a reason for hiding this comment

cleacos commented May 10, 2024

cleacos left a comment

Choose a reason for hiding this comment

cleacos May 9, 2024 •

edited

Loading