Skip to content

Remove use of Klaviyo API v2, use 2024-05-15 revision instead#74

Merged
loevgaard merged 6 commits intoSetono:0.4.xfrom
lusimeon:69-klaviyo-api-upgrade
Aug 5, 2024
Merged

Remove use of Klaviyo API v2, use 2024-05-15 revision instead#74
loevgaard merged 6 commits intoSetono:0.4.xfrom
lusimeon:69-klaviyo-api-upgrade

Conversation

@lusimeon
Copy link
Copy Markdown
Contributor

@lusimeon lusimeon commented Jun 21, 2024

Description

Klaviyo v2 API is EOL : https://developers.klaviyo.com/en/docs/api_versioning_and_deprecation_policy. This PR updates calls to Klaviyo API to be compatible with new API endpoints.

Breaking changes

  • Remove spatie/data-transfer-object package because not maintained and not used.
  • Return type of Setono\SyliusKlaviyoPlugin\Client\RestClientInterface::get() and Setono\SyliusKlaviyoPlugin\Client\RestClientInterface::post() change from array to Symfony\Contracts\HttpClient\ResponseInterface\ResponseInterface
  • Updates on Setono\SyliusKlaviyoPlugin\DTO\Properties\CustomerProperties properties:
    • Move zip, city, region, country properties in Setono\SyliusKlaviyoPlugin\DTO\Properties\Location class and define a new property location.
    • Remove id property in favor of externalId.
    • Remove exchangeId property.
    • Remove consent property in favor of subscriptions property (Setono\SyliusKlaviyoPlugin\DTO\Properties\Subscriptions).
    • Automatically populate properties for CustomerInterface (see Setono\SyliusKlaviyoPlugin\DTO\Properties\CustomerProperties::populateFromCustomer()).
  • setono_sylius_klaviyo.event_subscriber.update_customer_properties_from_order service constructor require one new service setono_sylius_klaviyo.dto.properties.factory.properties.
  • setono_sylius_klaviyo.client.track_identify service require setono_sylius_klaviyo.client.rest service in constructor instead of http_client service.
  • setono_sylius_klaviyo.message.handler.subscribe_customer service require setono_sylius_klaviyo.dto.properties.factory.properties and serializer in constructor.

close #69

@JoppeDC
Copy link
Copy Markdown
Contributor

JoppeDC commented Jun 21, 2024

This has been on my todo list for ages. You are a legend.

First quick glance it looks good. Maybe check if you can fix the codestyle and the static analysis warnings.

@lusimeon lusimeon force-pushed the 69-klaviyo-api-upgrade branch from 2889dc4 to f7c2598 Compare June 21, 2024 13:23
@lusimeon lusimeon marked this pull request as ready for review June 21, 2024 13:36
@lusimeon
Copy link
Copy Markdown
Contributor Author

I've fixed codestyle and i've updated tests to match with new klaviyo's endpoints payload. A maintainer seems to be required to re-run CI checks.

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 22, 2024

Codecov Report

Attention: Patch coverage is 58.69565% with 38 lines in your changes missing coverage. Please review.

Project coverage is 25.68%. Comparing base (57f6681) to head (5c9159a).

Files Patch % Lines
src/Message/Handler/SubscribeCustomerHandler.php 0.00% 25 Missing ⚠️
src/Synchronizer/ListSynchronizer.php 0.00% 5 Missing ⚠️
src/Client/RestClient.php 78.57% 3 Missing ⚠️
...er/UpdateCustomerPropertiesFromOrderSubscriber.php 0.00% 3 Missing ⚠️
src/DTO/Factory/EventFactory.php 0.00% 1 Missing ⚠️
src/EventSubscriber/SaveEmailSubscriber.php 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              0.4.x      #74      +/-   ##
============================================
+ Coverage     22.66%   25.68%   +3.02%     
- Complexity      188      200      +12     
============================================
  Files            41       44       +3     
  Lines           675      728      +53     
============================================
+ Hits            153      187      +34     
- Misses          522      541      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lusimeon
Copy link
Copy Markdown
Contributor Author

Hi, any help to fix dependency analysis CI checks?
The symfony/http-client package is marked by composer-unused as unused but it is really needed. It works locally but not on CI. I tried to ignore it, without success.

@loevgaard
Copy link
Copy Markdown
Member

Hi, @lusimeon

This is a great PR. Thank you. I will merge it into the branch and work on other improvements in another PR.

@loevgaard loevgaard merged commit 0fc6fb1 into Setono:0.4.x Aug 5, 2024
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