Make WordPress Core

Opened 5 months ago

Closed 3 weeks ago

Last modified 3 weeks ago

#59636 closed enhancement (wontfix)

Add meta to the revisions screen

Reported by: adamsilverstein's profile adamsilverstein Owned by: adamsilverstein's profile adamsilverstein
Milestone: Priority: normal
Severity: normal Version: 6.4
Component: Revisions Keywords: has-patch
Focuses: Cc:

Description

This ticket is a follow up to #20564 where we added opt-in support for meta revisions.

When meta is opted in to revisions, either at registration or using a filter, and stored as part of the revision storage, we should display that meta value on the revisions screen.

This core code would replace the filtered in display to the footer meta added by Gutenberg - WordPress/gutenberg@3e260aa/packages/block-library/src/footnotes/index.php#L92-L120.

Plugins can still overwrite the default display using the _wp_post_revision_field_{$meta_key} filter, the point of the change is that any revisioned value will be displayed by default.

This display is already covered by Gutenberg's "works with revisions" E2E test, this test should continue to pass even after this filter code is removed from Gutenberg. We can easily bring those tests over for core as well now that we use the same testing framework.

Attachments (1)

59636.diff (8.2 KB) - added by adamsilverstein 5 months ago.

Download all attachments as: .zip

Change History (10)

This ticket was mentioned in PR #5364 on WordPress/wordpress-develop by @adamsilverstein.


5 months ago
#1

  • Keywords has-patch added; needs-patch removed

Follow up to - follow up https://core.trac.wordpress.org/changeset/56745, this PR now includes the logic for displaying the revisioned meta on the revisions screen.

This core code would replace the filtered in display added by Gutenberg - https://github.com/WordPress/gutenberg/blob/3e260aa41d3c50684efac9728bdbba918658d131/packages/block-library/src/footnotes/index.php#L92-L120. Plugins can still overwrite the default display using the _wp_post_revision_field_{$meta_key} filter.

Trac ticket: https://core.trac.wordpress.org/ticket/59636

#2 @adamsilverstein
5 months ago

59636.diff includes changes from the PR.

#3 @jsmoriss
4 months ago

Note that if anything else than a string is returned by _wp_post_revision_field_{$meta_key}, then PHP issues an error:

PHP Warning:  trim() expects parameter 1 to be string, array given in wordpress/wp-includes/formatting.php on line 5506

Since metadata can contain other types than just strings (like arrays, for example), it would be nice if WP could handle those types returned by _wp_post_revision_field_{$meta_key}.

Here is an example of an array formatted using print_r() in the revisions page:

https://surniaulula.com/wp-content/uploads/screenshots/Screen%20Shot%202023-10-30%20at%204.34.00%20PM.png

And the code I used to hook those filters and format the metadata array:

https://github.com/surniaulula/wpsso/blob/master/lib/abstract/wp-meta.php#L250-L326

js.

Last edited 4 months ago by jsmoriss (previous) (diff)

@adamsilverstein commented on PR #5364:


4 months ago
#4

If you haven't already created a trac ticket for this improvement/enchancement specifically (I didn't find one on a quick search), I think it might make it easier to follow that way.

Yes good idea, I will go ahead and open up a follow up ticket to add this in 6.5

Here is the follow up ticket - https://core.trac.wordpress.org/ticket/59636

@adamsilverstein commented on PR #5364:


4 months ago
#5

If you haven't already created a trac ticket for this improvement/enchancement specifically (I didn't find one on a quick search), I think it might make it easier to follow that way.

Yes good idea, I will go ahead and open up a follow up ticket to add this in 6.5

Here is the follow up ticket - https://core.trac.wordpress.org/ticket/59636

#6 follow-up: @adamsilverstein
3 weeks ago

  • Milestone 6.5 deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

@jsmoriss

Was this already resolved elsewhere?

3 months ago
Note that if anything else than a string is returned by _wp_post_revision_field_{$meta_key}, then PHP issues an error:

Reconsidering whether we should add this automatically - developers would be better suited to know what they want to display on this screen, core should only handle fields we add directly (eg. footnotes). Closing as Wontfix, please re-open if I have missed something or this is still desired.

@adamsilverstein commented on PR #5364:


3 weeks ago
#7

@Mamaduka rethinking if we should add fields automatically, see my comment on the trac ticket. let me know what you think.

#8 in reply to: ↑ 6 @jsmoriss
3 weeks ago

Replying to adamsilverstein:

@jsmoriss

Was this already resolved elsewhere?

3 months ago
Note that if anything else than a string is returned by _wp_post_revision_field_{$meta_key}, then PHP issues an error:

Reconsidering whether we should add this automatically - developers would be better suited to know what they want to display on this screen, core should only handle fields we add directly (eg. footnotes). Closing as Wontfix, please re-open if I have missed something or this is still desired.

I don't think you understand the issue - plugin authors CANNOT add anything to this screen if the meta is not a string. A lot of plugin authors store values in arrays to optimize querying the post meta (one query vs many). If this isn't fixed, plugin authors cannot show the post meta differences if the post meta is an array.

As a concrete example, these two filters cannot be enabled:

https://github.com/surniaulula/wpsso/blob/main/lib/abstract/wp-meta.php#L176-L178

js.

@Mamaduka commented on PR #5364:


3 weeks ago
#9

Thank you, @adamsilverstein! Your reasoning makes sense to me.

Note: See TracTickets for help on using tickets.