putyourlightson / craft-campaign

Send and manage email campaigns, contacts and mailing lists in Craft CMS.
https://putyourlightson.com/plugins/campaign
Other
63 stars 25 forks source link

Custom fields lost during Craft 3 → 4 migration #391

Closed bossanova808 closed 1 year ago

bossanova808 commented 1 year ago

I am still doing test migrations from Craft3/Commerce3 (latest of both) to Craft4/Commerce4 (latest of both). Obviously Campaign is updated along with this. (Have I missed some sort of Campaign upgrade guide?)

I am busy dealing with all the firstName LastName -> fullName changes that result from the upgrade. I therefore noticed some errors during checkout testing, where I create new contacts (if they tick the box etc) -> that it was complaing there is no firstName field.

In diagnosing these, I see that Campaign loses those custom fields I have set up for Contacts during the upgrade.

I presume this is not intended? As far as I can see, all my other settings migrate over ok.

I go from this:

image

to this:

image

Any help would be appreciated!

bossanova808 commented 1 year ago

(no errors in the upgrade just BTW)

bencroker commented 1 year ago

I haven’t encountered that before but have made a change in https://github.com/putyourlightson/craft-campaign/commit/d001c8a2c4478afde1276f1aeeaac24bb930f15b that might help. You can test this by running composer require putyourlightson/craft-campaign:dev-develop as 2.7.3.

bossanova808 commented 1 year ago

Thanks Ben, am doing another test upgrade early next week, will let you know the outcome.

bossanova808 commented 1 year ago

I was doing a craft up this morning, and these changes resulted in:

Applying changes from your project config files ...
- updating plugins.seomatic.settings ... done
- updating campaign.contactFieldLayout.cee6029f-14a1-4cbb-ba1f-2675739b3147.tabs.0.elements.0 ...
  - updating campaign.contactFieldLayout ...
error: craft\helpers\StringHelper::encodeMb4(): Argument #1 ($str) must be of type string, null given, called in /var/www/html/vendor/craftcms/cms/src/services/Fields.php on line 1427
Failed to run craft up: exit status 1

I rolled back to 2.7.1 for now

bossanova808 commented 1 year ago

(somewhat related to this - support for a console utility to craft resave contacts where you can update fields etc, as with other Elements, would be very handy!)

bencroker commented 1 year ago

I rolled back to 2.7.1 for now

So what is the current status of your site? Were you able to upgrade to Craft 4 and keeping the contact fields?

bossanova808 commented 1 year ago

This was just while doing a composer update, moving between dev environments with a Craft 4 db.

Tomorrow I'll be doing the next test c3 -> c4 migration, ran out of time today. So I guess I should be on 2.7.2 when I do that?

bencroker commented 1 year ago

Yeah maybe do the big upgrade while on 2.7.2. Then, if the custom fields are missing, test with 2.7.3 (no need to do the Craft upgrade again though).

bossanova808 commented 1 year ago

Will do. Might end up not being tomorrow, as ideally I'd like another migration fix to be in (https://github.com/craftcms/commerce/pull/3177) - but it might be a while, so I'll probably run one soon, with 2.7.2 and let you know.

bossanova808 commented 1 year ago

Ok, happy to report back this worked - on my latest migration (using 2.7.2 specifically) - all went well and I see the fields there now.

Many thanks (and presume I'll be ok to do the final upgrade using 2.8.0 or whatever when the time actually comes...hopefully not too long now....I'm finally almost there I think!)

bossanova808 commented 1 year ago

Actually a follow up. So, the fields now do exist on the Contacts in Campaign following the migration and existing contacts still have the correct data in them.

image

BUT - I have some code triggered by Commerce's beforeOrderComplete event, and am trying to create and subscribe a contact, the new (to me) Campaing V2 way.

