Use fewer postmeta values

[jdevalk]

This:

	// save summary data as post meta.
	if ( ! add_post_meta( $post_id, '_edac_summary', $summary, true ) ) {
		update_post_meta( $post_id, '_edac_summary', $summary );
	}

	if ( ! add_post_meta( $post_id, '_edac_summary_passed_tests', $summary['passed_tests'], true ) ) {
		update_post_meta( $post_id, '_edac_summary_passed_tests', $summary['passed_tests'] );
	}

	if ( ! add_post_meta( $post_id, '_edac_summary_errors', $summary['errors'], true ) ) {
		update_post_meta( $post_id, '_edac_summary_errors', $summary['errors'] );
	}

	if ( ! add_post_meta( $post_id, '_edac_summary_warnings', $summary['warnings'], true ) ) {
		update_post_meta( $post_id, '_edac_summary_warnings', $summary['warnings'] );
	}

	if ( ! add_post_meta( $post_id, '_edac_summary_ignored', $summary['ignored'], true ) ) {
		update_post_meta( $post_id, '_edac_summary_ignored', $summary['ignored'] );
	}

	if ( ! add_post_meta( $post_id, '_edac_summary_contrast_errors', $summary['contrast_errors'], true ) ) {
		update_post_meta( $post_id, '_edac_summary_contrast_errors', $summary['contrast_errors'] );
	}

could be one post meta value containing an array instead of 6, saving loads and loads of post meta table rows.

[SteveJonesDev]

The reason these are saved as individual post meta is for the custom column sorting in the Pro plugin. https://github.com/equalizedigital/accessibility-checker-pro/blob/develop/includes/custom-columns.php#L71-L116

The post meta _edac_summary is a serialized array in the database, and WordPress doesn't support ordering by serialized array values out of the box. We'll need to look at maybe writing a custom MySQL function or some other solution.

[aristath]

Actually, we can get rid of the individual post-meta, and then implement custom ordering logic without having to write a custom SQL query or function (something which admittedly would overcomplicate things).
Instead, we can manipulate the $wp_query global var in the page, and reorder the posts in it.

So for example, we could add an override here: https://github.com/equalizedigital/accessibility-checker-pro/blob/e1090d24b49e06cf11800c5fd1816a4cacb66867/includes/custom-columns.php#L125, inside the edacp_sortable_columns function, writing something like this:

global $wp_query;
// When to apply the custom ordering
if ( 'title' === $wp_query->query['orderby'] ) {
	// Get the array of posts from the query.
	$posts = $wp_query->posts;

	// Reorder the posts here by applying whatever logic is applicable.
	$ordered_posts = array( $posts[1], $posts[0] );

	// Set the posts in the Query to our custom-ordered ones.
	$wp_query->posts = $ordered_posts;
}

This way, we can run the default SQL query and then use some relatively simple PHP logic to order the posts accordingly.

Of course the edacp_sortable_columns method may not be the ideal place for this tweak, but for the sake of this demonstration/POC it's fine.

[boonedev]

@aristath would $wp_query->posts in wp-admin/edit.php be a paginated list (ie. LIMIT 0, 20) so that ordering with php would be for 20 posts instead of all posts?

If that's the case, would storing separate post metadata for each of the sortable custom columns (passed tests, errors, contrast errors, warnings, ignores) and consolidating the rest into a single array be the most efficient option?

[SteveJonesDev]

Based on this feedback we'll only keep the necessary individual post meta fields required for sorting. The rest will be added to a single array meta field.

We'll revisit the sorting later in a nother issues.

[SteveJonesDev]

@boonedev expects PR by Feb 5th.

[SteveJonesDev]

@boonedev now expects PR by EOD Feb 9th.

[SteveJonesDev]

@pattonwebz, I'm reprioritizing this for the next release as it will make significant performance and organizational improvements. I've closed out @boonedev's PR due to not being limited in scope but it's there for review if needed.

This PR should only include a refactor of the post-meta handling. The idea is to have one serialized post-meta field that handles all post-meta. This ideally will have a fallback and not be a breaking change.

Keep in mind that we have three plugins, all with their own uninstall clean-up. That might play a factor in how we handle this.

There are some challenges around sorting admin columns in the Pro plugin which you can see discussion on further up in this thread.

[pattonwebz]

I have dug through the work needed for this and found it a sizable undertaking. The TL;DR is that doing this properly requires rethinking and rearchitecting how the plugins deal with and store their data. It's more complicated than simply swapping out some calls.

The pro plugin relies on dedicated meta fields for column sorting, but I also found that the full site scanner, especially the JS portion, relies on additional meta fields and queries them via direct query calls in several places, complicating any change that puts that data inside of single rows with a shape that needs serialization.

To properly do this work and make it robust enough to support all the plugins and the future will require breaking changes.

I also looked over the work to do a similar thing for data stored in options, and there are fewer complications there. However, the changes are even more far-reaching through the codebase than the post-meta changes. That work is probably best tackled as part of the bigger redesign as well to use fewer keys and provide singular access points for each data type.

[aristath]

@pattonwebz I have not dug into this as deep as you have, so this is a bit of a naive question:
Would it not be possible to add a few filters in order to provide backward-compatibility and avoid breaking changes? 🤔
So something like

add_filter( 'get_post_metadata', function( $value, $object_id, $meta_key, $single, $meta_type ) {
	// Check if we're trying to get the old post meta.
	// If we are, return the new post meta.
	// If not, return the value as is.
	return $value;
}, 10, 5 );

[pattonwebz]

@aristath It's not a naive question at all, a filter like that could work for back compat :).

On the investigation I undertook I opted for a different approach that was a back compat class overlay that effectively was doing this same thing but avoiding needing to filter ever post meta request. It is likely somewhat over engineered but I was also trying to consider a pattern that could be used for post meta as well as options.

A filter for meta is suitable for the places that could be consolidated but I found that it would only actually reduce the total number of post meta in this plugin by 3, keeping at least 6 as separate fields for sorting or querying. The value of doing it doesn't add up as we would still have more individual meta fields than we would consolidate. Consolidating values in this plugin had implications for the pro plugin as there are places it makes direct queries expecting meta fields to exist, and changing those to handle querying serialisable is very brittle. It really needs those places to be refactored to remove the use of direct queries like it is doing currently.

As per the conversation above about keeping the fields used for sorting even more fields were discovered that are used in the same way in the pro plugin (density and scan status).

Solving this properly I feel is really best handled by removing the need to store scan data in the post meta and instead only storing results (as a singular key). That improves the overall scanning system as the primary benefit, reducing the number of post meta rows at the same time as a free bonus.