-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Enable collaborators #20353
Changes from all commits
6c5616c
52ac1e7
63b5901
69f080e
69bdeaa
92193b3
7766d3b
6bf9dc8
5afeecf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -158,7 +158,9 @@ export class ContextService { | |
user.id, | ||
context.repository.cloneUrl, | ||
options?.organizationId, | ||
true, | ||
); | ||
// todo(ft): solve for this case with collaborators who can't select projects directly | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. todo: file an issue for this |
||
if (projects.length > 1) { | ||
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Multiple projects found for clone URL."); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ export const sortSuggestedRepositories = (repos: SuggestedRepositoryWithSorting[ | |
// This allows us to consider the lastUse of a recently used project when sorting | ||
// as it will may have an entry for the project (no lastUse), and another for recent workspaces (w/ lastUse) | ||
|
||
const projectURLs: string[] = []; | ||
let projectURLs: string[] = []; | ||
let uniqueRepositories: SuggestedRepositoryWithSorting[] = []; | ||
|
||
for (const repo of repos) { | ||
|
@@ -88,7 +88,7 @@ export const sortSuggestedRepositories = (repos: SuggestedRepositoryWithSorting[ | |
uniqueRepositories = uniqueRepositories.map((repo) => { | ||
if (repo.projectId && !repo.projectName) { | ||
delete repo.projectId; | ||
delete projectURLs[projectURLs.indexOf(repo.url)]; | ||
projectURLs = projectURLs.filter((url) => url !== repo.url); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This deletion would not work properly when you had two repos with the same project URL. Because we're iterating over the unique repos, it would clean up only the first occurrence of the project URL, instead of cleaning up all of them, leaving an incorrect state in |
||
} | ||
|
||
return repo; | ||
|
There was a problem hiding this comment.
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!