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

Story Locking: Ensure author users and below correctly see information #12559

Merged
merged 6 commits into from
Nov 3, 2022

Conversation

spacedmonkey
Copy link
Contributor

@spacedmonkey spacedmonkey commented Oct 28, 2022

Context

Summary

Instead of using the embedded user rest api request, just hardcode the user data in the api response.

Relevant Technical Choices

To-do

User-facing changes

Testing Instructions

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

  1. Create a story as a user with Author role.
  2. Save draft.
  3. Open another browser.
  4. Login as another user.
  5. Open draft story.
  6. See lock message.
  7. Take over editing
  8. As the Author user, see the other user's name & avatar in the takeover dialog

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #12441
Fixes #12604

@spacedmonkey spacedmonkey added Type: Bug Something isn't working Pod: WP Group: Story Locking aka post locking labels Oct 28, 2022
@spacedmonkey spacedmonkey requested a review from timarney October 28, 2022 13:04
@spacedmonkey spacedmonkey self-assigned this Oct 28, 2022
if (showLockedDialog && currentUserLoaded) {
getStoryLockById(storyId, stories)
.then(({ locked, nonce: newNonce, _embedded }) => {
(async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Converted this to async code is not needed for this, but made it easier to read.

@googleforcreators-bot
Copy link
Collaborator

googleforcreators-bot commented Oct 28, 2022

Plugin builds for bb234cb are ready 🛎️!

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2022

Size Change: -50 B (0%)

Total Size: 2.72 MB

ℹ️ View Unchanged
Filename Size Change
assets/css/carousel-view-rtl.css 702 B 0 B
assets/css/carousel-view.css 701 B 0 B
assets/css/web-stories-block-rtl.css 4.51 kB 0 B
assets/css/web-stories-block.css 4.56 kB 0 B
assets/css/web-stories-embed-rtl.css 318 B 0 B
assets/css/web-stories-embed.css 317 B 0 B
assets/css/web-stories-list-styles-rtl.css 2.35 kB 0 B
assets/css/web-stories-list-styles.css 2.38 kB 0 B
assets/css/web-stories-theme-style-twentyeleven-rtl.css 102 B 0 B
assets/css/web-stories-theme-style-twentyeleven.css 102 B 0 B
assets/css/web-stories-theme-style-twentyfifteen-rtl.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfifteen.css 251 B 0 B
assets/css/web-stories-theme-style-twentyfourteen-rtl.css 287 B 0 B
assets/css/web-stories-theme-style-twentyfourteen.css 287 B 0 B
assets/css/web-stories-theme-style-twentyseventeen-rtl.css 288 B 0 B
assets/css/web-stories-theme-style-twentyseventeen.css 288 B 0 B
assets/css/web-stories-theme-style-twentysixteen-rtl.css 224 B 0 B
assets/css/web-stories-theme-style-twentysixteen.css 224 B 0 B
assets/css/web-stories-theme-style-twentyten-rtl.css 143 B 0 B
assets/css/web-stories-theme-style-twentyten.css 143 B 0 B
assets/css/web-stories-theme-style-twentytwelve-rtl.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwelve.css 256 B 0 B
assets/css/web-stories-theme-style-twentytwenty-rtl.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwenty.css 86 B 0 B
assets/css/web-stories-theme-style-twentytwentyone-rtl.css 326 B 0 B
assets/css/web-stories-theme-style-twentytwentyone.css 326 B 0 B
assets/css/web-stories-widget-rtl.css 482 B 0 B
assets/css/web-stories-widget.css 482 B 0 B
assets/css/wp-dashboard-rtl.css 657 B 0 B
assets/css/wp-dashboard.css 659 B 0 B
assets/css/wp-story-editor-rtl.css 769 B 0 B
assets/css/wp-story-editor.css 771 B 0 B
assets/js/2468.js 7.74 kB 0 B
assets/js/4422.js 49.3 kB 0 B
assets/js/6071.js 92.9 kB 0 B
assets/js/9162.js 36.5 kB 0 B
assets/js/9206.js 193 kB 0 B
assets/js/9750.js 12.8 kB 0 B
assets/js/carousel-view.js 3.41 kB 0 B
assets/js/chunk-colorthief.js 2.64 kB 0 B
assets/js/chunk-ffmpeg.js 5.88 kB 0 B
assets/js/chunk-html-to-image.js 4.5 kB 0 B
assets/js/chunk-opentype.js 96 B 0 B
assets/js/chunk-react-calendar.js 12.5 kB 0 B
assets/js/chunk-react-color.js 44.3 kB 0 B
assets/js/chunk-selfie-segmentation.js 12.5 kB 0 B
assets/js/chunk-web-animations-js.js 14.6 kB 0 B
assets/js/chunk-web-stories-template-0-metaData.js 546 B 0 B
assets/js/chunk-web-stories-template-0.js 11.4 kB 0 B
assets/js/chunk-web-stories-template-1-metaData.js 540 B 0 B
assets/js/chunk-web-stories-template-1.js 9.6 kB 0 B
assets/js/chunk-web-stories-template-10-metaData.js 532 B 0 B
assets/js/chunk-web-stories-template-10.js 7.36 kB 0 B
assets/js/chunk-web-stories-template-11-metaData.js 539 B 0 B
assets/js/chunk-web-stories-template-11.js 9.07 kB 0 B
assets/js/chunk-web-stories-template-12-metaData.js 496 B 0 B
assets/js/chunk-web-stories-template-12.js 9.68 kB 0 B
assets/js/chunk-web-stories-template-13-metaData.js 525 B 0 B
assets/js/chunk-web-stories-template-13.js 7.39 kB 0 B
assets/js/chunk-web-stories-template-14-metaData.js 582 B 0 B
assets/js/chunk-web-stories-template-14.js 7.37 kB 0 B
assets/js/chunk-web-stories-template-15-metaData.js 544 B 0 B
assets/js/chunk-web-stories-template-15.js 9 kB 0 B
assets/js/chunk-web-stories-template-16-metaData.js 588 B 0 B
assets/js/chunk-web-stories-template-16.js 10.8 kB 0 B
assets/js/chunk-web-stories-template-17-metaData.js 540 B 0 B
assets/js/chunk-web-stories-template-17.js 9.2 kB 0 B
assets/js/chunk-web-stories-template-18-metaData.js 587 B 0 B
assets/js/chunk-web-stories-template-18.js 9.9 kB 0 B
assets/js/chunk-web-stories-template-19-metaData.js 501 B 0 B
assets/js/chunk-web-stories-template-19.js 10.8 kB 0 B
assets/js/chunk-web-stories-template-2-metaData.js 586 B 0 B
assets/js/chunk-web-stories-template-2.js 9.29 kB 0 B
assets/js/chunk-web-stories-template-20-metaData.js 548 B 0 B
assets/js/chunk-web-stories-template-20.js 8.99 kB 0 B
assets/js/chunk-web-stories-template-21-metaData.js 536 B 0 B
assets/js/chunk-web-stories-template-21.js 9.85 kB 0 B
assets/js/chunk-web-stories-template-22-metaData.js 525 B 0 B
assets/js/chunk-web-stories-template-22.js 7.83 kB 0 B
assets/js/chunk-web-stories-template-23-metaData.js 604 B 0 B
assets/js/chunk-web-stories-template-23.js 7.47 kB 0 B
assets/js/chunk-web-stories-template-24-metaData.js 517 B 0 B
assets/js/chunk-web-stories-template-24.js 11.7 kB 0 B
assets/js/chunk-web-stories-template-25-metaData.js 543 B 0 B
assets/js/chunk-web-stories-template-25.js 7.05 kB 0 B
assets/js/chunk-web-stories-template-26-metaData.js 600 B 0 B
assets/js/chunk-web-stories-template-26.js 7.26 kB 0 B
assets/js/chunk-web-stories-template-27-metaData.js 542 B 0 B
assets/js/chunk-web-stories-template-27.js 7.8 kB 0 B
assets/js/chunk-web-stories-template-28-metaData.js 532 B 0 B
assets/js/chunk-web-stories-template-28.js 9.05 kB 0 B
assets/js/chunk-web-stories-template-29-metaData.js 561 B 0 B
assets/js/chunk-web-stories-template-29.js 9.24 kB 0 B
assets/js/chunk-web-stories-template-3-metaData.js 539 B 0 B
assets/js/chunk-web-stories-template-3.js 8.39 kB 0 B
assets/js/chunk-web-stories-template-30-metaData.js 576 B 0 B
assets/js/chunk-web-stories-template-30.js 7.88 kB 0 B
assets/js/chunk-web-stories-template-31-metaData.js 503 B 0 B
assets/js/chunk-web-stories-template-31.js 10.3 kB 0 B
assets/js/chunk-web-stories-template-32-metaData.js 552 B 0 B
assets/js/chunk-web-stories-template-32.js 13.3 kB 0 B
assets/js/chunk-web-stories-template-33-metaData.js 491 B 0 B
assets/js/chunk-web-stories-template-33.js 9.06 kB 0 B
assets/js/chunk-web-stories-template-34-metaData.js 570 B 0 B
assets/js/chunk-web-stories-template-34.js 7.57 kB 0 B
assets/js/chunk-web-stories-template-35-metaData.js 565 B 0 B
assets/js/chunk-web-stories-template-35.js 8.9 kB 0 B
assets/js/chunk-web-stories-template-36-metaData.js 575 B 0 B
assets/js/chunk-web-stories-template-36.js 12.7 kB 0 B
assets/js/chunk-web-stories-template-37-metaData.js 529 B 0 B
assets/js/chunk-web-stories-template-37.js 6.71 kB 0 B
assets/js/chunk-web-stories-template-38-metaData.js 572 B 0 B
assets/js/chunk-web-stories-template-38.js 7.92 kB 0 B
assets/js/chunk-web-stories-template-39-metaData.js 589 B 0 B
assets/js/chunk-web-stories-template-39.js 8.07 kB 0 B
assets/js/chunk-web-stories-template-4-metaData.js 564 B 0 B
assets/js/chunk-web-stories-template-4.js 12.7 kB 0 B
assets/js/chunk-web-stories-template-40-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-40.js 10.2 kB 0 B
assets/js/chunk-web-stories-template-41-metaData.js 573 B 0 B
assets/js/chunk-web-stories-template-41.js 7.74 kB 0 B
assets/js/chunk-web-stories-template-42-metaData.js 521 B 0 B
assets/js/chunk-web-stories-template-42.js 6.99 kB 0 B
assets/js/chunk-web-stories-template-43-metaData.js 557 B 0 B
assets/js/chunk-web-stories-template-43.js 8.75 kB 0 B
assets/js/chunk-web-stories-template-44-metaData.js 583 B 0 B
assets/js/chunk-web-stories-template-44.js 11.1 kB 0 B
assets/js/chunk-web-stories-template-45-metaData.js 565 B 0 B
assets/js/chunk-web-stories-template-45.js 7.51 kB 0 B
assets/js/chunk-web-stories-template-46-metaData.js 531 B 0 B
assets/js/chunk-web-stories-template-46.js 5.21 kB 0 B
assets/js/chunk-web-stories-template-47-metaData.js 591 B 0 B
assets/js/chunk-web-stories-template-47.js 9.41 kB 0 B
assets/js/chunk-web-stories-template-48-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-48.js 9.08 kB 0 B
assets/js/chunk-web-stories-template-49-metaData.js 518 B 0 B
assets/js/chunk-web-stories-template-49.js 9.69 kB 0 B
assets/js/chunk-web-stories-template-5-metaData.js 556 B 0 B
assets/js/chunk-web-stories-template-5.js 9.91 kB 0 B
assets/js/chunk-web-stories-template-50-metaData.js 503 B 0 B
assets/js/chunk-web-stories-template-50.js 9.13 kB 0 B
assets/js/chunk-web-stories-template-51-metaData.js 526 B 0 B
assets/js/chunk-web-stories-template-51.js 10.3 kB 0 B
assets/js/chunk-web-stories-template-52-metaData.js 601 B 0 B
assets/js/chunk-web-stories-template-52.js 10.3 kB 0 B
assets/js/chunk-web-stories-template-53-metaData.js 551 B 0 B
assets/js/chunk-web-stories-template-53.js 5.78 kB 0 B
assets/js/chunk-web-stories-template-54-metaData.js 547 B 0 B
assets/js/chunk-web-stories-template-54.js 7.66 kB 0 B
assets/js/chunk-web-stories-template-55-metaData.js 574 B 0 B
assets/js/chunk-web-stories-template-55.js 7.12 kB 0 B
assets/js/chunk-web-stories-template-56-metaData.js 542 B 0 B
assets/js/chunk-web-stories-template-56.js 9.85 kB 0 B
assets/js/chunk-web-stories-template-57-metaData.js 528 B 0 B
assets/js/chunk-web-stories-template-57.js 14.9 kB 0 B
assets/js/chunk-web-stories-template-58-metaData.js 554 B 0 B
assets/js/chunk-web-stories-template-58.js 5.74 kB 0 B
assets/js/chunk-web-stories-template-59-metaData.js 590 B 0 B
assets/js/chunk-web-stories-template-59.js 8.94 kB 0 B
assets/js/chunk-web-stories-template-6-metaData.js 568 B 0 B
assets/js/chunk-web-stories-template-6.js 7.06 kB 0 B
assets/js/chunk-web-stories-template-60-metaData.js 509 B 0 B
assets/js/chunk-web-stories-template-60.js 9.52 kB 0 B
assets/js/chunk-web-stories-template-7-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-7.js 7.45 kB 0 B
assets/js/chunk-web-stories-template-8-metaData.js 569 B 0 B
assets/js/chunk-web-stories-template-8.js 8.9 kB 0 B
assets/js/chunk-web-stories-template-9-metaData.js 580 B 0 B
assets/js/chunk-web-stories-template-9.js 8.45 kB 0 B
assets/js/chunk-web-stories-templates.js 1.17 kB 0 B
assets/js/chunk-web-stories-textset-0.js 5.04 kB 0 B
assets/js/chunk-web-stories-textset-1.js 6.63 kB 0 B
assets/js/chunk-web-stories-textset-2.js 7.62 kB 0 B
assets/js/chunk-web-stories-textset-3.js 15 kB 0 B
assets/js/chunk-web-stories-textset-4.js 4.14 kB 0 B
assets/js/chunk-web-stories-textset-5.js 5.45 kB 0 B
assets/js/chunk-web-stories-textset-6.js 5.25 kB 0 B
assets/js/chunk-web-stories-textset-7.js 10.1 kB 0 B
assets/js/generateBlurhash.worker.worker.js 1.1 kB 0 B
assets/js/imgareaselect.js 3.77 kB 0 B
assets/js/lightbox.js 550 B 0 B
assets/js/tinymce-button.js 2.85 kB 0 B
assets/js/web-stories-activation-notice.js 27.1 kB 0 B
assets/js/web-stories-block.js 22.6 kB 0 B
assets/js/web-stories-embed.js 20 B 0 B
assets/js/web-stories-widget.js 587 B 0 B
assets/js/wp-dashboard.js 64.2 kB -25 B (0%)
assets/js/wp-story-editor.js 1.44 MB -25 B (0%)

compressed-size-action

Copy link
Collaborator

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

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

At first glance looks good 👍 (besides the failing tests of course)

@swissspidy
Copy link
Collaborator

Just ocurred to me that the dashboard needs changing here as well. Can you check that?

see #12604

@kkalarickal
Copy link

@spacedmonkey @swissspidy -- I tested the 12559 branch to confirm that author-users were seeing the take over dialog.
image.png

This take over dialog is different from the one that the Admin-user sees, when Author-user takes over a story that Admin-user was editing:
image.png

I am not clear which of these two dialogs should appear in Author-users editor when Admin-user takes over the story that Author-user was editing.

@swissspidy
Copy link
Collaborator

Both users can see both dialogs.

You see one when trying to open a locked story, the other when someone takes over editing from you.

In both cases, users should at all time see the name & avatar of the other user.

@spacedmonkey
Copy link
Contributor Author

Added a fix for #12604

@swissspidy
Copy link
Collaborator

@kkalarickal Can you re-test #12604 here?

@kkalarickal
Copy link

Confirmed that non-admin users can:

  • see lock icon and thumbnail of user who is currently editing a story within the dashboard
  • are given appropriate takeover dialog when then edit a locked story
  • if the take over occurs, then the original user is given the other type of dialog to indicate that non-admin user has taken over editingimage.png
  • thumbnail is now changed on the story being edited, when viewed by admin & non-admin users in their respective dashboards
    BEFORE:image.png
    AFTER:image.png

@kkalarickal
Copy link

kkalarickal commented Nov 3, 2022

@swissspidy I noticed that the following and wanted to get your take on if this is ok:

  • If a user profile has lower privileges than the owner of a story, then she is only able to view the published story. That user cannot edit the story. In such a case, even if that story is opened by someone else, it will NOT show the thumbnail if it is viewed by a user with lower privileges.

Example: This is the view for Admin userimage.png

The same dashboard, when viewed by Author userimage.png

  • if a user opens a story, then logs into another browser instance with the same credentials and views the story dashboard, the original opened story does NOT show the lock icon or thumbnail. It looks like for a user, the info about lock/thumbnail is not shared if viewed from another login of the same user.

@swissspidy
Copy link
Collaborator

@kkalarickal Not sure if I understood correctly, but if a user is not allowed to edit a story, then there's no point in showing them that it's currently locked, so that is expected and sounds correct to me.

@swissspidy swissspidy changed the title Author users and below not seeing story takeover dialog Story Locking: Ensure author users and below correctly see information Nov 3, 2022
@swissspidy swissspidy merged commit b4003ff into main Nov 3, 2022
@swissspidy swissspidy deleted the fix/12441 branch November 3, 2022 11:06
@kkalarickal
Copy link

@swissspidy - thank you for confirming. I agree that if user-profile cannot edit a story authored by someone with higher level of access, then there is no need to show the locked/thumbnail.

For the scenario where the same user is accessing the Story dashboard from different browser instances, I was thinking we should indicate that another instance is open.

However, when I looked at the WP Post workflow - it also does not show the lock-icon or thumbnail for same user login from another browser. So I guess we are at par with WP Post workflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: Story Locking aka post locking Type: Bug Something isn't working
Projects
None yet
5 participants