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

feat: add approve PR button to workflow UI #6055

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mmkal
Copy link
Contributor

@mmkal mmkal commented Dec 15, 2021

Fixes #1593

Summary

Adds an approve button to the workflow UI. Many limitations, this is about as far as I could get without some context/guidance:

  • GitHub-only, not GitLab, Azure etc. - I am only able to test with GitHub. Can the others come later/be implemented by someone using them?
  • Approve button always appears:
    • Even if the current user authored the entry - how do I compare entry author with current user?
    • Even if the backend used doesn't support approvals (yet) - is there a standard way to thread backend "capabilities" all the way to the UI?
  • Not much testing yet - can/should something like this be tested? Hard to think of a meaningful way to without creating real GitHub PRs
  • Docs - where should I write docs for this?
  • Non-English language messages - what's the process for translation?

Test plan

See above. I tried it and it worked.

Checklist

  • I have read the contribution guidelines.
  • Code is formatted via running yarn format.
  • Tests are passing via running yarn test.
  • The status checks are successful (continuous integration). Those can be seen below.

@mmkal mmkal requested a review from a team December 15, 2021 01:31
@erezrokah erezrokah added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Dec 15, 2021
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Hi @mmkal, this is very cool 🎉

I can follow up with the rest of the backends (GitLab, Azure and Bitbucket).

To your questions:

Even if the backend used doesn't support approvals (yet) - is there a standard way to thread backend "capabilities" all the way to the UI?

We can use https://github.com/netlify/netlify-cms/blob/0e9732ac5567bf2025d6ff3eacebe61660db73fa/packages/netlify-cms-backend-github/src/implementation.tsx#L130 to detect if the backend supports approvals

Even if the current user authored the entry - how do I compare entry author with current user?

We return the author here https://github.com/netlify/netlify-cms/blob/0e9732ac5567bf2025d6ff3eacebe61660db73fa/packages/netlify-cms-backend-github/src/API.ts#L606
So we can compare it to the logged in user.

Docs - where should I write docs for this?

I don't think we need docs for this, unless we see that users find the UI confusing (then maybe we should fix the UI)

Non-English language messages - what's the process for translation?

Those are contributed by the community, usually in follow up PRs

@mmkal
Copy link
Contributor Author

mmkal commented Jan 4, 2022

@erezrokah thanks for the feedback! Your suggestions look doable, will try to find some time for them but likely will not be possible for a few weeks.

@mmkal
Copy link
Contributor Author

mmkal commented Jan 27, 2022

@erezrokah still haven't had a chance to look at this, but I had a thought so thought I'd add here. Since opening this PR a few more requests have come up for more GitHub PR management options, e.g. comments, enable automerge, request reviewers, etc. etc. I don't think it'd be reasonable to expect netlify-cms to build and maintain all of those features, but one of the awesome things about netlify-cms is how customiseable it is. So, an idea: some kind of "workflow widget" concept - similar to field widgets, they could receive the backend instance via props and can render anything they want which could appear in the Workflow box (where I've added the approve button right now). API could be something like this:

import {useQuery, useMutation} from 'react-query' // or use any other react library

CMS.registerWorkflowWidget({
  name: 'github-comment-list-and-approve-button',
  control: ({ entry, backend }) => {
    const commentsQuery = useQuery('comments', () => backend.request(`/foo/bar/${entry.pullRequest.id}/comments`))
    const approveMutation = useMutation('approve', () => backend.request(`...`))
    return <>
      {commentsQuery.data?.map(c => <div>{c.body}</div>)}
      <button disabled={!approveMutation.isIdle} onClick={() => approveMutation.mutate()}>Approve</button>
    </>
  }
})

That way, functionality like what's in this PR could just be an in-built WorkflowWidget, similar to the Cloudinary and Uploadcare media libraries.

Either way, no reason for this not to go in first, then to be refactored/widget-ised.

@erezrokah
Copy link
Contributor

Either way, no reason for this not to go in first, then to be refactored/widget-ised.

Agree the proposal should not block this PR.

For the proposal itself, I suggest opening a new issue for it to discuss. The CMS has been very much about abstracting the underlying git services (e.g. GitHub) from the user/editor, so we'll need to see how the proposal fits into that.

@erezrokah
Copy link
Contributor

@mmkal, I tried to bring this PR up to speed with other backends, however I realized each git service acts differently.

For example - GitHub doesn't let you approve your own PRs, while GitLab does.
Also once you approve in GitLab, you'll need to revoke in order to approve again (not sure on GitHub).

We'll have to consider all these different states between provider to avoid confusion in the UI.

Please let me know what you think.

@stale
Copy link

stale bot commented Apr 26, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@mmkal
Copy link
Contributor Author

mmkal commented May 22, 2023

@martinjagodic since this is pinned, just an FYI - we ended up building a totally custom editorial-workflow alternative, and I definitely won't be able to do dive deep enough to implement this for other backends. I still think it'd be a nice feature, and in general would be a good thing for different backends to have different capabilities, so I'd argue this could go in without implementing GitLab, BitBucket etc. But either way, someone else would have to pick it up. I think it's pretty much ready to go in, maybe with a few more docs explaining it only works with GitHub for now. I'll update the branch, and am happy to help handover to someone on the team (if you make me a contributor on this repo with permissions to create feature branches, I could transfer it to this repo so it's easier for the decap team to take over).

@netlify
Copy link

netlify bot commented May 22, 2023

Deploy Preview for decap-www canceled.

Name Link
🔨 Latest commit 1a163b2
🔍 Latest deploy log https://app.netlify.com/sites/decap-www/deploys/646b5268c220e200083890c6

@martinjagodic
Copy link
Member

Thanks for the info @mmkal. I pinned this as it seems a good feature and PR, but it's not a priority right now. Let's get back here when the time is right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to approve pr in the netlify-cms gui
3 participants