Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Copy Post: Fix content for non-standard post formats. #11295

Merged
merged 5 commits into from Feb 20, 2019
Merged

Copy Post: Fix content for non-standard post formats. #11295

merged 5 commits into from Feb 20, 2019

Conversation

kwight
Copy link
Contributor

@kwight kwight commented Feb 7, 2019 •
edited

When the new post screen is loading, it believes it is loading a newly-created post with default content, and attempts to set the post format to the default selected in Settings > Writing. Since Copy Post has already taken care of assigning the proper post format, we can take advantage of set_post_format's shortcircuit to stop core from making any changes.

Fixes #11256 .

Testing instructions:

  • Apply this PR.
  • Set the default post format to something other than standard, in Settings > Writing.
  • Set up a test post that has a post format other than standard, and different from the default.
  • Copy the test post.
  • Verify that content is correct, and that the post format of the source post is preserved in the newly-created post.

Proposed changelog entry for your changes:

  • Fixed a bug with Copy Post and its handling of non-standard post formats.

@kwight kwight requested a review from a team as a code owner February 7, 2019 00:22
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello kwight! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D24037-code before merging this PR. Thank you!

@kwight kwight mentioned this pull request Feb 7, 2019
Closed
@jetpackbot
Copy link

jetpackbot commented Feb 7, 2019 •
edited

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: March 5, 2019.
Scheduled code freeze: February 26, 2019

Generated by 🚫 dangerJS against 912cc37

@kwight
Copy link
Contributor Author

kwight commented Feb 7, 2019

This is the first step; the block editor will no longer modify content for post formats. However, the post format itself is still incorrectly set by the block editor to whatever the default is in Settings > Writing > Default Post Format.

@matticbot
Copy link
Contributor

kwight, Your synced wpcom patch D24037-code has been updated.

@matticbot
Copy link
Contributor

kwight, Your synced wpcom patch D24037-code has been updated.

@kwight kwight requested a review from a team February 7, 2019 00:29
@kwight kwight self-assigned this Feb 7, 2019
@matticbot
Copy link
Contributor

kwight, Your synced wpcom patch D24037-code has been updated.

@kwight
Short-circuit core's attempt to set a non-standard post format,
3eab3b2 
believing the post to be a new post (I guess it's not really wrong).
@matticbot
Copy link
Contributor

kwight, Your synced wpcom patch D24037-code has been updated.

@kwight kwight added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Feb 15, 2019
@kwight kwight changed the title Copy Post: Fix content in the block editor for non-standard post Copy Post: Fix content for non-standard post formats. Feb 15, 2019
jeherve
jeherve requested changes Feb 15, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

It works well for me. I only have one small remark.

modules/copy-post.php Outdated Show resolved Hide resolved
@jeherve jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 15, 2019
@jeherve jeherve added this to the 7.1 milestone Feb 15, 2019
mmtr
mmtr reviewed Feb 15, 2019
View reviewed changes
Copy link
Member

@mmtr mmtr left a comment

Choose a reason for hiding this comment

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

Tested manually and it worked as expected :)

@matticbot
Copy link
Contributor

kwight, Your synced wpcom patch D24037-code has been updated.

@kwight kwight added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Feb 15, 2019
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 15, 2019
jeherve
jeherve approved these changes Feb 15, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This is looking good. Merge when ready.

@kwight kwight merged commit 4ca9bf4 into master Feb 20, 2019
@kwight kwight deleted the fix/11256-post-format-handling branch February 20, 2019 20:23
@ghost ghost removed [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 20, 2019
jeherve added a commit that referenced this pull request Feb 22, 2019
@jeherve
changelog: add #11295
a4f275c
@noahtallen noahtallen mentioned this pull request Oct 16, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmtr mmtr mmtr left review comments

@jeherve jeherve jeherve approved these changes

Assignees

@kwight kwight

Labels
[Feature] Copy a Post Touches WP.com Files
Projects
None yet
Milestone
7.1
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kwight @matticbot @jetpackbot @jeherve @mmtr