Publishing changes fails to respect the permissions of GitHub team members

[kylekirkby]

Describe the bug
We've recently moved our marketing team members to a GitHub team with write permissions. The base branch requires a review to be merged. If an approving review is made and the marketing team member goes to publish the post. They are hit with a GrapQL error complaining about not having the required permissions; even though they can merge on GitHub directly (this would cause the cms/branch exists error though when editing the same page again).

To Reproduce

• Add branch protection rules to the base branch that NetlifyCMS merges changes into.
• Add GitHub team-based collaborators
• Review/approve a change
• Try clicking publish in NetlifyCMS (as a member of the GitHub team with write permissions

Expected behavior
NetlifyCMS to respect the GitHub team member permissions and allow the end-user to publish their changes.

Screenshots

Applicable Versions:

• Netlify CMS version: [e.g. 2.0.4]
• Git provider: [e.g. GitHub, BitBucket]
• OS: [e.g. Windows 7]
• Browser version [e.g. chrome 22, safari 11]

• Node.JS version:

CMS configuration
https://github.com/Linaro/website/blob/develop/admin/config.yml

[erezrokah]

Hi @kylekirkby, can you share a screen shot of the branch protection rules?

[erezrokah]

Closing this issue. Please provide more information on the branch protection rules configured if still relevant

[pmpinto]

We're having this issue too.

Netlify CMS is pointing at master, which has some protection rules, namely:

• Require 1 approval from a reviewer

Currently we're forcing non-devs to go to GitHub just so they can approve the PR and publish the changes. Which sometimes also makes people merge the changes from GitHub instead of the CMS.

A solution for this could be:
With the editorial workflow, when marking the changes as "Ready", add an approval in the background so the changes can be published without ever going to GitHub.

@erezrokah Can we open this one?

[erezrokah]

Hi @pmpinto your feature request is a bit different than the issue.
I believe the original issue author expected that having write permissions will allow CMS users to publish to a protected branch.

It seems you're asking for a new feature to be able to request a PR review from the CMS.

However, I think you should also be able to use code owners to automatically request a review which should be an improvement.

If you'd like to automate this one step further, you should be able to use https://github.com/AndrewMusgrave/automatic-pull-request-review to automatically approve PRs (or any similar approach).

If those suggestions don't work for you, I recommend opening a new feature request - I believe it will request some UI changes (blocking publish until the PR is approved)

[pmpinto]

Hey @erezrokah! Thanks for taking a look at this.

It seems you're asking for a new feature to be able to request a PR review from the CMS.

Sorry if I didn't make that clear.
I believe what I'm looking for is exactly what the original poster described.

The issue being: users with write permissions on the target branch are unable to publish a change (on a protected branch) from the CMS directly. Because it will complain it needs at least 1 review (in my case).

A possible solution:
Just like we can drag the changes around in the workflow tab and the CMS will manage what tag should be applied to the PR in the background, I was hoping we could approve the associated PR, considering the user dragging the change.

I mean:

1. I create some changes through the CMS
2. The CMS creates a PR for me
3. I drag the changes to the "in review" column, in the workflow tab
4. The CMS tweaks the label on the PR
5. You (the reviewer, with write permissions) come along and make sure everything is ok, moving the change to the "ready" column
6. The CMS tweaks the label, and approves the PR in your name
7. I can now merge the change directly from the CMS without ever going to GitHub

Step 6 is what I'm suggesting as a solution for this issue.
Does that make sense to you?

[erezrokah]

Hi @pmpinto, thanks that makes more sense.

Just realized we have another issue for this #1593.
So it's better to discuss it there anyway