#4786 Add a collaborator level to projects.
Merged by pingou. Opened by pingou.
branch_acl  into  master

Download 4786.patch

The collaborators are collaborators that are only granted limited access
to the project (for example, one or a few branches in the repository).
They are also provided ticket access.
They are not granted full commit on the entire project.

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

This is really not ready for review/merging but people can already start to have a look at it and I'd like to see what breaks in the CI :)

rebased onto c36322d36104821782e39538dfef382915dd66c5

Shouldn't this be collaborator? Especially throughout the rest of the file?

Good catch, thanks! :)

rebased onto c70565aa9d23d0f04a7346c780e6b09932e6becb

How does this affect PR workflow? Can someone merge a PR that targets their branch but not see the merge button if it doesn't?

How does this affect PR workflow? Can someone merge a PR that targets their branch but not see the merge button if it doesn't?

To be figured out

How does this affect PR workflow? Can someone merge a PR that targets their branch but not see the merge button if it doesn't?

If desired we could use the new is_collaborator util method to leverage that on the merge button logic

Is it possible to add collaborators without branches defined?

We could want that in conjuction with https://pagure.io/pagure/issue/4470 , that way we could allow to specify a group of users without commit access but with voting capabilities

rebased onto a02f94af8895115a03c06c0d4ff7e2c893e943ab

pretty please pagure-ci rebuild

Blech...

05:38:02  Failed tests:
05:38:02  FAILED test: py3-test_pagure_exclude_group_index
05:38:02  FAILED test: py3-test_pagure_flask_api_fork
05:38:02  FAILED test: py3-test_pagure_flask_api_fork_update
05:38:02  FAILED test: py3-test_pagure_flask_api_group
05:38:02  FAILED test: py3-test_pagure_flask_api_project
05:38:02  FAILED test: py3-test_pagure_flask_api_ui_private_repo
05:38:02  FAILED test: py3-test_pagure_flask_api_user
05:38:02  FAILED test: py3-test_pagure_flask_ui_app_browse
05:38:02  FAILED test: py3-test_pagure_flask_ui_repo
05:38:02  FAILED test: py3-test_pagure_lib
05:38:02  FAILED test: py3-test_pagure_lib_git
05:38:02  FAILED test: py3-test_pagure_lib_git_auth
05:38:02  FAILED test: py3-test_pagure_lib_git_auth_paguregitauth
05:38:02  FAILED test: py3-test_style

rebased onto d11d8a654ef9f4b32e0abeb11dceed461ee56553

rebased onto 70e47fac575828a4ba9aeb32e0a25701e5864d48

rebased onto e43d4cd6dbbe2f64a95db1f2e8ece5c46dea1abd

rebased onto 6c6c52a28d47008b1a6db89c44fc47eb86d0f844

:sob:

