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.

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

Fix bug preventing previewing posts authored by other users #597

Merged
merged 1 commit into from Apr 7, 2020

Conversation

mallorydxw
Copy link
Contributor

@mallorydxw mallorydxw commented Mar 30, 2020

Bug description:

The author of a post can preview changes to their own published posts,
but other users see the content of the published post when previewing.

This is due to checking for the autosaves of $query_args->post_author
instead of the autosaves of get_current_user_id().

Steps to reproduce:

  1. Log in as user A
  2. Create a new post, press "Publish"
  3. Log in as user B
  4. Visit the edit page for the post that was just created
  5. Add to the post's content (without pressing "Update")
  6. Click "Preview"

Expected behaviour:

The content added by user B appears in the preview.

Actual behaviour:

The content added by user B doesn't appear in the preview of the post.

@mallorydxw
Copy link
Contributor Author

@mallorydxw mallorydxw commented Mar 30, 2020

I don't think fix_preview_link_part_three() has any unit tests so I didn't modify any tests.

However one test is failing:

  - Installing composer/installers (v1.7.0): Downloading (connecting...)Downloading (0%)GitHub API limit (0 calls/hr) is exhausted, could not fetch https://api.github.com/repos/composer/installers/zipball/141b272484481432cda342727a427dc1e206bfa0. Create a GitHub OAuth token to go over the API rate limit. You can also wait until ? for the rate limit to reset.
Head to https://github.com/settings/tokens/new?scopes=repo&description=Composer+on+2d5ab9740c13+2020-03-30+1824
to retrieve a token. It will be stored in "/tmp/auth.json" for future use by Composer.
Token (hidden): 

I've been seeing the same errors on my own company's tests that use composer. I'm guessing GitHub has recently lowered their API rate limits.

@cojennin
Copy link
Contributor

@cojennin cojennin commented Apr 2, 2020

I haven't been able to recreate this behavior with WP 5.4 using the block editor or the classic editor. What version of WP are you using? And is it possible to add a screencapture of the behavior you're seeing?

Thanks!

@mallorydxw
Copy link
Contributor Author

@mallorydxw mallorydxw commented Apr 2, 2020

Using WordPress 5.4, and Edit-Flow at 8d9cc3aaa2bfd812c506f3d519e28c392c5fdf11 (master at time of writing). The only active plugin is Edit-Flow, so I'll be using the block editor. The theme is twentytwenty. I've just deleted the database and made a fresh installation. WordPress isn't configured to be a multisite installation.

I follow the above steps. I get the "actual behaviour" described above.

Here's the screenshot with User B logged in, before and after clicking on the "Preview" button.

Screenshot 2020-04-02 at 10 26 34

Screenshot 2020-04-02 at 10 26 38

@cojennin
Copy link
Contributor

@cojennin cojennin commented Apr 3, 2020

@mallorydxw Ah I was able to recreate it now, thanks for sending along those screen captures!

I think based on your findings it seems like it'd be better to just rely on the latest autosave and not pass a user id at all, what do you think?

@cojennin
Copy link
Contributor

@cojennin cojennin commented Apr 5, 2020

@mallorydxw let me know what you think about removing the user id, and feel free to merge whenever you're ready

@mallorydxw
Copy link
Contributor Author

@mallorydxw mallorydxw commented Apr 6, 2020

I think based on your findings it seems like it'd be better to just rely on the latest autosave and not pass a user id at all, what do you think?

I wonder what would happen if user A makes a change and it autosaves, then user B takes over editing and clicks "preview" without making any changes. It feels safer to include the current user's ID.

@mallorydxw let me know what you think about removing the user id, and feel free to merge whenever you're ready

Thanks! Since you trust me to decide I'll go with what seems safer.

@mallorydxw mallorydxw requested a review from cojennin Apr 6, 2020
cojennin
cojennin previously approved these changes Apr 6, 2020
Copy link
Contributor

@cojennin cojennin left a comment

Ah yea, makes sense. LGTM!

@mallorydxw
Copy link
Contributor Author

@mallorydxw mallorydxw commented Apr 7, 2020

Thanks for approving @cojennin!

feel free to merge whenever you're ready

GitHub says I don't have the right permissions: "You’re not authorized to merge this pull request."

@cojennin
Copy link
Contributor

@cojennin cojennin commented Apr 7, 2020

@mallorydxw Ah yea, think it's a protected branch. Try now? Just updated the settings.

@mallorydxw
Copy link
Contributor Author

@mallorydxw mallorydxw commented Apr 7, 2020

@cojennin It's still showing the the same message for me. Do you want to go ahead and merge it?

Bug description:

The author of a post can preview changes to their own published posts,
but other users see the content of the published post when previewing.

This is due to checking for the autosaves of `$query_args->post_author`
instead of the autosaves of `get_current_user_id()`.

Steps to reproduce:

1. Log in as user A
2. Create a new post, press "Publish"
3. Log in as user B
4. Visit the edit page for the post that was just created
5. Add to the post's content (without pressing "Update")
6. Click "Preview"

Expected behaviour:

The content added by user B appears in the preview.

Actual behaviour:

The content added by user B doesn't appear in the preview of the post.
@mallorydxw mallorydxw force-pushed the fix/previewing-others-posts branch from 5b4a3a8 to 233750f Compare Apr 7, 2020
@cojennin cojennin merged commit 58cccda into Automattic:master Apr 7, 2020
1 check was pending
@cojennin
Copy link
Contributor

@cojennin cojennin commented Apr 7, 2020

@mallorydxw Just merged! I'll try and figure out what's going on with the repository permissions

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

Successfully merging this pull request may close these issues.

None yet

2 participants