Garbage-collect validated URL posts which have all validation errors reviewed, and remove empty validation error terms

[westonruter]

Feature description

When a URL is validated, an amp_validated_url post type instance is created to capture the results of validating the page for AMP validation issues. Other data is stored in postmeta as well, including the queried object, the stylesheets, and details for any validation error that occured. At present, these validated URLs remain in perpetuity until a user manually selects validated URLs to “forget” (that is, delete). And with #1756 the validated URLs will be created in the background on an ongoing basis. This isn't great because as was reported on the support forum, these posts and their postmeta actually consume a lot of storage. What's more is that the validated URLs actually serve no real purpose once all of the validation errors on them have been reviewed (or if they have no validation errors in the first place).

Therefore, we should introduce a WP Cron task which goes through on a daily basis to forget (delete) all amp_validated_url posts which are older than 1 week and which also have no unreviewed validation errors. Whenever a URL is validated (or re-validated) its post_date is set to the current datetime, so this process will only delete URLs that haven't been touched in a week.

This means of garbage collection is similar to how auto-draft posts are deleted after a week. See wp_delete_auto_drafts(). The difference for our purposes here is that we will skip any posts that have unreviewed validation errors. Note that this can be done by simply joining the wp_posts table with the amp_validation_error taxonomy terms which have the term_group set to 0 (unreviewed kept) or 1 (unreviewed removed).

Additionally, during the same process we also need to delete amp_validation_error terms in the reviewed state (reviewed removed and reviewed kept) that have zero URLs associated with them.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

[westonruter]

Note a related issue for garbage-collecting amp_validation_error taxonomy terms that don't have any associated amp_validated_url posts (i.e. the terms which are empty). This was originally suggested in #1415 but I decided against it because it can mask something important about the validation errors being reported, namely ones which have variable script content in them (e.g. nonces, per #4753). Therefore, it is important for site owners to manually clear out on their own. See #1415 (comment).

[westonruter]

Note a related issue for garbage-collecting amp_validation_error taxonomy terms that don't have any associated amp_validated_url posts (i.e. the terms which are empty). This was originally suggested in #1415 but I decided against it because it can mask something important about the validation errors being reported, namely ones which have variable script content in them (e.g. nonces, per #4753). Therefore, it is important for site owners to manually clear out on their own. See #1415 (comment).

Actually, we should do this for amp_validation_error terms that are in the reviewed state. This will address the concern I had about masking validation errors that occur only once, since such validation errors would almost always be in the new state.

[westonruter]

Actually, humm, this could still be problematic. Consider this scenario:

1. I validate a URL.
2. I evaluate the validation validation errors and decide to leave all as removed. I mark all as reviewed.
3. After a week, the validated URL is post gets garbage-collected since all of its validation errors have been reviewed.

If at this point, the reviewed validation errors are also deleted, then this will have a negative effect: the next time the URL is validated, new validation errors will be created which appear as unreviewed, even though the user had previously marked them as reviewed.

So we can't actually automatically delete validation errors after all, regardless of their status.

[amedina]

Makes sense.

Alternatively, we could consider having a separate screen for Validated URLs which have been "fully reviewed". The goal would be to declutter the information handling for the user, while still keep the validation errors would be kept until they become stale.

Another (related) aspect we have discussed before (I think): could we automatically delete "Empty" validation errors (Clear Empty)?

[westonruter]

Alternatively, we could consider having a separate screen for Validated URLs which have been "fully reviewed". The goal would be to declutter the information handling for the user, while still keep the validation errors would be kept until they become stale.

We could default to showing validated URLs with unreviewed errors:

But still, the underlying need for the issue here is to reduce the database size. So we still should remove the reviewed validated URLs regardless.

Another (related) aspect we have discussed before (I think): could we automatically delete "Empty" validation errors (Clear Empty)?

Yes, that's what we discussed in #1415 but I don't think we can do so because of the reason I closed the issue, and for the reason I outlined in #4779 (comment): once a validated URL was forgotten after all its validation errors were reviewed, then the result would be validation errors which would now be empty. The next time a URL is validated, there would then be unreviewed validation errors all of a sudden.

This would also happen if the user clicked Clear Empty after garbage-collection of reviewed Validated URLs, however. So that's something to consider further.