De-duplicate logic between REST API and URL Metrics post type #1867

ShyamGadde · 2025-02-12T01:14:59Z

Summary

Fixes #1858

Relevant technical choices

Introduced OD_URL_Metrics_Post_Type::update_post( string $slug, string $post_title, OD_URL_Metric_Group_Collection $url_metric_group_collection ) to replace store_url_metric(), eliminating redundant logic.
Updated REST API to use update_post() instead of store_url_metric(), ensuring the existing OD_URL_Metric_Group_Collection is passed directly.
Moved store_url_metric() to Optimization_Detective_Test_Helpers for test compatibility.
Refactored affected tests to align with the new method structure.

Signed-off-by: Shyamsundar Gadde <[email protected]>

codecov · 2025-02-12T01:23:57Z

Codecov Report

Attention: Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.59%. Comparing base (4a79036) to head (e8cd6bd).
Report is 13 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...lugins/optimization-detective/storage/rest-api.php	75.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk    #1867   +/-   ##
=======================================
  Coverage   69.59%   69.59%           
=======================================
  Files          86       86           
  Lines        6985     6983    -2     
=======================================
- Hits         4861     4860    -1     
+ Misses       2124     2123    -1

Flag	Coverage Δ
multisite	`69.59% <94.44%> (+<0.01%)`	⬆️
single	`39.62% <0.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Shyamsundar Gadde <[email protected]>

plugins/optimization-detective/storage/class-od-url-metrics-post-type.php

…etric Signed-off-by: Shyamsundar Gadde <[email protected]>

Signed-off-by: Shyamsundar Gadde <[email protected]>

github-actions · 2025-02-12T08:18:14Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ShyamGadde <[email protected]>
Co-authored-by: westonruter <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

plugins/optimization-detective/storage/class-od-url-metrics-post-type.php

westonruter · 2025-02-12T17:38:33Z

plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php

+		$post_data = array(
+			'post_title' => $new_url_metric->get_url(),
+		);
+
+		$post = OD_URL_Metrics_Post_Type::get_post( $slug );
+		if ( $post instanceof WP_Post ) {
+			$post_data['ID']        = $post->ID;
+			$post_data['post_name'] = $post->post_name;
+			$url_metrics            = OD_URL_Metrics_Post_Type::get_url_metrics_from_post( $post );
+		} else {
+			$post_data['post_name'] = $slug;
+			$url_metrics            = array();
+		}
+
+		$etag = $new_url_metric->get_etag();
+
+		$group_collection = new OD_URL_Metric_Group_Collection(
+			$url_metrics,
+			$etag,
+			od_get_breakpoint_max_widths(),
+			od_get_url_metrics_breakpoint_sample_size(),
+			od_get_url_metric_freshness_ttl()
+		);
+
+		try {
+			$group = $group_collection->get_group_for_viewport_width( $new_url_metric->get_viewport_width() );
+			$group->add_url_metric( $new_url_metric );
+		} catch ( InvalidArgumentException $e ) {
+			return new WP_Error( 'invalid_url_metric', $e->getMessage() );
+		}
+
+		$post_data['post_content'] = wp_json_encode(
+			$group_collection->get_flattened_url_metrics(),
+			JSON_UNESCAPED_SLASHES // No need for escaping slashes since this JSON is not embedded in HTML.
+		);
+		if ( ! is_string( $post_data['post_content'] ) ) {
+			return new WP_Error( 'json_encode_error', json_last_error_msg() );
+		}
+
+		$has_kses = false !== has_filter( 'content_save_pre', 'wp_filter_post_kses' );
+		if ( $has_kses ) {
+			// Prevent KSES from corrupting JSON in post_content.
+			kses_remove_filters();
+		}
+
+		$post_data['post_type']   = OD_URL_Metrics_Post_Type::SLUG;
+		$post_data['post_status'] = 'publish';
+		$slashed_post_data        = wp_slash( $post_data );
+		if ( isset( $post_data['ID'] ) ) {
+			$result = wp_update_post( $slashed_post_data, true );
+		} else {
+			$result = wp_insert_post( $slashed_post_data, true );
+		}
+
+		if ( $has_kses ) {
+			kses_init_filters();
+		}
+
+		return $result;


Couldn't this be simplified now to just construct the OD_URL_Metric_Group_Collection, add the URL Metric to it, and then pass it to OD_URL_Metrics_Post_Type::update_post()?

Makes sense. Done in aaf48f6.

Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter

Sorry for the delay! Just a couple minor things left.

plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php

plugins/optimization-detective/storage/class-od-url-metrics-post-type.php

plugins/optimization-detective/tests/class-optimization-detective-test-helpers.php

plugins/optimization-detective/tests/storage/test-class-od-url-metrics-post-type.php

Co-authored-by: Weston Ruter <[email protected]>

…andling Co-authored-by: Weston Ruter <[email protected]>

Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter

Thank you!

ShyamGadde added 3 commits February 12, 2025 06:30

Replace the store_url_metric method with update_post

d83801e

Signed-off-by: Shyamsundar Gadde <[email protected]>

Add test case for update_post method

b1ccc83

Signed-off-by: Shyamsundar Gadde <[email protected]>

Move store_url_metric method to Optimization_Detective_Test_Helpers

4909aed

Signed-off-by: Shyamsundar Gadde <[email protected]>

ShyamGadde added [Type] Enhancement A suggestion for improvement of an existing feature [Plugin] Optimization Detective Issues for the Optimization Detective plugin labels Feb 12, 2025

ShyamGadde added this to the optimization-detective 1.0.0-beta3 milestone Feb 12, 2025

Remove extra blank lines

f382547

Signed-off-by: Shyamsundar Gadde <[email protected]>

ShyamGadde commented Feb 12, 2025

View reviewed changes

plugins/optimization-detective/storage/class-od-url-metrics-post-type.php Outdated Show resolved Hide resolved

ShyamGadde added 2 commits February 12, 2025 12:51

Sort URL Metrics in decreasing order by timestamp to get latest URL M…

cf0ea0e

…etric Signed-off-by: Shyamsundar Gadde <[email protected]>

Add codeCoverageIgnore to logically unreachable exception handler

010f621

Signed-off-by: Shyamsundar Gadde <[email protected]>

ShyamGadde marked this pull request as ready for review February 12, 2025 08:18

ShyamGadde requested review from adamsilverstein and felixarntz as code owners February 12, 2025 08:18

ShyamGadde requested a review from westonruter February 12, 2025 08:18

westonruter reviewed Feb 12, 2025

View reviewed changes

plugins/optimization-detective/storage/class-od-url-metrics-post-type.php Show resolved Hide resolved

westonruter reviewed Feb 12, 2025

View reviewed changes

ShyamGadde added 2 commits February 13, 2025 08:05

Refactor store_url_metric() to reuse update_post()

aaf48f6

Signed-off-by: Shyamsundar Gadde <[email protected]>

Ensure update_post method handles empty URL Metric group collection

2182053

Signed-off-by: Shyamsundar Gadde <[email protected]>

ShyamGadde requested a review from westonruter February 13, 2025 04:02

Merge branch 'trunk' into update/logic-for-updating-url-metrics

6d3ddc5

westonruter requested changes Feb 27, 2025

View reviewed changes

ShyamGadde and others added 3 commits February 27, 2025 09:32

Improve docblocks for clarity

fe93912

Co-authored-by: Weston Ruter <[email protected]>

Simplify URL Metric addition logic and remove unnecessary exception h…

b75838f

…andling Co-authored-by: Weston Ruter <[email protected]>

Remove unused variable

e8cd6bd

Signed-off-by: Shyamsundar Gadde <[email protected]>

westonruter approved these changes Feb 27, 2025

View reviewed changes

westonruter merged commit c17a3a3 into WordPress:trunk Feb 27, 2025
16 checks passed

westonruter mentioned this pull request Feb 28, 2025

Fix triggering post update actions after storing a URL Metric and refactor REST API endpoint logic into class #1865

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

De-duplicate logic between REST API and URL Metrics post type #1867

De-duplicate logic between REST API and URL Metrics post type #1867

Uh oh!

ShyamGadde commented Feb 12, 2025

Uh oh!

codecov bot commented Feb 12, 2025 •

edited

Loading

Uh oh!

Uh oh!

github-actions bot commented Feb 12, 2025

Uh oh!

Uh oh!

westonruter Feb 12, 2025

Uh oh!

ShyamGadde Feb 13, 2025

Uh oh!

westonruter left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

westonruter left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

De-duplicate logic between REST API and URL Metrics post type #1867

De-duplicate logic between REST API and URL Metrics post type #1867

Uh oh!

Conversation

ShyamGadde commented Feb 12, 2025

Summary

Relevant technical choices

Uh oh!

codecov bot commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

github-actions bot commented Feb 12, 2025

Uh oh!

Uh oh!

westonruter Feb 12, 2025

Choose a reason for hiding this comment

Uh oh!

ShyamGadde Feb 13, 2025

Choose a reason for hiding this comment

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

westonruter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Feb 12, 2025 •

edited

Loading