Skip to content

Calypsoify: Delay use of core admin functions until they are loaded.#11651

Merged
kraftbj merged 1 commit intomasterfrom
fix/11650-calypsoify-fatal
Mar 25, 2019
Merged

Calypsoify: Delay use of core admin functions until they are loaded.#11651
kraftbj merged 1 commit intomasterfrom
fix/11650-calypsoify-fatal

Conversation

@mdawaffe
Copy link
Copy Markdown
Member

@mdawaffe mdawaffe commented Mar 22, 2019 •
edited
Loading

Fixes #11650

Changes proposed in this Pull Request:

Calypsoify is loaded on wp_loaded, which is before core admin functions are loaded. It's only safe to use core admin functions during or after admin_init.

This PR keeps the current get_user_meta() checks where they are (in wp_loaded), but does the rest of the setup on admin_init. To do that, it juggles other admin_init priorities to ensure everything happens in the same order as before.

Testing instructions:

  1. Go to /wp-admin/post-new.php?calypsoify=1.
  2. Add a title and some content, then click "Preview".

Prior to this PR, the preview tab will spin forever, you'll see some errors in the JS console of the editor tab:

POST …/wp-admin/post.php?post=660&action=edit&meta-box-loader=1&_wpnonce=4ca1d0cb7f&_locale=user 500
Uncaught (in promise) Response {type: "basic", url: "…/wp-admin/post.php…", redirected: false, status: 500, ok: false, …}

and there will be a fatal error in your PHP error log:

PHP Fatal error: Uncaught Error: Call to undefined function use_block_editor_for_post_type() in /var/www/html/wp-content/plugins/jetpack/modules/calypsoify/class.jetpack-calypsoify.php:335

With this PR, the preview tab will eventually load, there will be fewer errors in the JS console of the editor tab, and there will be no PHP fatal errors.

Proposed changelog entry for your changes:

None needed.

@mdawaffe
Calypsoify: Delay use of core admin functions until they are loaded.
f27af2d 
Calypsoify is loaded on `wp_loaded`, which is before core admin functions
are loaded. It's only safe to use core admin functions during or after
`admin_init`.

Keep the current `get_user_meta()` checks where they are in `wp_loaded`,
but do the rest of the setup on `admin_init`. Juggles other `admin_init`
priorities to ensure things happens in the same order.

Fixes #11650
@mdawaffe mdawaffe added this to the 7.2 milestone Mar 22, 2019
@mdawaffe mdawaffe self-assigned this Mar 22, 2019
@mdawaffe mdawaffe requested review from a team and kwight March 22, 2019 02:21
@mdawaffe
Copy link
Copy Markdown
Member Author

mdawaffe commented Mar 22, 2019

This PR's diff is easier to see when ignoring whitespace: https://github.com/Automattic/jetpack/pull/11651/files?w=1

@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Mar 22, 2019

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 2, 2019.
Scheduled code freeze: March 26, 2019

Generated by 🚫 dangerJS against f27af2d

@kwight
Copy link
Copy Markdown
Contributor

kwight commented Mar 22, 2019

Calypsoify is loaded on wp_loaded, which is before core admin functions are loaded. It's only safe to use core admin functions during or after admin_init.

Doh! Silly me – I started down that route, but opened wp-settings.php by habit, saw wp_loaded was the last thing firing with init before it and thought, "nope, that's not it..." 🙃.

Thanks for the heavy lifting, will test and confirm this soon today.

kwight
kwight approved these changes Mar 22, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@kwight kwight left a comment

Choose a reason for hiding this comment

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

Fantastic, works as expected – thank you! <3

@kraftbj kraftbj self-assigned this Mar 22, 2019
@kraftbj
Copy link
Copy Markdown
Contributor

kraftbj commented Mar 22, 2019

I couldn't duplicate the original failure on a fresh JN site, but this PR also works as advertised.

@kraftbj kraftbj added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 22, 2019
@mdawaffe
Copy link
Copy Markdown
Member Author

mdawaffe commented Mar 22, 2019

I couldn't duplicate the original failure

…strange. What branch were you testing for JN?

@kwight
Copy link
Copy Markdown
Contributor

kwight commented Mar 22, 2019 •
edited
Loading

I couldn't duplicate the original failure

Looks like you need your site connected for the module to load?

@kraftbj
Copy link
Copy Markdown
Contributor

kraftbj commented Mar 25, 2019

Must have tested it from an older branch before the breaking PR or something. Tried it fresh and able to duplicate/resolve with this PR.

@kraftbj kraftbj merged commit f232b97 into master Mar 25, 2019
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 25, 2019
@kraftbj kraftbj deleted the fix/11650-calypsoify-fatal branch March 25, 2019 14:37
@mdawaffe mdawaffe mentioned this pull request May 23, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kraftbj kraftbj kraftbj approved these changes

+1 more reviewer

@kwight kwight kwight approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kraftbj kraftbj

@mdawaffe mdawaffe

Labels

[Feature] Calypsoify [Pri] BLOCKER

Projects

None yet

Milestone

7.2

Development

Successfully merging this pull request may close these issues.

5 participants

@mdawaffe @jetpackbot @kwight @kraftbj @matticbot