Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix customer custom properties KlaviyoV3Api.php #289

Merged
merged 5 commits into from
May 22, 2024

Conversation

JamesFX2
Copy link
Contributor

@JamesFX2 JamesFX2 commented Feb 6, 2024

Hey, you've formatted the properties incorrectly, in the profile object, in properties is a child of data / attributes - see https://jsonformatter.org/fa628e for reference which was taken from the API example on Klaviyo

Description

Fixes bug with sending custom data on a profile to Klaviyo events API

Manual Testing Steps

This is an event we sync to Klaviyo that updates a customer profile. This field is no longer updating the contact. I've checked the API documentation and I have found this bug.

 {"data":{"type":"event","attributes":{"properties":{"$value":null},"time":"2024-02-06T13:46:34","value":null,"metric":{"data":{"type":"metric","attributes":{"name":"Customer Sync","service":"magentotwo"}}},"profile":{"data":{"type":"profile","attributes":{"email":"[email protected]","first_name":"Leila","last_name":"Test"},"properties":{"$first_name":"Leila","$last_name":"Test","email":"[email protected]","$nhs_last_date":"2024-02-06","nhs_last_date":"2024-02-06"}}}}}}

Hey, you've formatted the properties incorrectly, in the profile object, in properties is a child of data / attributes - see https://jsonformatter.org/fa628e for reference which was taken from the API example on Klaviyo
cykolln
cykolln previously requested changes Feb 6, 2024
Copy link
Contributor

@cykolln cykolln left a comment

Choose a reason for hiding this comment

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

Thanks for catching this, can you please update the Changelog and add this fix under Unreleased. We will include it in a release this week.

@JamesFX2
Copy link
Contributor Author

JamesFX2 commented Feb 6, 2024

@cykolln sorry, are you talking to me?

@cykolln
Copy link
Contributor

cykolln commented Feb 6, 2024

@cykolln sorry, are you talking to me?

@JamesFX2 Yes, sorry!

@JamesFX2
Copy link
Contributor Author

JamesFX2 commented Feb 8, 2024

Added requested changes @cykolln

@jordanleslie
Copy link
Contributor

Added requested changes @cykolln

Once CI is passing i'll merge this in.

@cykolln
Copy link
Contributor

cykolln commented May 21, 2024

@JamesFX2 Hey! Looks like precommit is failing for this PR - locally you can run pre-commit run --all-files to resolve. We can get this merged in as soon as CI is passing.

@cykolln cykolln merged commit eaa7693 into klaviyo:master May 22, 2024
3 checks passed
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.

4 participants