search

equalizedigital / accessibility-checker

GNU General Public License v2.0
15 stars 8 forks source link

Use Fewer post meta values #578

Open pattonwebz opened 4 months ago

pattonwebz commented 4 months ago

This PR aims to reduce the number of different post meta values in favor of storing as much data inside of a single post meta as possible.

The exception is that for sorting by columns some meta still must exist in 2 places.

It reduces the total number of post meta by 5 and introduces a single interface to getting any piece of post meta.

https://github.com/equalizedigital/accessibility-checker/pull/578/files#diff-ac3197d4f34d0b1b8fafdf64365bd3e7cb32db3aa83571ea74fcdfe464adf0eeR118-R122

Closes: #402

SteveJonesDev commented 4 months ago

@pattonwebz, here is more context to the full site scanning not working that @christophermhinds mentioned.

Free: 1.10.2 Pro: 1.6.0 Full site scan works as expected

Free: 1.11.0-alpha.1 Pro: 1.6.0 Full site scan pass two doesn't run No console or log errors

Free: 1.11.0-alpha.1 Pro: Version 1.7.0-alpha.1 Full site scan pass two doesn't run No console errors It does log a deprecation. Should it be doing if both versions are the current alpha? Maybe you need to check for the edac_summary for backward compatibility? [18-Apr-2024 18:02:19 UTC] PHP Deprecated: Function edac_summary is deprecated since version 1.9.0! Use EDAC\Inc\Summary_Generator instead. in /Users/stevejones/Local Sites/edactest/app/public/wp-includes/functions.php on line 6078

Free: 1.10.2 Pro: 1.7.0-alpha.1 Full site scan works as expected Same deprecation log. [18-Apr-2024 18:02:19 UTC] PHP Deprecated: Function edac_summary is deprecated since version 1.9.0! Use EDAC\Inc\Summary_Generator instead. in /Users/stevejones/Local Sites/edactest/app/public/wp-includes/functions.php on line 6078

These findings lead me to believe the issue is in Free: 1.11.0-alpha.1

pattonwebz commented 4 months ago

@SteveJonesDev I found the issue with the JS scan not working but it isn't proving easy to find a solution that doesn't involve rebuilding much of the JS scan checking logic in pro. It uses some direct queries against meta keys to count posts and more direct queries to update the values.

It probably means that the _edac_post_checked_js meta is another key that will need to live outside of the single meta - unless a lot of attention is given to the JS scan logic in pro to refactor avoiding the need for the dedicated meta key. I'll push up the changes after I have done more local testing on it.

I know the pro plugin's refactoring is coming up, so I propose refactoring that logic as part of the overall plugin refactoring work. I can make a card on the pro repo.

pattonwebz commented 3 months ago

After some discussion and undertaking the work required to reduce post meta, it was decided to postpone this. The reduction in post meta rows was minor, but the changes were far-reaching (14 files changed in this PR and the value gained is small. These changes don't entirely account for all the post meta used by the pro plugin, which is an even more significant chunk.

After reviewing the work, it is clear that to get the most value from the changes, it requires redesigning how the suite of plugins handles data entirely, particularly column filtering and how the full site scan—the JS portion especially—deals with their status and tracking. This would be a breaking change requiring a major version bump and not something that can be squeezed into a typical release.