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

Remove CAPI tracker by Pinterest request. #1026

Merged
merged 4 commits into from
Jun 25, 2024
Merged

Conversation

message-dimke
Copy link
Contributor

@message-dimke message-dimke commented Jun 21, 2024

Changes proposed in this Pull Request:

Disabling Conversion API tracker according to some temporary legal aspects from Pinterest.

How to test.

  1. Install Pinterest extension.
  2. Following the Connect Wizard, set up conversion tracking.
Pinterest
  1. Setting up conversion tracking will send the extension corresponding boolean flags to indicate Pinterest Tag tracking and Conversions API tracking must be enabled.
  2. Check pinterest_for_woocommerce option inside wp_options database table.
  3. The option must include s:22:"track_conversions_capi";b:0; serialized data.
  4. The above will indicate the despite Pinterest is sending us Conversions API is ON, we force it OFF into options.
    e.g. s:17:"track_conversions";b:1;s:22:"track_conversions_capi";b:0;: Pinterest Tag on ON, but Conversions API is OFF.

Changelog entry

Update - Disabling CAPI tracker.

@message-dimke message-dimke self-assigned this Jun 21, 2024
@message-dimke message-dimke added the changelog: update Big changes to something that wasn't broken. label Jun 21, 2024
@message-dimke message-dimke requested a review from a team June 21, 2024 11:22
Copy link
Collaborator

@budzanowski budzanowski left a comment

Choose a reason for hiding this comment

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

We should do it via separate setting for CAPI. We should make the capi tracker option false. No UI for it would mean that we don't have to do anything else for now.

During the testing you just switch the CAPI tracker option to true and you can run the tests. We don't want to delete them because we will have to reintroduce them back very soon.

…r status. Forcing it to false not to enable the tracker per Pinterest requirement. Adding unit tests to cover trackers initialization logic (CAPI can only be initialized if Tag is also ON, no CAPI tracker alone is possible).
@message-dimke
Copy link
Contributor Author

We should do it via a separate setting for CAPI. We should make the capi tracker option false. No UI for it would mean that we don't have to do anything else for now.

When connecting to Pinterest, Pinterest replies back with a set of features and their corresponding statuses. The set looks like this:

tags => bool
CAPI => bool
catalog => bool

The three flags above indicate what was chosen during the onboarding wizard (a Pinterest-hosted wizard). For example, choosing to sync catalog will send the catalog flag set to true.

The problem is tracking. Pinterest has a single wizard screen for tracking and a single yes/no button that turns ON/OFF both tags and CAPI flags synchronously. So, without forcing CAPI tracking to false, we can not disable that type of tracking.

Introducing the new track_conversions_capi setting will default to false for all the existing plugin users, but for the new ones, it will be set to the value selected while onboarding. Taking this into account, I will force the setting to false when the new user is connecting, no matter what the wizard returns to us.

Does it make sense?

@message-dimke message-dimke marked this pull request as ready for review June 23, 2024 11:46
@budzanowski
Copy link
Collaborator

I will force the setting to false when the new user is connecting, no matter what the wizard returns to us.

For now this is good enough. We can change it later when they will have separate consents.

@budzanowski
Copy link
Collaborator

Looks good. Do we need to add plugin update procedure that sets CAPI tracking to false explicitly? Or not set will default to false?

@message-dimke
Copy link
Contributor Author

message-dimke commented Jun 24, 2024

If the setting is empty or there is no such setting, it will resolve to false.

public static function get_setting( $key, $force = false ) {
$settings = self::get_settings( $force );
return empty( $settings[ $key ] ) ? false : $settings[ $key ];
}

If I read it correct =)
Anyway, I have unit tests that should test this as well.

Copy link
Collaborator

@budzanowski budzanowski left a comment

Choose a reason for hiding this comment

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

OK lets do it. Please remember that this should be a GitHub only release.

@message-dimke message-dimke merged commit 2642d71 into develop Jun 25, 2024
4 checks passed
@message-dimke message-dimke deleted the remove-capi branch June 25, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: update Big changes to something that wasn't broken.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants