UpdraftPlus plugins doesn't fully work in Playground #1272
Comments
adamziel
added
[Type] Enhancement
|
In the SQLite plugin we should either:
In v1 we could use a hardcoded mapping of variable -> value. In v2 we could create a SQLite database table and source the data from there, also supporting |
|
I created WordPress/sqlite-database-integration#104 for this and added some notes over the weekend. Planning to work on this today. |
|
I ended up fixing the unit tests for sqlite-database-integration today before adding a test to prove these queries are failing. After adding failing test(s), my general plan is:
After that, we can consider allowing SET to simply modify this mapping. This seems straightforward for |
|
NOTE: It turns out @@persist vars are "not permitted in expressions", which I believe includes expressions within SELECT statements based on this doc: |
|
Started a PR with failing tests for this issue here: WordPress/sqlite-database-integration#109 As a v1, let's start with support for the specific variables used by UpdraftPlus, with an eye to broaden support over time. The set I found in a recent checkout of UpdraftPlus is: |
|
It turns out that sqlite-database-integration already had a hack to avoid failures for queries for } elseif (
strpos( $updated_query, '@@SESSION.sql_mode' ) !== false
|| strpos( $updated_query, 'CONVERT( ' ) !== false
) {
/*
* If the query contains a function that is not supported by SQLite,
* return a dummy select. This check must be done after the query
* has been rewritten to use parameters to avoid false positives
* on queries such as `SELECT * FROM table WHERE field='CONVERT('`.
*/
$updated_query = 'SELECT 1=0';
$params = array();It would be great to have actual support for working with some of these vars, but as a v0, maybe we should just broaden the condition to match all I don't yet know how to test dev version of sqlite-database-integration with Playground but plan to find out and test with this change: |
|
I tested with the above workaround, and there are no more query errors. Of course, faking the query is not the same as the real thing, but perhaps avoiding the errors in the meantime is a net gain.
@wojtekn I tested UpdraftPlus on a WordPress.com site, and am getting the same errors. There are no query errors, but the same JS errors remain so that it is not possible to trigger a backup from the admin page you linked to. |
The purpose of this PR is to avoid errors when selecting MySQL system variables. Fixes #104. Related to: WordPress/wordpress-playground#1272 - "UpdraftPlus plugins doesn't fully work in Playground."
|
The sqlite-database-integration plugin has been updated to at least tolerate selecting MySQL system vars, and Playground will pick up the changes when rebuilding WordPress. Will see if I can trigger that workflow rather than waiting on a schedule. |
|
@wojtekn with the latest changes to sqlite-database-integration, there are no more SQL var errors here (blueprint with updraft plus and the admin page). Since the WordPress zips have been updated and deployed with the latest sqlite-database-integration, I believe this means that wp-now and Studio get the fix when doing new WP downloads. Is that correct? |
|
I believe this can be closed now, great work @brandonpayton! @wojtekn feel free to reopen if any other issues come up. |
|
Thanks @brandonpayton !
Currently, we prepackage the SQLite plugin with Studio, so it won't be automatically updated for current installations. The new Studio release will include the new SQLite version. We are going to change it under https://github.com/Automattic/dotcom-forge/issues/6939 |
|
I've noted that the Full backup logSteps to Reproduce
@adamziel is it appropriate to reopen this issue based on this discovery? Footnotes
|
|
Thank you for reporting! And yes, it is appropriate. Cc @brandonpayton |
|
@dcalhoun thanks for reporting the SQLite-database-integration fatal. I added this to my list for investigation. Regarding the JS error: This is something I observed when running the plugin directly on a WP Cloud site, so I have not considered it a Playground-related issue. Please let me know if you disagree. |
Yes — thank you for sharing this! I have experienced similar things on non-Playground sites sporadically but kept doubting myself. 😆 While the reproduction I shared in #1272 (comment) is definitely consistent and seemingly related to |
ah, OK. I'll keep my eye out in case SQLite-database-integration is contributing to the incidence of that JS error. |
UpdraftPlus plugin only works in Playground partially.
Steps to reproduce:
/wp-admin/admin.php?page=updraftplusThere is also JS errors displayed
Uncaught ReferenceError: updraftlion is not definedin the console. Clicking different buttons triggers more errors likeUncaught ReferenceError: updraft_backup_dialog_open is not defined, but I'm not sure if those are related to the Playground environment or the plugin itself.The text was updated successfully, but these errors were encountered: