-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add/jetpack affiliation - PRESS4-516 #64
Conversation
bootstrap.php
Outdated
'crazy-domains' => '57687', | ||
'hostgator-india' => '57686', | ||
'bluehost-india' => '86241', | ||
'hostgator-latam' => '57686', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HostGator LATAM requires an additional region check to differentiate from normal HostGator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to verify additional regions because we have the same Jetpack affiliation code for all hosts, including Hostgator, Hostgator-India, and Hostgator-Latam?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manikantakailasa Ah, no. Let's remove the hostgator-india
, bluehost-india
, and hostgator-latam
options then, as the other cases will cover these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing hostgator-india, bluehost-india, and hostgator-latam will result in the brands mapping to default and not having their respective Jetpack affiliation codes.
$container->plugin()->id; container will return brand as hostgator-india, bluehost-india, and hostgator-latam
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manikantakailasa The container()->plugin()->id
will ONLY return the base brand depending on the plugin:
- bluehost
- hostgator
- crazy-domains
- web
- mojo
It is not configured to include any suffixes.
bootstrap.php
Outdated
'web' => '86239', | ||
'crazy-domains' => '57687', | ||
'hostgator-india' => '57686', | ||
'bluehost-india' => '86241', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have a good way to determine if a site is Bluehost India or HostGator India right now. We can keep this code, but it won't do anything.
bootstrap.php
Outdated
@@ -79,6 +80,8 @@ function ( $value ) { | |||
'newfold_container_set', | |||
function ( Container $container ) { | |||
|
|||
NFD_DATA_MODULE_VERSION === '2.4.21' && nfd_update_options_table( $container ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did the upgrade routine approach not work? That is the preferred method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are getting this error - Uncaught NewfoldLabs\Container\NotFoundException : No entry was found for "plugin" identifier.
As we talked about earlier, we're employing a hook call and specifying a version condition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manikantakailasa, I was thinking we would add the hook to the upgrade routine file. So in the 2.4.22.php
file, add the newfold_container_set
callback and move the code out of the called function into the hook.
bootstrap.php
Outdated
} | ||
$jetpack_affiliate_code = get_option( 'jetpack_affiliate_code' ); | ||
! $jetpack_affiliate_code && | ||
update_option( 'jetpack_affiliate_code', $brand_code[ $brand ] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to fix code formatting here
upgrades/2.4.22.php
Outdated
add_action( | ||
'newfold_container_set', | ||
function ( Container $container ) { | ||
nfd_update_options_table( $container ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@manikantakailasa This is looking good. One last request. Let's remove the nfd_update_options_table()
function and add the code inline here since it is only used once.
set jetpack Affiliation option (auto upgrade)
Brand Bucket Code (jetpack_affiliate_code)
Bluehost 86241
Hostgator 57686
vDeck 57687
LATAM 57686
APAC
Web 86239
Newfold (Dreamscape) 86240
Generic Newfold Catchall 86240
story - https://jira.newfold.com/browse/PRESS4-516