Due to the checkout flow here (this is concurrent with the payment stage), I don't have easy access to existing form fields here, so I need to create them in this function, from the Commerce order:

                    // Should really never happen!
                    if ($mailingList === null) {
                       self::error("Campaign - could not find mailing list 'newsletter' ?! while trying to subscribe: " . $order->email);
                        throw new NotFoundHttpException("Mailing list [newsletter] not found!");
                    }

                    $fieldValues = [
                        "fields[firstName]" => $order->billingAddress->firstName,
                        "fields[lastName]" => $order->billingAddress->lastName,
                    ];

                    self::debug($fieldValues);

                    $contact = Campaign::$plugin->forms->createAndSubscribeContact($order->email, $fieldValues, $mailingList, 'web', 'Checkout', false);
                    if ($contact->hasErrors()) {
                        self::error("(Checkout) - unable to save new Campaign contact:" . $order->email);
                        self::error($contact->getErrors());
                    }

This gives:

Error: Setting unknown property: craft\behaviors\CustomFieldBehavior::fields[firstName]

So then I also tried as just firstName etc, without the fields[] name-space thing. That worked, in that no error was raised, and the contact was indeed created and subscribed (and the verify email arrived - much easier flow here, yay!) -> but then none of the custom fields in that contact actually have any value, i.e. firstName and lastName are blank. The debug line shows what is being pass is as expected (this without the fields in, as I think is correct):

    [info] 2023-06-02 15:03:15 subscribeNewsletter is true - attempt subscribe to newsletter (onBeforeCompleteOrderHandler)
[debug] 2023-06-02 15:03:15 Array

        [firstName] => Testy
        [lastName] => McTesty

 (onBeforeCompleteOrderHandler)
    [info] 2023-06-02 15:03:17 Subscribed Testy McTesty to newsletter as test@mctesty.com (onBeforeCompleteOrderHandler)

I think I must be missing something very obvious here...or should that in fact work?

bencroker commented 1 year ago

The field values should not be namespaced with fields[]. I just ran this code locally and everything checks out.

$mailingList = MailingListElement::find()->one();
$fieldValues = [
    'firstName' => 'Testy',
];
$contact = Campaign::$plugin->forms->createAndSubscribeContact('test@mctesty.com', $fieldValues, $mailingList, 'web', 'Checkout');
if ($contact->hasErrors()) {
    die('Errors: ' . implode(', ', $contact->getFirstErrors()));
} else {
    die('Success');
}

Screenshot 2023-06-02 at 09 03 55

Can you save the contact field layout in the CP and then retest your code?

bossanova808 commented 1 year ago

Yep, that makes it work indeed. That seems weird to me, but I have no idea how field layouts work, really!

bossanova808 commented 1 year ago

BTW, what is this supposed to link to - because it (https://craft-dev.ddev.site/ghost/campaign/contacts/Checkout) 404s for me:

image

bencroker commented 1 year ago

The source parameter should be a URL or null, so your code should be written as follows.

Campaign::$plugin->forms->createAndSubscribeContact('test@mctesty.com', $fieldValues, $mailingList, 'checkout', '/url/to/checkout/or/null');
bossanova808 commented 1 year ago

Ah ok. Is this source stuff documented anywhere? At the moment I use my own source dropdown field to hold a simple version of where they subscribed, but perhaps I can do away with that if I can use your source stuff more properly.

Also - re-saving the field layout - is that something that can be improved in your recent migration fixes, so it won't be necessary, or is that the suggested ongoing fix (remember to re-save the layout post migration)? It's not really a bother - there are plenty of other manual things I have to do, so can just add this to the list, but others might hit this perhaps.

bencroker commented 1 year ago

Ah ok. Is this source stuff documented anywhere?

Just in the source code example at https://putyourlightson.com/plugins/campaign#subscribing-a-contact The sourceType and source are really just intended for keeping track of where subscribers came from and will generally be set to web and the the request’s referring URL when subscribers come through a subscription form, respectively.

Also - re-saving the field layout - is that something that can be improved in your recent migration fixes, so it won't be necessary, or is that the suggested ongoing fix (remember to re-save the layout post migration)?

Did that make a difference? If so then I can try to improve the workflow so that is no longer a necessary step.

bossanova808 commented 1 year ago

Yes that worked - I had to resave the layout before I could actually save values to the fields (i.e. they were present in the layout but unusable)

Noted re: source, thanks!

bencroker commented 1 year ago

Great! I think the field layout issue is resolved now as I haven’t received any other such reports and it should be covered by the plugin migrations.