Skip to content

Affiliates: introduce class to get the affiliate code#10749

Merged
jeherve merged 1 commit intomasterfrom
add/affiliate-code
Nov 30, 2018
Merged

Affiliates: introduce class to get the affiliate code#10749
jeherve merged 1 commit intomasterfrom
add/affiliate-code

Conversation

@eliorivero
Copy link
Copy Markdown
Contributor

@eliorivero eliorivero commented Nov 29, 2018

This PR seeks to introduce affiliate codes in Jetpack creating a class that allows to get an affiliate code from a database option jetpack_affiliate_code, and filtering it. There are two ways to set this code in the website:

  1. Partners that can setup an affiliate code through WP CLI when deploying a site, can issue:
    wp option add jetpack_affiliate_code abc123
  2. other partners like agencies that don't have this control from the beginning can add a filter:
    add_filter( 'jetpack_affiliate_code', function() { return 'abc123'; } );

Additionally, this PR will also:

  • add the affiliate code, if found, to the connect url, as the query parameter aff
  • send it to Jetpack UI in the Initial_State object as the property aff
    it will be added to upgrade links in a follow up PR, since we need to refactor how upgrade links are written (right now they're inline) to be able to add the affiliate code or another param in the future in a consistent way using a function that given some params, returns the upgrade link.

Testing instructions

a. manually

  1. do wp option add jetpack_affiliate_code abc123 (or yarn docker:wp option add jetpack_affiliate_code abc123)
  2. verify that the URL of the Set up Jetpack button has the aff parameter with the right value
  3. verify that the Initial_Setup.aff property has the right value

b. running unit tests. There's a new test suite named affiliate

  1. run phpunit --testsuite=affiliate or yarn docker:phpunit --testsuite=affiliate

Proposed changelog entry

Not necessary to mention this change since it will be known for those that have to know about it.

@eliorivero eliorivero added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. Admin Page React-powered dashboard under the Jetpack menu labels Nov 29, 2018
@eliorivero eliorivero self-assigned this Nov 29, 2018
@eliorivero eliorivero requested review from a team and ebinnion November 29, 2018 20:05
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Nov 29, 2018

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: January 10, 2019.
Scheduled code freeze: January 3, 2019

Generated by 🚫 dangerJS

@eliorivero eliorivero force-pushed the add/affiliate-code branch 2 times, most recently from 8ef5c46 to 8a695a9 Compare November 29, 2018 21:14
@eliorivero eliorivero added [Status] In Progress and removed [Status] Needs Review This PR is ready for review. labels Nov 29, 2018
@eliorivero eliorivero added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Nov 29, 2018
ebinnion
ebinnion previously approved these changes Nov 29, 2018
Copy link
Copy Markdown
Contributor

@ebinnion ebinnion left a comment

Choose a reason for hiding this comment

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

This LGTM. I left a couple of minor comments, but otherwise 👍

I tested that the affiliate code was present in the Jetpack react admin page as well as in the connect URL.

We should also ensure that JITM links get the affiliate code added to them as well.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd suggest adding some () around the assignment here. Maybe something like:

if ( '' !== ( $aff = Jetpack_Affiliate::init()->get_affiliate_code() ) )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be more exact if we did something like the following, since I think this test is checking that aff is missing.

$this->assertNotContains( 'aff=', Jetpack::init()->build_connect_url() );

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense 👍

…n and allow to filter it. Add affiliate code to connect url and send it to Jetpack UI (it will be introduced in upgrade links in a follow up PR).
@eliorivero
Copy link
Copy Markdown
Contributor Author

Comments addressed. And yes, I'll add the affiliate link to JITMs in a follow up PR.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Nov 30, 2018
@jeherve jeherve added this to the 6.9 milestone Nov 30, 2018
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

It tests well for me. 👍 Merging.

@jeherve jeherve merged commit 764938a into master Nov 30, 2018
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Nov 30, 2018
@jeherve jeherve deleted the add/affiliate-code branch November 30, 2018 11:49
jeherve added a commit that referenced this pull request Dec 19, 2018
jeherve added a commit that referenced this pull request Jan 3, 2019
jeherve added a commit that referenced this pull request Jan 3, 2019
* Add first version of the Changelog and testing list for 6.9

* Changelog: add #10710

* changelog: add #10538

* changelog: add #10741

* changelog: add #10749

* changelog: add #10664

* changelog: add #10224

* changelog: add #10788

* Changelog: add #10560

* Chanegelog: add #10812

* changelog: add #10556

* Changelog: add #10668

* Changelog: add #10846

* Changelog: add #10947

* Changelog: add #10962

* Changelog: add #10956

* Changelog: add #10940

* Changelog: add #10934

* Changelog: add #10912

* changelog: add #10866

* changelog: add #10924

* Changelog: add #10936

* Changelog: add #10833

* changelog: add #10867

* Changelog: add #10960

* Changelog: add #10888

* changelog: add #10840

* changelog: add #10972

* Changelog: add #10979

* changelog: add #10909

* Changelog: add #10958

* Changelog: add #10981

* Changelog: add #10564

* Changelog: add #10809

* Changelog: add #10982

* Changelog: add #10706

* Changelog: add #10978

* Changelog: add #10132

* Changelog: add #11022

* Changelog: add #11024

* Changelog: add #10875

* Changelog: add #11030

* Changelog: add #11053

* Changelog: add #10880

* Changelog: add #9359

* Changelog: add #11037

* Update block list

* Changelog: add #11060

* Changelog: add #10755

* changelog: add #11000

* Changelog: add #10786

* Changelog: add #10945

* Changelog: add #10597
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Admin Page React-powered dashboard under the Jetpack menu Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants