Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Footnotes: use core’s meta revisioning if available #52988

Merged
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8a169de
Footnotes: use core’s meta revisioning if available
adamsilverstein Jul 26, 2023
f8c55c2
Merge branch 'trunk' into redo-footnote-revisioning
adamsilverstein Sep 8, 2023
59bc54f
Ensure preview menu is closed after opening preview
adamsilverstein Sep 8, 2023
fd9d4a0
Improve docs and add some new conditionals
adamsilverstein Sep 9, 2023
c18cae0
Add test to verify published post meta is unchanged when previewing m…
adamsilverstein Sep 9, 2023
7d944d2
Correct overwriting of live published post meta when previewing
adamsilverstein Sep 9, 2023
fb6d61e
Comment out tests that require core changes to pass
adamsilverstein Sep 11, 2023
c91b95b
Merge branch 'trunk' into redo-footnote-revisioning
adamsilverstein Sep 26, 2023
d7a96e1
Include test to verify published post uneffected when previewing meta…
adamsilverstein Sep 26, 2023
a67bc45
Global styles tests: fire hooks when updating post
adamsilverstein Sep 26, 2023
9b1115d
Improve published post navigation in test
adamsilverstein Sep 26, 2023
037eb5a
Comment out failing test for published post
adamsilverstein Sep 26, 2023
766a777
Remove incorrect comment line
adamsilverstein Sep 26, 2023
cb37c48
Try: remove filters present in core already
adamsilverstein Sep 27, 2023
10edd9c
Merge branch 'trunk' into redo-footnote-revisioning
adamsilverstein Sep 27, 2023
c580c79
Move footnote filter shim to an external compat file
adamsilverstein Sep 27, 2023
82ad972
phpcbf
adamsilverstein Sep 27, 2023
9d2f120
Remove remaining hooks from footnotes that are causing issues
adamsilverstein Sep 27, 2023
1109df2
Revert "Remove remaining hooks from footnotes that are causing issues"
adamsilverstein Sep 28, 2023
7fee4f5
Temporarily disable slashing test
adamsilverstein Sep 28, 2023
2a55084
Restore test to verify that the published post is unchanged after pre…
adamsilverstein Sep 28, 2023
352bba5
Move the meta revision compat code to the footnotes shim file
adamsilverstein Sep 28, 2023
de6882c
remove hooks from footnotes file
adamsilverstein Sep 28, 2023
95f6b42
Ensure functions are guarded against redeclaration.
adamsilverstein Sep 28, 2023
a8e98f6
phpcbf
adamsilverstein Sep 28, 2023
90057fc
phpcbf2
adamsilverstein Sep 28, 2023
c614958
restore typing “3”
adamsilverstein Sep 28, 2023
90d7472
remove extra space
adamsilverstein Sep 28, 2023
6a4f28b
Restore slashed data test
adamsilverstein Sep 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 39 additions & 18 deletions packages/block-library/src/footnotes/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
* Renders the `core/footnotes` block on the server.
*
* @since 6.3.0
* @since 6.4.0 Core added post meta revisions, so this is no longer needed.
adamsilverstein marked this conversation as resolved.
Show resolved Hide resolved
*
* @param array $attributes Block attributes.
* @param string $content Block default content.
Expand Down Expand Up @@ -68,9 +69,10 @@ function register_block_core_footnotes() {
$post_type,
'footnotes',
array(
'show_in_rest' => true,
'single' => true,
'type' => 'string',
'show_in_rest' => true,
'single' => true,
'type' => 'string',
'revisions_enabled' => true,
)
);
}
Expand All @@ -87,6 +89,7 @@ function register_block_core_footnotes() {
* Saves the footnotes meta value to the revision.
*
* @since 6.3.0
* @since 6.4.0 Core added post meta revisions, so this is no longer needed.
*
* @param int $revision_id The revision ID.
*/
Expand All @@ -102,12 +105,15 @@ function wp_save_footnotes_meta( $revision_id ) {
}
}
}
add_action( 'wp_after_insert_post', 'wp_save_footnotes_meta' );
if ( ! function_exists( 'wp_post_revision_meta_keys' ) ) {
add_action( 'wp_after_insert_post', 'wp_save_footnotes_meta' );
}

/**
* Keeps track of the revision ID for "rest_after_insert_{$post_type}".
*
* @since 6.3.0
* @since 6.4.0 Core added post meta revisions, so this is no longer needed.
*
* @global int $wp_temporary_footnote_revision_id The footnote revision ID.
*
Expand All @@ -117,7 +123,9 @@ function wp_keep_footnotes_revision_id( $revision_id ) {
global $wp_temporary_footnote_revision_id;
$wp_temporary_footnote_revision_id = $revision_id;
}
add_action( '_wp_put_post_revision', 'wp_keep_footnotes_revision_id' );
if ( ! function_exists( 'wp_post_revision_meta_keys' ) ) {
add_action( '_wp_put_post_revision', 'wp_keep_footnotes_revision_id' );
}

/**
* This is a specific fix for the REST API. The REST API doesn't update
Expand All @@ -131,6 +139,7 @@ function wp_keep_footnotes_revision_id( $revision_id ) {
* `"rest_after_insert_{$post_type}"` action.
*
* @since 6.3.0
* @since 6.4.0 Core added post meta revisions, so this is no longer needed.
*
* @global int $wp_temporary_footnote_revision_id The footnote revision ID.
*
Expand Down Expand Up @@ -160,13 +169,16 @@ function wp_add_footnotes_revisions_to_post_meta( $post ) {
}
}

add_action( 'rest_after_insert_post', 'wp_add_footnotes_revisions_to_post_meta' );
add_action( 'rest_after_insert_page', 'wp_add_footnotes_revisions_to_post_meta' );
if ( ! function_exists( 'wp_post_revision_meta_keys' ) ) {
add_action( 'rest_after_insert_post', 'wp_add_footnotes_revisions_to_post_meta' );
add_action( 'rest_after_insert_page', 'wp_add_footnotes_revisions_to_post_meta' );
}

/**
* Restores the footnotes meta value from the revision.
*
* @since 6.3.0
* @since 6.4.0 Core added post meta revisions, so this is no longer needed.
*
* @param int $post_id The post ID.
* @param int $revision_id The revision ID.
Expand All @@ -180,10 +192,12 @@ function wp_restore_footnotes_from_revision( $post_id, $revision_id ) {
delete_post_meta( $post_id, 'footnotes' );
}
}
add_action( 'wp_restore_post_revision', 'wp_restore_footnotes_from_revision', 10, 2 );
if ( ! function_exists( 'wp_post_revision_meta_keys' ) ) {
add_action( 'wp_restore_post_revision', 'wp_restore_footnotes_from_revision', 10, 2 );
}

/**
* Adds the footnotes field to the revision.
* Adds the footnotes field to the revisions display.
*
* @since 6.3.0
*
Expand All @@ -197,7 +211,7 @@ function wp_add_footnotes_to_revision( $fields ) {
add_filter( '_wp_post_revision_fields', 'wp_add_footnotes_to_revision' );

/**
* Gets the footnotes field from the revision.
* Gets the footnotes field from the revision for the revisions screen.
*
* @since 6.3.0
*
Expand All @@ -218,6 +232,7 @@ function wp_get_footnotes_from_revision( $revision_field, $field, $revision ) {
* `_wp_put_post_revision` when it creates a new autosave.
*
* @since 6.3.0
* @since 6.4.0 Core added post meta revisions, so this is no longer needed.
*
* @param int|array $autosave The autosave ID or array.
*/
Expand All @@ -242,13 +257,17 @@ function _wp_rest_api_autosave_meta( $autosave ) {
return;
}

update_post_meta( $id, 'footnotes', wp_slash( $body['meta']['footnotes'] ) );
// Can't use update_post_meta() because it doesn't allow revisions.
update_metadata( 'post', $id, 'footnotes', wp_slash( $body['meta']['footnotes'] ) );
}

if ( ! function_exists( 'wp_post_revision_meta_keys' ) ) {
// See https://github.com/WordPress/wordpress-develop/blob/2103cb9966e57d452c94218bbc3171579b536a40/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php#L391C1-L391C1.
add_action( 'wp_creating_autosave', '_wp_rest_api_autosave_meta' );
// See https://github.com/WordPress/wordpress-develop/blob/2103cb9966e57d452c94218bbc3171579b536a40/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php#L398.
// Then https://github.com/WordPress/wordpress-develop/blob/2103cb9966e57d452c94218bbc3171579b536a40/src/wp-includes/revision.php#L367.
add_action( '_wp_put_post_revision', '_wp_rest_api_autosave_meta' );
}
// See https://github.com/WordPress/wordpress-develop/blob/2103cb9966e57d452c94218bbc3171579b536a40/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php#L391C1-L391C1.
add_action( 'wp_creating_autosave', '_wp_rest_api_autosave_meta' );
// See https://github.com/WordPress/wordpress-develop/blob/2103cb9966e57d452c94218bbc3171579b536a40/src/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php#L398.
// Then https://github.com/WordPress/wordpress-develop/blob/2103cb9966e57d452c94218bbc3171579b536a40/src/wp-includes/revision.php#L367.
add_action( '_wp_put_post_revision', '_wp_rest_api_autosave_meta' );

/**
* This is a workaround for the autosave endpoint returning early if the
Expand All @@ -261,6 +280,7 @@ function _wp_rest_api_autosave_meta( $autosave ) {
* course, this is temporary fix.
*
* @since 6.3.0
* @since 6.4.0 Core added post meta revisions, so this is no longer needed.
*
* @param WP_Post $prepared_post The prepared post object.
* @param WP_REST_Request $request The request object.
Expand All @@ -282,5 +302,6 @@ function _wp_rest_api_force_autosave_difference( $prepared_post, $request ) {
$prepared_post->footnotes = '[]';
return $prepared_post;
}

add_filter( 'rest_pre_insert_post', '_wp_rest_api_force_autosave_difference', 10, 2 );
if ( ! function_exists( 'wp_post_revision_meta_keys' ) ) {
add_filter( 'rest_pre_insert_post', '_wp_rest_api_force_autosave_difference', 10, 2 );
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public static function wpSetupBeforeClass( $factory ) {
),
);

wp_update_post( $new_styles_post, true, false );
wp_update_post( $new_styles_post, true, true );

$new_styles_post = array(
'ID' => self::$global_styles_id,
Expand Down Expand Up @@ -160,7 +160,7 @@ public static function wpSetupBeforeClass( $factory ) {
),
);

wp_update_post( $new_styles_post, true, false );
wp_update_post( $new_styles_post, true, true );

$new_styles_post = array(
'ID' => self::$global_styles_id,
Expand Down Expand Up @@ -190,7 +190,7 @@ public static function wpSetupBeforeClass( $factory ) {
),
);

wp_update_post( $new_styles_post, true, false );
wp_update_post( $new_styles_post, true, true );
wp_set_current_user( 0 );
}

Expand Down
22 changes: 19 additions & 3 deletions test/e2e/specs/editor/various/footnotes.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@

await previewPage.close();
await editorPage.bringToFront();
await editor.canvas.click( 'p:text("first paragraph")' );

// Open revisions.
await editor.openDocumentSettingsSidebar();
Expand All @@ -355,7 +356,7 @@
.getByRole( 'region', { name: 'Editor settings' } )
.getByRole( 'button', { name: 'Post' } )
.click();
await page.locator( 'a:text("2 Revisions")' ).click();

Check failure on line 359 in test/e2e/specs/editor/various/footnotes.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 2

[chromium] › editor/various/footnotes.spec.js:284:2 › Footnotes › works with revisions

1) [chromium] › editor/various/footnotes.spec.js:284:2 › Footnotes › works with revisions ──────── TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for locator('a:text("2 Revisions")') ============================================================ 357 | .getByRole( 'button', { name: 'Post' } ) 358 | .click(); > 359 | await page.locator( 'a:text("2 Revisions")' ).click(); | ^ 360 | await page.locator( '.revisions-controls .ui-slider-handle' ).focus(); 361 | await page.keyboard.press( 'ArrowLeft' ); 362 | await page.locator( 'input:text("Restore This Revision")' ).click(); at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/footnotes.spec.js:359:49

Check failure on line 359 in test/e2e/specs/editor/various/footnotes.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 2

[chromium] › editor/various/footnotes.spec.js:284:2 › Footnotes › works with revisions

1) [chromium] › editor/various/footnotes.spec.js:284:2 › Footnotes › works with revisions ──────── Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for locator('a:text("2 Revisions")') ============================================================ 357 | .getByRole( 'button', { name: 'Post' } ) 358 | .click(); > 359 | await page.locator( 'a:text("2 Revisions")' ).click(); | ^ 360 | await page.locator( '.revisions-controls .ui-slider-handle' ).focus(); 361 | await page.keyboard.press( 'ArrowLeft' ); 362 | await page.locator( 'input:text("Restore This Revision")' ).click(); at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/footnotes.spec.js:359:49

Check failure on line 359 in test/e2e/specs/editor/various/footnotes.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 2

[chromium] › editor/various/footnotes.spec.js:284:2 › Footnotes › works with revisions

1) [chromium] › editor/various/footnotes.spec.js:284:2 › Footnotes › works with revisions ──────── Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── TimeoutError: locator.click: Timeout 10000ms exceeded. =========================== logs =========================== waiting for locator('a:text("2 Revisions")') ============================================================ 357 | .getByRole( 'button', { name: 'Post' } ) 358 | .click(); > 359 | await page.locator( 'a:text("2 Revisions")' ).click(); | ^ 360 | await page.locator( '.revisions-controls .ui-slider-handle' ).focus(); 361 | await page.keyboard.press( 'ArrowLeft' ); 362 | await page.locator( 'input:text("Restore This Revision")' ).click(); at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/footnotes.spec.js:359:49
await page.locator( '.revisions-controls .ui-slider-handle' ).focus();
await page.keyboard.press( 'ArrowLeft' );
await page.locator( 'input:text("Restore This Revision")' ).click();
Expand Down Expand Up @@ -391,9 +392,10 @@

await page.keyboard.type( '1' );

// Publish post.
// Publish post with the footnote set to "1".
await editor.publishPost();

// Test previewing changes to meta.
await editor.canvas.click( 'ol.wp-block-footnotes li span' );
await page.keyboard.press( 'End' );
await page.keyboard.type( '2' );
Expand All @@ -408,8 +410,7 @@
await previewPage.close();
await editorPage.bringToFront();

// Test again, this time with an existing revision (different code
// path).
// Test again, this time with an existing revision (different code path).
await editor.canvas.click( 'ol.wp-block-footnotes li span' );
await page.keyboard.press( 'End' );
// Test slashing.
Expand All @@ -420,6 +421,21 @@
// Note: quote will get curled by wptexturize.
await expect(
previewPage2.locator( 'ol.wp-block-footnotes li' )
).toHaveText( '123″ ↩︎' );

Check failure on line 424 in test/e2e/specs/editor/various/footnotes.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 2

[chromium] › editor/various/footnotes.spec.js:385:2 › Footnotes › can be previewed when published

2) [chromium] › editor/various/footnotes.spec.js:385:2 › Footnotes › can be previewed when published Error: Timed out 5000ms waiting for expect(received).toHaveText(expected) Expected string: "123″ ↩︎" Received string: "" Call log: - expect.toHaveText with timeout 5000ms - waiting for locator('ol.wp-block-footnotes li') - waiting for locator('ol.wp-block-footnotes li') 422 | await expect( 423 | previewPage2.locator( 'ol.wp-block-footnotes li' ) > 424 | ).toHaveText( '123″ ↩︎' ); | ^ 425 | 426 | // Verify that the published post is unchanged after previewing changes to meta. 427 | await previewPage2.close(); at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/footnotes.spec.js:424:5

Check failure on line 424 in test/e2e/specs/editor/various/footnotes.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 2

[chromium] › editor/various/footnotes.spec.js:385:2 › Footnotes › can be previewed when published

2) [chromium] › editor/various/footnotes.spec.js:385:2 › Footnotes › can be previewed when published Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: Timed out 5000ms waiting for expect(received).toHaveText(expected) Expected string: "123″ ↩︎" Received string: "" Call log: - expect.toHaveText with timeout 5000ms - waiting for locator('ol.wp-block-footnotes li') - waiting for locator('ol.wp-block-footnotes li') 422 | await expect( 423 | previewPage2.locator( 'ol.wp-block-footnotes li' ) > 424 | ).toHaveText( '123″ ↩︎' ); | ^ 425 | 426 | // Verify that the published post is unchanged after previewing changes to meta. 427 | await previewPage2.close(); at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/footnotes.spec.js:424:5

Check failure on line 424 in test/e2e/specs/editor/various/footnotes.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 2

[chromium] › editor/various/footnotes.spec.js:385:2 › Footnotes › can be previewed when published

2) [chromium] › editor/various/footnotes.spec.js:385:2 › Footnotes › can be previewed when published Retry #2 ─────────────────────────────────────────────────────────────────────────────────────── Error: Timed out 5000ms waiting for expect(received).toHaveText(expected) Expected string: "123″ ↩︎" Received string: "" Call log: - expect.toHaveText with timeout 5000ms - waiting for locator('ol.wp-block-footnotes li') - waiting for locator('ol.wp-block-footnotes li') 422 | await expect( 423 | previewPage2.locator( 'ol.wp-block-footnotes li' ) > 424 | ).toHaveText( '123″ ↩︎' ); | ^ 425 | 426 | // Verify that the published post is unchanged after previewing changes to meta. 427 | await previewPage2.close(); at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/various/footnotes.spec.js:424:5

// Verify that the published post is unchanged after previewing changes to meta.
await previewPage2.close();
await editorPage.bringToFront();
await editor.openDocumentSettingsSidebar();
await page
.getByRole( 'region', { name: 'Editor settings' } )
.getByRole( 'button', { name: 'Post' } )
.click();
await page.locator( 'a:text("View Post")' ).click();

// Verify that the published post footnote still says "1".
await expect( page.locator( 'ol.wp-block-footnotes li' ) ).toHaveText(
'1 ↩︎'
);
} );
} );
Loading