Skip to content

Conversation

vcanales
Copy link
Member

@vcanales vcanales commented Apr 26, 2023

Related. #294 (comment)
I've been looking into why the Export function is broken on the site editor, and the problem is two-fold.
First, this error was thrown:

[26-Apr-2023 20:52:44 UTC] PHP Fatal error:  Uncaught Error: Call to undefined function download_url() in /var/www/html/wp-content/plugins/create-block-theme/admin/create-theme/theme-zip.php:114

This happens because when hitting the endpoint, theme-zip.php is not within the admin context, so wp-admin/includes.php is never included, and download_url never defined.

It can be solved by modifying add_media_to_zip to import wp-admin/includes/admin.php when needed, although we might want to add that somewhere up the chain in case other admin functions are made available through the REST API.

Second, uri has a different name in the frontend app — theme_uri — and tags_custom is absent.

[26-Apr-2023 20:52:44 UTC] PHP Warning:  Undefined array key "uri" in /var/www/html/wp-content/plugins/create-block-theme/admin/class-create-theme.php on line 312
[26-Apr-2023 20:52:44 UTC] PHP Warning:  Undefined array key "tags_custom" in /var/www/html/wp-content/plugins/create-block-theme/admin/class-create-theme.php on line 315

@vcanales vcanales changed the base branch from try/clone-and-activate to trunk April 26, 2023 22:00
@vcanales vcanales force-pushed the try/clone-and-activate branch from e2152a8 to fd16416 Compare April 26, 2023 22:04
Vicente Canales added 2 commits April 26, 2023 18:05
Placeholder for what should be a comma-separated list of custom tags,
currently not editable in the form, but required by the endpoint.
@vcanales vcanales force-pushed the try/clone-and-activate branch from fd16416 to baa863f Compare April 26, 2023 22:06
Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this!

This fixes the error for me, I'm no longer seeing the issue I saw when testing #294 🎉

However, I'm not sure about including all of admin.php under the is_admin() check. According to this comment, we might be able to use this instead:

if ( ! function_exists( 'download_url' ) ) {
	require_once ABSPATH . 'wp-admin/includes/file.php';
}

Did you try this file already?

Co-authored-by: Sarah Norris <sarah@sekai.co.uk>
@vcanales
Copy link
Member Author

@mikachan good point; yes, file.php is enough. Changed on 6f97d1d

Copy link
Member

@mikachan mikachan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mikachan mikachan merged commit 2fdde23 into WordPress:trunk Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants