Product variation migration (WIP)

[lukeyangjma]

Added in a feature that migrates the CiviCRM financial and membership types for product variations.

• Products with a membership type will have the entity type set as membership, even if a financial type is defined. Financial type will also default as donation if no financial type is set before the migration.
• Products with a financial type set and no membership type will have the financial type entity.

WIP:

• Testing still needs to be done for QA

[christianwach]

@lukeyangjma Thanks for this. While you're testing, can you run your code through phpcs please? We try to keep the code style in keeping with the standards set out in phpcs.xml. Thanks!

[seamuslee001]

@lukeyangjma I found that it seems that only one of the variations get updated I would suggest swapping the array_push for $variation_product_id[] = .... instead

[lukeyangjma]

@seamuslee001 all the variations seem to update for me. Did you restart the plugins using something like the WP Reset plugin when testing a second time? The migration only applies when the 'update' buttons are pressed on the migration page (which only happens once).

If not, do you mind telling me the steps you took to recreate the issue.

[lukeyangjma]

@christianwach I applied phpcs and used phpcbf to automatically fix most errors. Some errors still persist for the entire file, but in regards to the lines of code I changed, only warnings about characters exceeding 85 in the some lines exist.

[christianwach]

@lukeyangjma Hmm, perhaps the overrides in phpcs.xml aren't comprehensive enough... seems like your PR now alters way more of the codestyle than when I run phpcbf or compare via phpcs. I'll take a look and see what I can do.

[christianwach]

@lukeyangjma Can you give me an idea of how your PHPCS is set up? It seems to apply many more rules by default than mine.

[lukeyangjma]

@christianwach I installed PHPCS from this repository using curl. I then ran phpcs /path/to/code-directory from the root of extension directory.

[christianwach]

@lukeyangjma Did you also add the WordPress Coding Standards?

[lukeyangjma]

@christianwach I don't believe so, should I add the WordPress Coding Standards and reapply PHPCS from a previous commit?

[christianwach]

should I add the WordPress Coding Standards and reapply PHPCS from a previous commit?

@lukeyangjma Yes please - you'll probably need to make a "backup" branch, then git reset to the commit before the PHPCS one, then cherry pick the commit after the PHPCS one.

[lukeyangjma]

@christianwach Running phpcs --standard=WordPress /Users/luke/Sites/cpf/wp-content/plugins/wpcv-woo-civi-integration and phpcbf --standard=WordPress /Users/luke/Sites/cpf/wp-content/plugins/wpcv-woo-civi-integration still results in what seems like all files being changed.

[christianwach]

@lukeyangjma Ah, I think you need to run phpcs whilst in the plugin's directory - otherwise the local phpcs.xml declarations won't be read. Also, no need to run it on the whole plugin - just the file you've changed e.g.

phpcbf includes/classes/class-woo-civi-admin-migrate.php

You can ignore the Processing form data without nonce verification errors - they're false positives IIRC.

[christianwach]

Also, it seems I have the following in my CodeSniffer.conf file:

<?php
 $phpCodeSnifferConfig = array (
  'installed_paths' => '/path/to/WordPress-Coding-Standards/',
)
?>

So it automatically uses that library.

Methinks some "notes on contributing" are in order. Thanks for your patience!

[Edzelopez]

Hey @christianwach

It's @lukeyangjma's last day at JMA today, so I'm going to ask Monish to take over from here. Thanks for all of your help with this!

[christianwach]

Thanks for the info @Edzelopez and good luck with your new projects @lukeyangjma - You've helped me a lot in terms of understanding how to develop a Contribution Guidelines document.

[christianwach]

@Edzelopez @seamuslee001 Just returning to this... and have a quick question: how did you get the CiviCRM mappings into Product Variations in the first place? IIRC that wasn't possible with the previous version of the plugin.

[JoeMurray]

@monishdeb could you try to rework things so that the diff doesn't include whitespace changes?

[monishdeb]

Sure :) 👍

[monishdeb]

@Edzelopez @seamuslee001 Just returning to this... and have a quick question: how did you get the CiviCRM mappings into Product Variations in the first place? IIRC that wasn't possible with the previous version of the plugin.

@christianwach sorry for the delay. So in our custom plugin we are mapping each product variation in WC to a text price field in Civi. And the code is written under the custom action save_civicrm_variation_settings.

    add_action( 'wpcv_woo_civi/product/variation/attributes/saved/after', [$this, 'save_civicrm_variation_settings'], 30, 3);

    ...
      public function save_civicrm_variation_settings($variation, $loop, $entity_type) { ... }

In that action, there is a reserved price set to manage each product variation as respective text price field in Civi. Say for example the variable product 'T-Shirt' has two colours (green and white) with six sizes Extra Small (XS), Small (S), Medium (M), Large (L), Extra Large (XL), Double Extra Large (XXL) in other words a variable product 'T-Shirt' with 2x6 variations. If a staff person save a variation say Green - Extra Large, without selecting FT and PFV, then on save:

1. it adds a metadata _wpcv_wci_variable_contribution_financial_type_id with parent product's FT or default FT configured in the WC-Civi integration setting.
2. For PFV, it first search for Price Field with name 'T-Shirt - Green, Extra Large' basically what $variation->get_name() returns. If it doesn't find one then using the necessary parameters : regular_price as option_amount, $variation->get_name() as label, reserved price_set_id, it creates a PF and fetch/use the PFV id to add the metadata _wpcv_wci_variable_contribution_pfv_id for that variation.

In further improvement, instead of using reserved Price Set, I am adding a new price set (if not exist) using parent product label or post title as label, in this case it would be 'T-shirt #12313' and #12313 here is the post id.

[christianwach]

@monishdeb Thanks for the explanation. It seems that the code here is very specific to your custom install. I suspect that no-one else (perhaps not even any site other than the one you're migrating) is going to use the code in this PR.

I'm wondering if there might be a better approach... to add in actions and filters at key points in the migration process which you can then create callbacks for that do what you want with your additional data in your specific circumstances?

[christianwach]

Closing because the code is too specific to a particular install. Feel free to reopen if you take the "actions and filters" approach that I mentioned in my last comment.