xwp / wp-customize-snapshots

Customize Snapshots WordPress Plugin
https://wordpress.org/plugins/customize-snapshots/
52 stars 11 forks source link

Ensure that revisions get created when changesets are updated via explicit saves #111

Closed westonruter closed 7 years ago

westonruter commented 7 years ago

Found a bug with 0.6.0-rc1 whereby revisions were no longer being made as expected when explicitly clicking the Save Draft button in the UI. The opt-in for creating a revision in core is currently indicated by whether or not a customize_changeset_status param is present. This is not ideal, but since the param is getting stripped by Customize Snapshots if the status is the same as the current status, I added a workaround for this to explicitly indicate the need to create a revision via another channel. There is probably a better solution for this, not at least to allow a save_revision to be passed when calling wp.customize.previewer.save().

See: https://github.com/WordPress/wordpress-develop/blob/8900e2466e3a01c9c228ac31784aa70e8dcf3137/src/wp-includes/class-wp-customize-manager.php#L2279

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.06%) to 76.923% when pulling e5aea6adebed0348c7b12624411649b618d6093d on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.

PatelUtkarsh commented 7 years ago

I think the code was adding status(revision) to request only when we change status from drop down. @sayedtaqui Can you update such that it pass status when saved explicitly?

PatelUtkarsh commented 7 years ago

I think this is a better approach than doing it via just JS.

mohdsayed commented 7 years ago

I think checking the param customize_snapshots_create_revision is smarter than removing customize_changeset_status plus this

isSameStatus = api.state( 'changesetStatus' ).get() === originalOptions.data.customize_changeset_status

would return true in case of existing changeset which has draft status so creating revision for the first time makes perfect sense

Looks good đź‘Ť

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.06%) to 76.923% when pulling 66b6bb095d04c0954562241f98921e67ca980d1d on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.

westonruter commented 7 years ago

@sayedtaqui I'm noticing something else here that is unexpected. If I open the customizer, change a setting, and then save the changeset as a draft; then if I click the edit icon, and then try supplying a title the changed title won't get saved until I try making another change to settings. In other words, it doesn't seem that the title changes are getting sent on change as expected.

Are you able to tie up the lose ends on this ticket, including sending the flag to indicate a revision should be made, as well as ensuring changes to the title cause a title update to be sent? Also, related is #112 where the date fields are being unexpectedly shown.

mohdsayed commented 7 years ago

Sure working on them.

mohdsayed commented 7 years ago

try supplying a title the changed title won't get saved until I try making another change to settings. In other words, it doesn't seem that the title changes are getting sent on change as expected.

@westonruter The title would not get saved if the date is not of future, once user updates the date to be of future, it will start auto-saving, but yes it should be happening only in case of future, which I have updated. Now working on the main issue..

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 77.005% when pulling 93ad522a7d5b21adeaea4f735f271b13a7016b76 on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.

mohdsayed commented 7 years ago

@westonruter I wasn't able to finish it today, and I am going on a leave till next Tuesday, so may be @PatelUtkarsh can continue on this PR or I can do it when I come back. đź‘‹

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.04%) to 76.824% when pulling 4b69f1f0db1e08c745b6f067b5b53cfbb579b8ff on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.04%) to 76.824% when pulling 4b69f1f0db1e08c745b6f067b5b53cfbb579b8ff on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.04%) to 76.824% when pulling d3166d4ce369a7ac39391cf02bb9ce5909740d08 on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.04%) to 76.824% when pulling 7913f93b56beb938006ea9f08f92e5c14ef9ac45 on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.04%) to 76.824% when pulling a268785d309b010d4ba8a4d019bce4a1bc009142 on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.

mohdsayed commented 7 years ago

@westonruter Ready for review.

westonruter commented 7 years ago

@sayedtaqui On 4.6 I'm getting an error if I Save a snapshot, then make a change, and then hit Update. A modal pops open with “The snapshot could not be saved”, with the Ajax request returning with a bad_schedule_time error.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.02%) to 76.878% when pulling 383787c80e6c96a30ccd34876b5089f071cefd83 on bugfix/create-revisions into 7910d6bc1f6b06d33b541bd241fa7a0b37a81242 on develop.