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

Enable collaborators #20353

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

filiptronicek
Copy link
Member

@filiptronicek filiptronicek commented Nov 8, 2024

Description

Makes the following changes to the Collaborator role:

  1. Allows Collaborators to read environment variables of a project
  2. Allows Collaborators to read prebuilds of a project
  3. Allows Collaborators to start a project (this is necessary for the two new capabilities above), but just by starting a workspace on the same repo that an existing project exists on. Any details, including project name, will not be shown.

To be merged sometime around November 25th to give users a heads-up.

Related Issue(s)

Fixes CLC-815

How to test

  1. Join my org, you'll become a collaborator to it
  2. Try starting any repo, it should not let you.
  3. Try starting https://github.com/gitpod-samples/spring-petclinic, it should have environment variables (check for HELLO_VARIABLES) and it should be prebuilt.

Documentation

This does require documentation changes. I will work on those as we get closer to the merge date

/hold

@@ -92,7 +92,7 @@ schema: |-
permission read_billing = member + owner + installation->admin
permission write_billing = owner + installation->admin

permission read_prebuild = member + owner + installation->admin
permission read_prebuild = collaborator + member + owner + installation->admin
Copy link
Member Author

Choose a reason for hiding this comment

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

This org-wide read prebuild permission I flipped on accidentally when looking for the project-specific one, and it ended up raising a question: do we want to let collaborators view the list of prebuilds of the org? I'm guessing not, because we'd have to change those the related pages quite a bit (to signal that you can't start / stop prebuilds or view any of their settings), but it very theoretically could be useful if they wanted to at check what's wrong with a repo's prebuild config and if it's something they can fix themselves.
The detail of a single prebuild still will need some small tweaks to be properly accessible (currently, it just errors), but it probably wouldn't take too long to change.

@@ -128,15 +128,12 @@ export function deduplicateAndFilterRepositories(
if (results.length === 0) {
// If the searchString is a URL, and it's not present in the proposed results, "artificially" add it here.
if (isValidGitUrl(searchString)) {
console.log("It's valid man");
Copy link
Member Author

Choose a reason for hiding this comment

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

These super rad debug logs I accidentally left in with #20287. Yikes!

);
// todo(ft): solve for this case with collaborators who can't select projects directly
Copy link
Member Author

Choose a reason for hiding this comment

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

I'd love ideas around this: how can we not block collaborators if a repo they're working on gets imported twice? Or, in the very least, how do we educate members and owners about this possibility?

It's admittedly a case on the edge™, but IMO worth considering.

Copy link
Contributor

This pull request 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.

@github-actions github-actions bot added the meta: stale This issue/PR is stale and will be closed soon label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants