WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 years ago

#39459 reviewing defect (bug)

Database upgrade is triggered on AJAX request

Reported by: schlessera Owned by: schlessera
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7
Component: Upgrade/Install Keywords: good-first-bug has-patch 2nd-opinion
Focuses: Cc:
PR Number:

Description

After an update of WordPress Core from 4.5 to 4.7 on a multisite (not sure whether that's relevant or not here), my AJAX requests started failing. Looking at the responses I got back from the server showed that the server tried to trigger a database upgrade procedure on each AJAX request.

I'm not sure whether this is intended behavior or not, but it was certainly unexpected for me.

Should an AJAX request trigger the upgrade procedure? As the upgrade procedure is not meant to be needed immediately after an update, my first guess would be that AJAX requests should not trigger it and be able to complete successfully.

Attachments (1)

39459.diff (844 bytes) - added by jgrodel 2 years ago.
checks if doing ajax before doing upgrades

Download all attachments as: .zip

Change History (6)

#1 @johnbillion
3 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

#2 @adamsilverstein
3 years ago

  • Keywords good-first-bug added

@jgrodel
2 years ago

checks if doing ajax before doing upgrades

#3 @jgrodel
2 years ago

  • Keywords has-patch added; needs-patch removed

#4 @johnbillion
2 years ago

  • Owner set to schlessera
  • Status changed from new to reviewing

#5 @schlessera
2 years ago

  • Keywords 2nd-opinion added

Patch looks good and tests are still passing.

However, I would personally prefer to have the multi-line conditions be split up before the boolean operator, so that these are the first element for each line. I think this is easier to read and reason about:

} elseif ( ! wp_doing_ajax()
        && get_option( 'db_version' ) != $wp_db_version
        && empty($_POST)
) {

I didn't find anything conclusive in the handbook, so not sure whether this is defined somewhere.

Also, I couldn't find integration tests that test mechanisms like this. What is the usual approach for testing something similar?

Note: See TracTickets for help on using tickets.