New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meta Boxes: Only attempt apiRequest preload if path (fix meta box save breakage) #6130

Merged
merged 2 commits into from Apr 12, 2018

Conversation

Projects
None yet
3 participants
@aduth
Member

aduth commented Apr 11, 2018

Related: #6076 (comment)

This pull request seeks to resolve an error which prevents saving a post while a meta box is present. See #6076 (comment) for more context on the reason for the error.

Included is a new end-to-end test suite for meta boxes. Currently this only includes a verification that the post is saveable after dirtying, previously failing and now passing with the fix to the wp.apiRequest shim.

Testing instructions:

Verify in your own browser that a post with meta boxes present can be saved.

Ensure end-to-end tests pass:

npm run test-e2e
@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Apr 12, 2018

Thanks for the fix Andrew, it seems that the e2e tests are still failing on travis though

@aduth

This comment has been minimized.

Member

aduth commented Apr 12, 2018

Oddly the tests were passing locally. I adjusted the tests to use a similar technique for awaiting click + navigate, defining the states of the button we expect to occur before triggering the save.

It feels a bit odd, because the call to page.waitForSelector( '.editor-post-save-draft' ), taking place before the save should be satisfied immediately. Reverting the "fix" reveals that the tests fail again, as we would want.

There seems to be some pattern emerging for defining the expected "waitFor" before any interaction which would cause those to be satisfied.

In any case, the tests pass now.

await Promise.all( [
// Transitions between two states "Saving..." -> "Save Draft" (the
// button is always visible while meta are present).

This comment has been minimized.

@youknowriad

youknowriad Apr 12, 2018

Contributor

The button is hidden for 1 second though, we could also check the "saved" state between the two states :)

This comment has been minimized.

@aduth

aduth Apr 12, 2018

Member

The button is hidden for 1 second though, we could also check the "saved" state between the two states :)

Ah, I missed this change come through. Added a test for it in 0c4d522, refactoring the PostSavedState a bit to apply modifier classes.

@youknowriad

LGTM 👍

@c-shultz

This comment has been minimized.

c-shultz commented Apr 12, 2018

I saw that error pop up after rebasing a PR I'm working on.

For what's it's worth, I just tested this PR and confirmed that it fixed the issue I was seeing! 👍

@aduth aduth merged commit 2940053 into master Apr 12, 2018

2 checks passed

codecov/project 44.44% (-0.01%) compared to 73b7f0f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@aduth aduth deleted the fix/meta-box-save branch Apr 12, 2018

@aduth aduth added this to the 2.7 milestone Apr 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment