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 constant from the deprecated.php in optimization-detective plugin #1920

Open
hbhalodia opened this issue Mar 12, 2025 · 8 comments
Open
Labels
blocked [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Bug An existing feature is broken

Comments

@hbhalodia
Copy link
Contributor

hbhalodia commented Mar 12, 2025

Bug Description

Part of - #1859 and in PR - #1865, We have removed the use of the constants from the file, instead we have added those constant in a class.

For backward compatibility, we have kept those constant in deprecated.php file, since these constants are being used in Image Priortizer plugin as well.

The constants are OD_REST_API_NAMESPACE and OD_URL_METRICS_ROUTE, in newer versions after 2 or 3 releases, (see #1943 (comment)) we need to update the plugin to use the class constants instead of deprecated once and remove it from the file. See

We need to pick this once the PR - #1865 is merged and Optimization detective plugin is released with Image Priortizer as well.

Steps to reproduce

  • None

Screenshots

  • None

Cc: @westonruter @felixarntz

@westonruter
Copy link
Member

As part of this issue, let's also remove:

case 'url_metrics_group_collection':
// TODO: Remove this when no plugins are possibly referring to the url_metrics_group_collection property anymore.
_doing_it_wrong(
esc_html( __CLASS__ . '::$' . $name ),
esc_html(
sprintf(
/* translators: %s is class member variable name */
__( 'Use %s instead.', 'optimization-detective' ),
__CLASS__ . '::$url_metric_group_collection'
)
),
'optimization-detective 0.7.0'
);
return $this->url_metric_group_collection;

@westonruter
Copy link
Member

westonruter commented Mar 14, 2025

Also these class aliases:

class_alias( OD_URL_Metric_Group::class, 'OD_URL_Metrics_Group' ); // Temporary class alias for back-compat after rename.
class_alias( OD_URL_Metric_Group_Collection::class, 'OD_URL_Metrics_Group_Collection' ); // Temporary class alias for back-compat after rename.

@hbhalodia
Copy link
Contributor Author

Hi @westonruter, Should we work on this issue? as we have merged, the PR related to the REST API class for URL metrics? Or should we wait for the releases for the plugins in Optimization Detective and Image Priortizer?

@westonruter
Copy link
Member

@hbhalodia it can be worked on now. I've created the release branch for the release today, so if this gets merged it won't go out into the next release.

We probably will need to update the od_init action handler in Image Prioritizer to require Optimization Detective 1.0.0-beta3 or higher. If not, we should show the upgrade admin notice.

@westonruter
Copy link
Member

In other words, we can just bump the $required_od_version:

function image_prioritizer_init( string $optimization_detective_version ): void {
$required_od_version = '0.9.0';
if ( ! version_compare( (string) strtok( $optimization_detective_version, '-' ), $required_od_version, '>=' ) ) {
add_action(
'admin_notices',
static function (): void {
global $pagenow;
if ( ! in_array( $pagenow, array( 'index.php', 'plugins.php' ), true ) ) {
return;
}
wp_admin_notice(
esc_html__( 'The Image Prioritizer plugin requires a newer version of the Optimization Detective plugin. Please update your plugins.', 'image-prioritizer' ),
array( 'type' => 'warning' )
);
}
);
return;
}

We can also get rid of strtok() here since version_compare() handles pre-release version suffixes:

The function first replaces _, - and + with a dot . in the version strings and also inserts dots . before and after any non number so that for example '4.3.2RC1' becomes '4.3.2.RC.1'. Then it compares the parts starting from left to right. If a part contains special version strings these are handled in the following order: any string not found in this list < dev < alpha = a < beta = b < RC = rc < # < pl = p. This way not only versions with different levels like '4.1' and '4.1.2' can be compared but also any PHP specific version containing development state.

@hbhalodia
Copy link
Contributor Author

Hi @westonruter, I have raised the PR with the required changes - #1943, Can you please review the same.

Thank You,

@hbhalodia
Copy link
Contributor Author

Hi @westonruter, I spent some time resolving the failed unit test, but not able to identify the root caues from the PR. I am guessing it would be related to the part we removed from alias and switch case removal.

Also just to confirm, we need to remove the entire case in switch or just the below part and keep the case as it is.

// TODO: Remove this when no plugins are possibly referring to the url_metrics_group_collection property anymore. 
 	_doing_it_wrong( 
 		esc_html( __CLASS__ . '::$' . $name ), 
 		esc_html( 
 			sprintf( 
 				/* translators: %s is class member variable name */ 
 				__( 'Use %s instead.', 'optimization-detective' ), 
 				__CLASS__ . '::$url_metric_group_collection' 
 			) 
 		), 
 		'optimization-detective 0.7.0' 
 	); 

Thank You,

@westonruter
Copy link
Member

We'll remove the entire case, yes. There should no longer be a url_metrics_group_collection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Bug An existing feature is broken
Projects
Status: Not Started/Backlog 📆
Development

No branches or pull requests

2 participants