Include attachments and featured images for posts being exported #5720

nate-allen · 2023-11-29T21:43:01Z

I closed my previous PR on this issue because it was stuck on a failed PHP 5.6 test that doesn't apply anymore.

This pull request fixes an issue where attachments are not included when exporting posts, unless you choose the "all" option.

A unit test is also included.

Trac ticket: 17379

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

src/wp-admin/includes/export.php

hellofromtonya · 2024-02-13T20:10:48Z

Request: Have Tools>Export "specific author" generate the same metadata as the "all content" export, so that Export "specific author" will also include metadata reference to the Featured Image.

The Trac ticket is focused on exporting for a "specific author" rather than all authors. The PR works for both. Is this intentional to include when no author is selected (i.e. to do all authors)?

If yes it should also work for all authors, what might be the risks or impacts on sites with a large or massive amounts of content?

github-actions · 2024-02-13T20:11:00Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props nateallen, hellofromtonya, petitphp, mukesh27.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

nate-allen · 2024-02-13T20:37:56Z

The Trac ticket is focused on exporting for a "specific author" rather than all authors. The PR works for both. Is this intentional to include when no author is selected (i.e. to do all authors)?

The Trac ticket mentions the bug happens when you filter by specific author, but it actually happens when you do any filter other than "All content"

This patch fixes that by getting all the attachments for the posts being exported if $args['content'] isn't all. I'll update the patch to not do this if the posts being exported are attachment

If yes it should also work for all authors, what might be the risks or impacts on sites with a large or massive amounts of content?

The patch takes this into consideration by splitting the work into chunks of 20 post IDs at a time.

github-actions · 2024-02-13T21:20:19Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

hellofromtonya · 2024-02-15T13:24:52Z

src/wp-admin/includes/export.php

@@ -144,6 +144,40 @@ function export_wp( $args = array() ) {
 	// Grab a snapshot of post IDs, just in case it changes during the export.
 	$post_ids = $wpdb->get_col( "SELECT ID FROM {$wpdb->posts} $join WHERE $where" );

+	// Get IDs for the attachments of each post, unless all content is already being exported.
+	if ( ! in_array( $args['content'], array( 'all', 'attachment' ), true ) ) {
+		foreach ( array_chunk( $post_ids, 20 ) as $chunk ) {


There's already a chunking approach in the same file:

// Fetch 20 posts at a time rather than loading the entire table into memory. while ( $next_posts = array_splice( $post_ids, 0, 20 ) ) {

Could that approach also be used here for consistency?

Are there benefits to this approach over the other?

The approach you mentioned actually is similar, but it modifies the array instead of creating new ones. I went ahead and used it for consistency but I also needed to create a copy of the $post_ids so it wouldn't get modified.

src/wp-admin/includes/export.php

hellofromtonya · 2024-02-15T13:30:04Z

The Trac ticket mentions the bug happens when you filter by specific author, but it actually happens when you do any filter other than "All content"

Aha thanks @nate-allen for clarifying. Makes sense 👍

tests/phpunit/tests/export/attachments.php

hellofromtonya · 2024-02-15T13:37:46Z

It's really close for commit. Left some comments to finalize it.

src/wp-admin/includes/export.php

petitphp

Thanks for all the work on this. This looks ready for me.

I've left one small nitpick, but happy to see it merge as is.

src/wp-admin/includes/export.php

hellofromtonya · 2024-02-20T14:07:52Z

Thank you @nate-allen and everyone for the follow-ups to code review and suggestions. Reviewing today for commit.

hellofromtonya · 2024-02-20T14:38:12Z

tests/phpunit/tests/export/attachments.php

+ * @group attachments
+ * @ticket 17379
+ */
+class Test_Export_Includes_Attachments extends WP_UnitTestCase {


This test class covers the attachment part of ::export_wp. But the test location, file name, and test class name need to change. Below, I'll share context, information, and handbook links to help explain the changes.

Note: I'll push a commit shortly to take care of these changes.

Sharing the longer details:

The test suite coding standards are located here in the handbook.

What is the function under test?
export_wp()

This means the test class is for export_wp() coverage. Specifically for this bug, the coverage is for only the attachment fix within the export_wp().

Where should the file be located?
tests/phpunit/tests/admin/ directory as part of the admin group of tests.

The export_wp() function lives in the /admin/includes/export.php file as part of the export group of admin functionality.

Notice the test files in the files and functions in /admin/includes/ are located in tests/admin/.

What should the filename be?
exportWp.php.

Notice in the Test Classes standard, the test filename is the function under test's name in camelCase.

What should the test class name be?
Tests_Admin_ExportWp per the same standard, which defines the naming as

Tests_[TheGroup]_FunctionNameUnderTest

where the TheGroup is the root or primary group for the set of tests, i.e. identified by subdirectory which is in this case is the admin group of tests.

Done in 6c9d28d ✅

Currently, attachmets are only exported if you choose to export with the 'all' option. This change will get the attachment IDs of all of the posts being exported when the 'all' option isn't used.

This is more consistent to how it is being done elsewhere in the file and is a bit more memory efficient as well.

hellofromtonya · 2024-02-20T15:26:31Z

To ensure there are no merge conflicts, a rebase is needed against the current WordPress Develop trunk. Will force push shortly.

Update: Done ✅

Changes the test class to cover the export_wp() function, i.e. the function under test. 1. Relocates to tests/admin/. 2. Renames the test file to function's name in camelCase. 3. Renames the test class to follow Tests_[GroupName]_FunctionName. 4. Changes the groups to admin and export. 5. Renames tests to test_should_[what_it_should_do] format. 6. Moves the ticket annotation to the test methods. 7. Adds covers annotation. 8. Adds test failure reason when there are multiple assertions (i.e. to help identify which assertion failed). 9. Uses assertSame() as part of an effort in Core to reduce assertEquals() usage. 10. Switch to dataProvider. 11. Update docblocks.

hellofromtonya

Confirmed tests fail without the changes in export_wp() ✅
Confirmed this PR resolves the issue for the featured images. See test report ✅
Tests are per Core's test coding standards and practices ✅
The changes in export_wp() are using the same code design as preexisting in the export.php file ✅

This patch is ready for commit ✅

hellofromtonya · 2024-02-21T18:17:41Z

Committed via https://core.trac.wordpress.org/changeset/57681.

petitphp reviewed Jan 17, 2024

View reviewed changes

src/wp-admin/includes/export.php Outdated Show resolved Hide resolved

hellofromtonya reviewed Feb 15, 2024

View reviewed changes

src/wp-admin/includes/export.php Outdated Show resolved Hide resolved

hellofromtonya reviewed Feb 15, 2024

View reviewed changes

tests/phpunit/tests/export/attachments.php Outdated Show resolved Hide resolved

mukeshpanchal27 reviewed Feb 19, 2024

View reviewed changes

src/wp-admin/includes/export.php Outdated Show resolved Hide resolved

src/wp-admin/includes/export.php Outdated Show resolved Hide resolved

petitphp approved these changes Feb 19, 2024

View reviewed changes

src/wp-admin/includes/export.php Outdated Show resolved Hide resolved

hellofromtonya reviewed Feb 20, 2024

View reviewed changes

nate-allen added 10 commits February 20, 2024 09:25

Get attachment IDs for each post being exported

5ec1029

Currently, attachmets are only exported if you choose to export with the 'all' option. This change will get the attachment IDs of all of the posts being exported when the 'all' option isn't used.

Add unit test to ensure that attachments are included

5294234

Add newline at end of file

380156e

Update condition to skip if export is 'all' or 'attachment'

9be7e69

Use while loop and array_splice to chunk the data

0fa10ae

This is more consistent to how it is being done elsewhere in the file and is a bit more memory efficient as well.

Create placeholders to be used in multiple places

1084e78

Create a copy of post_ids to avoid modifying the original

462c84e

Add additional unit tests

8b67134

Each array value should start on a new line

382c7ba

Add full stop at end of comments

d91af27

hellofromtonya force-pushed the trac-17379 branch from 1257009 to 6c9d28d Compare February 20, 2024 23:38

Remove {} around $wpdb->postmeta

5f7c6bf

hellofromtonya approved these changes Feb 21, 2024

View reviewed changes

hellofromtonya closed this Feb 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include attachments and featured images for posts being exported #5720

Include attachments and featured images for posts being exported #5720

nate-allen commented Nov 29, 2023

hellofromtonya commented Feb 13, 2024

github-actions bot commented Feb 13, 2024 •

edited

Loading

nate-allen commented Feb 13, 2024

github-actions bot commented Feb 13, 2024

hellofromtonya Feb 15, 2024

nate-allen Feb 15, 2024

hellofromtonya commented Feb 15, 2024

hellofromtonya commented Feb 15, 2024

petitphp left a comment

hellofromtonya commented Feb 20, 2024

hellofromtonya Feb 20, 2024 •

edited

Loading

hellofromtonya Feb 20, 2024

hellofromtonya commented Feb 20, 2024 •

edited

Loading

hellofromtonya left a comment

hellofromtonya commented Feb 21, 2024

Include attachments and featured images for posts being exported #5720

Include attachments and featured images for posts being exported #5720

Conversation

nate-allen commented Nov 29, 2023

hellofromtonya commented Feb 13, 2024

github-actions bot commented Feb 13, 2024 • edited Loading

nate-allen commented Feb 13, 2024

github-actions bot commented Feb 13, 2024

Test using WordPress Playground

Some things to be aware of

hellofromtonya Feb 15, 2024

Choose a reason for hiding this comment

nate-allen Feb 15, 2024

Choose a reason for hiding this comment

hellofromtonya commented Feb 15, 2024

hellofromtonya commented Feb 15, 2024

petitphp left a comment

Choose a reason for hiding this comment

hellofromtonya commented Feb 20, 2024

hellofromtonya Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

hellofromtonya Feb 20, 2024

Choose a reason for hiding this comment

hellofromtonya commented Feb 20, 2024 • edited Loading

hellofromtonya left a comment

Choose a reason for hiding this comment

hellofromtonya commented Feb 21, 2024

github-actions bot commented Feb 13, 2024 •

edited

Loading

hellofromtonya Feb 20, 2024 •

edited

Loading

hellofromtonya commented Feb 20, 2024 •

edited

Loading