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

update the get_license_key function to be able to get from a file #97

Open
wants to merge 1 commit into
base: bugfix/multisite-token-logic
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Uplink/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Copy link
Contributor

@defunctl defunctl Feb 10, 2025

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).

Copy link
Contributor Author

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.

Copy link
Contributor

@defunctl defunctl Feb 12, 2025

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)?

Copy link
Contributor

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' );
}

Copy link
Contributor Author

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay

}

/**
Expand Down