-
Notifications
You must be signed in to change notification settings - Fork 1
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
update the get_license_key function to be able to get from a file #97
base: bugfix/multisite-token-logic
Are you sure you want to change the base?
Conversation
@@ -237,7 +237,7 @@ function get_license_key( string $slug ): string { | |||
|
|||
$network = allows_multisite_license( $resource ); | |||
|
|||
return $resource->get_license_key( $network ? 'network' : 'local' ); | |||
return $resource->get_license_key( $network ? 'network' : 'any' ); |
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.
This going to check the network again even if we don't allow multisite licenses, so you'll have to adjust this to first check for local
and if no local is found, check for default if you want this to fall back to a file (assuming $network === false
).
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.
The main branch checks the network options table if it exists. The difference is that with our branch you can have it only checking the network options and not allowing you to do anything on the local subsite if your in the mode.
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.
Am I incorrect that if $network = allows_multisite_license( $resource );
returns false, then passing any
to $resource->get_license_key()
would then possibly find a multisite license key that shouldn't be allowed to be used?
Because that ultimately does a number of complex checks to ultimately arrive at this chart..
If $network === false
and yet we still end up pulling a key, isn't that going to try register a multisite key on a single site if the plugin isn't network activated (something Uplink wasn't checking for)?
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.
What I'm suggesting is the following: We only specifically check network
or local
and fallback to "file", using default
for both (assuming you want that for network?).
/**
* A multisite license aware way to get a resource's license key automatically
* from the network or local site level.
*
* @param string $slug The plugin/service slug.
*
* @throws \RuntimeException
*
* @return string
*/
function get_license_key( string $slug ): string {
$resource = get_resource( $slug );
if ( ! $resource ) {
return '';
}
$network = allows_multisite_license( $resource );
if ( $network ) {
return $resource->get_license_key( 'network' ) ?: $resource->get_license_key( 'default' );
}
return $resource->get_license_key( 'local' ) ?: $resource->get_license_key( 'default' );
}
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.
it's totally ok for the key to be found on the network options table if it were somehow set there, this is how it works in the main branch. The difference is that if it's network license then it's one activation for the network, if the key is pulled from the network the subsite still has to activate itself as a different seat.
I don't think we need to move away from the main branch on this and your code would be moving us away from that main branch
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.
Okay
@kadencewp actually, should even bother checking the network first then? Should it just be |
@defunctl Yes I want to check the network because if it's in network mode then I do only want to check the network and in Kadence we don't even show any license input area for subsites |
Previously we were only setting this to Local so that the file was not looked at but now we do want to use embed so we need to make this default to either network if in network mode or any so that the file can be checked.