15:44:01  Failed tests:
15:44:01  FAILED test: py3-test_pagure_flask_api_fork_assign
15:44:01  FAILED test: py3-test_pagure_flask_api_fork_update
15:44:01  FAILED test: py3-test_pagure_flask_internal
15:44:01  FAILED test: py3-test_pagure_flask_ui_app
15:44:01  FAILED test: py3-test_pagure_flask_ui_archives
15:44:01  FAILED test: py3-test_pagure_flask_ui_fork
15:44:01  FAILED test: py3-test_pagure_flask_ui_issue_pr_link
15:44:01  FAILED test: py3-test_pagure_flask_ui_issues
15:44:01  FAILED test: py3-test_pagure_flask_ui_issues_acl_checks
15:44:01  FAILED test: py3-test_pagure_flask_ui_issues_open_access
15:44:01  FAILED test: py3-test_pagure_flask_ui_issues_private
15:44:01  FAILED test: py3-test_pagure_flask_ui_login
15:44:01  FAILED test: py3-test_pagure_flask_ui_no_master_branch
15:44:01  FAILED test: py3-test_pagure_flask_ui_old_commit
15:44:01  FAILED test: py3-test_pagure_flask_ui_plugins
15:44:01  FAILED test: py3-test_pagure_flask_ui_pr_edit
15:44:01  FAILED test: py3-test_pagure_flask_ui_pr_no_sources
15:44:01  FAILED test: py3-test_pagure_flask_ui_remote_pr
15:44:01  FAILED test: py3-test_pagure_flask_ui_repo
15:44:01  FAILED test: py3-test_pagure_flask_ui_repo_flag_commit
15:44:01  FAILED test: py3-test_pagure_flask_ui_repo_slash_name
15:44:01  FAILED test: py3-test_pagure_flask_ui_repo_view_blame
15:44:01  FAILED test: py3-test_pagure_flask_ui_repo_view_file
15:44:01  FAILED test: py3-test_pagure_flask_ui_repo_view_history
15:44:01  FAILED test: py3-test_pagure_flask_ui_roadmap
15:44:01  FAILED test: py3-test_pagure_flask_ui_slash_branch_name
15:44:01  FAILED test: py3-test_pagure_flask_ui_star_project
15:44:01  FAILED test: py3-test_pagure_merge_pr_no_fork
15:44:01  FAILED test: py3-test_style

rebased onto 8168720bea6fa8786d2e2c9d806c9e44faf100b1

rebased onto 5ade6fa8ec05b7cd6f54a377db18207d20e7ece1

rebased onto 94c1ea83b6c483b7b0ead14d98e54d87c5f2f08d

rebased onto 418f5d292dcdbf52ca6f8b6e319fc6196bd5a55f

rebased onto 1e13ecfd0a3cd6e83eabbdc9867058e49f58b672

rebased onto d259837f58aa1e2ee5cb389f7447395ce215d431

rebased onto c09e7973d92513f2beb9cefc053edc0bfc26dbaf

Looks like only the blacken style test failed...

:thumbsup:

At this point, it looks like this just needs to be rebased and merged, right?

rebased onto d76fd4010da3c4ebf4e399da6595b3a17d62416e

rebased onto a3e419a432c91d5e399d3e8c307a781ba806d723

Note that pagure/ui/fork.py line 1185 and 1187 only check for is_committer. This would imply that if you're also asking Pagure to delete the branch in the source repo but you're not a full committer, but would have permissions to delete this specific branch, that would still be denied.
This is fine with me (fail closed), but just figured I'd note it.

I would strongly suggest to add tests to test_pagure_lib_git_auth.py for these cases, both positive and negative (with different branches).

The code itself looks reasonable to me, just those two general notes.
I think the ui/fork.py one can definitely be in a follow-up, and while I'd strongly recommend getting tests for the AuthHelper, the current implementation looks reasonable.

Note that pagure/ui/fork.py line 1185 and 1187 only check for is_committer. This would imply that if you're also asking Pagure to delete the branch in the source repo but you're not a full committer, but would have permissions to delete this specific branch, that would still be denied.
This is fine with me (fail closed), but just figured I'd note it.

Yeah, the whole logic around PR needs to be think through and adjusted. It will likely be in a subsequent PR (in the spirit of doing small incremental changes).

I would strongly suggest to add tests to test_pagure_lib_git_auth.py for these cases, both positive and negative (with different branches).

Will do :)

rebased onto 4f28c744277cece8a3c72f3bcbc3220fe69298d8

rebased onto 40c5b7a9c6b07df3c08abb2fe57846da3e23cfff

rebased onto 5c435e3959e7923c9297e34cb9f4315883fe8b23

1 new commit added

  • Add a collaborator level to projects.

rebased onto 585ecfd6c844b538d22e4f0272ca1fcb5b9e6234

rebased onto 67f81f18830188e010cf3edc845f09be7d959d9d

:thumbsup:

Thanks for the reviews folks! Let's get this in :)

Pull-Request has been merged by pingou

Metadata