Block restricted status movements in editor

[alecgeatches]

Description

Now that we can restrict statuses to certain users via settings in #42, this PR adds support for those restrictions in the editor.

For the following demo, we're using 4 statuses:

Two of the statuses, "Status 2" and "Legal Review", have been set with restrictions to only allow an administrator to make changes:

---

With that setup, we can see how the custom save button behavior changes while making status movements.

1. First, a editor user creates a new post and promotes it until the first restricted status ("Status 2") is reached:

   ---

   

   ---

   Note that the tooltip on a disabled status says "Awaiting review from a privileged user". We may want to improve or change that.

2. After a status is blocked, we switch to the Admin user to approve the "Status 2" step:

   

3. We resume the role as an editor and progress the post to the final "Legal Review" status:

   

4. We can see that a restriction on the last status cause the "Publish" button to be blocked. Going back to the Admin user, we can sign off on the legal review step and publish:

   

Todo

• Ensure that there are backend controls to verify restrictions. In the demo above, it's currently possible to run VW_CUSTOM_STATUSES.current_user_id="1" as an editor user on page load, and then restrictions are removed.
• Add tests for backend verificaiton.
• Test what happens when every step requires an approval.

[ingeniumed]

One more area where I noticed the restrictions need to be enforced: The dropdown for the extended status can be used to change the status as well.

Instead of fixing that, I think we should switch that to just display the status. The next step would be to eliminate that entirely, and just update the regular post status to show something besides draft.

[ingeniumed]

LGTM, this is on the right track.

One thing that I have run into, in this PR as well as #43 is bad data being present since the users/editorial metadata were made before the deletion handling was added in. That's normal in the development phase for us, so just have to clean up that data.

But, it does help to point out areas where we should have error handling so that bad data can be ignored and then removed on the next update.

[ingeniumed]

Should be able to use wp.data.select("core").getCurrentUser() instead within the JS code itself so that we let WP/Gutenberg handle it?

[alecgeatches]

Good question! I should have left a comment about this. I originally went with getCurrentUser() through mapSelectProps(), but I ran into an issue. getCurrentUser() results in a REST request. Before the request resolves, the user returned from the call is an empty object. This creates some design issues. Before the user loads, should we hide the button? Disabled it? Ignore clicks? What should we say as the button's text? Now that we're using user permissions to determine the style and functionality of the save button, we need to wait until the user is done loading to fully style it.

This delay can put us into a "flash of unstyled content"-type bug where our custom button has to take some default and possibly different state before rendering correctly later when a user request completes.

The optimization here is that we already have the user's ID available when we load the page, and that's all we need to determine how the save button functions. I'm honestly a little curious whygetCurrentUser() isn't preloaded with the current user - the user that loads the page isn't going to change meaningfully until the entire page is reloaded anyhow.

[ingeniumed]

I feel like with the switch to the current_user, the only piece that would need some guarding would be the transition_post_status.

But, if are there too many bugs one thing we could do:

• Take away the edit_post capability so no save operation would work for the user that authored the post as long as the status is restricted.
• They can move the post back a status to edit it's content, and then send it back to the current status.

This would change the paradigm a touch but just a backup plan in case we need it.

[alecgeatches]

• Take away the edit_post capability so no save operation would work for the user that authored the post as long as the status is restricted.

This is what I was thinking, as this effectively stopped transitions in our prior work. The main problem was the UI and weird error message when that failed. However, if we have the UI covered and this is only a backup solution when the client-side code is manipulated, I think it'll work well.

• They can move the post back a status to edit it's content, and then send it back to the current status.

As of right now a user can still "Save draft" on a restricted status. This makes sense to me if we're in a "Pending Review"-type stage where only the editor can approve, but an author may be asked to make changes to the content in that stage. So the content is always editable, but the status should not be.

[alecgeatches]

One more area where I noticed the restrictions need to be enforced: The dropdown for the extended status can be used to change the status as well.

Instead of fixing that, I think we should switch that to just display the status. The next step would be to eliminate that entirely, and just update the regular post status to show something besides draft.

@ingeniumed I'm not sure how we handle a "oops, I clicked 'Move to' instead of 'Save Draft' by accident, now my post is stuck in the wrong status" situation. I agree that this shouldn't be a standard part of the UI, but we need some way for a status to get fixed. Maybe we make the "Edit" button administrator-only to start?

[alecgeatches]

@ingeniumed I added the ability to block post transitions in this commit: 9bfd070. The solution I found uses the wp_insert_post_data filter, mixed with this validation check a few lines later to block status transitions that don't follow our rules.

To test this, edit isRestrictedStatus to always return false in order to bypass client-side restrictions:

const isRestrictedStatus = useMemo( () => {
    return false;
}, [ savedStatus ] );

Next, set up a status that is restricted to another user. In the example below, I'm logged in as editor_0001 and the current "Assigned" status requires approval from vipgo. With client-side restrictions removed, try to transition to the next status ("In Progress" here):

This seems like a decent error message, and our users should only see it if they're trying to bypass client-side validation or their local page data is out of sync with recent custom status changes. If we want, we could also use a translation filter to customize this message, but I think it's clear enough to show for client-side edge cases for now.

---

FYI I also spent some time trying to utilize the user_has_cap filter for this purpose, but it was only getting called after the post transition had completed and it was too late to block changes.

[alecgeatches]

I noticed that we were explicitly setting a capability to true here, and it may be in error. If this status doesn't match an expected one, we should leave capabilities as default without modifying them.

[alecgeatches]

@ingeniumed This is ready for another review! Since your initial look, I've added:

• Backend verification for status changes via wp_insert_post_data filter.
• Tests to ensure that privileged users can change restricted statuses and unprivileged users can't transition post statuses if client-side restrictions are removed.
• Some test updates with a new base class we can use for self-cleaning utility methods like create_user().

[jarekmorawski]

Any chance we can start aligning the current UI with the new one? Even tweaking the copy would go a long way toward making the editing UX clearer. Here's a quick mock for the status details modal that uses the revised information architecture and UX copy.