Skip to content

Mention username who added project in a team #5128

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

Merged
merged 1 commit into from
Dec 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions components/dashboard/src/projects/NewProject.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -315,15 +315,17 @@ export default function NewProject() {
<div key={`repo-${index}-${r.account}-${r.name}`} className="flex p-3 rounded-xl hover:bg-gray-100 dark:hover:bg-gray-800 focus:bg-gitpod-kumquat-light transition ease-in-out group" >

<div className="flex-grow">
<div className="text-base text-gray-900 dark:text-gray-50 font-medium rounded-xl whitespace-nowrap">{toSimpleName(r.name)}</div>
<div className={"text-base text-gray-900 dark:text-gray-50 font-medium rounded-xl whitespace-nowrap" + (r.inUse ? " text-gray-400 dark:text-gray-500" : "text-gray-700")}>{toSimpleName(r.name)}</div>
<p>Updated {moment(r.updatedAt).fromNow()}</p>
</div>
<div className="flex justify-end">
<div className="h-full my-auto flex self-center opacity-0 group-hover:opacity-100">
<div className="h-full my-auto flex self-center opacity-0 group-hover:opacity-100 items-center mr-2 text-right">
{!r.inUse ? (
<button className="primary" onClick={() => setSelectedRepo(r)}>Select</button>
) : (
<p className="my-auto">already taken</p>
<p className="text-gray-500 font-medium">
@{r.inUse.userName} already<br/>added this repo
Copy link
Contributor Author

Choose a reason for hiding this comment

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

suggestion: Re-posting from #5128 (comment).

Could we include a link to the user mentioned here that links back to the provider and their profile page? What do you think?

Since users per instance are publicly accessibly this could save users from the mundane task of copying their username to reach out, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mentioning here for reference - No need to implement before merging this PR:
While I was testing today, I discovered a stronger reason for doing as @gtsiolis suggested. Users with multiple integrations will be shown using the user-id of the first signup, which is not necessarily the user-id for the integration of the repo in question. E.g I used a different username for GitHub vs. GitLab and the "already added" message is showing my GitHub userid even though the repo is on GitLab.

</p>
)}
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion components/gitpod-db/src/project-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const ProjectDB = Symbol('ProjectDB');
export interface ProjectDB {
findProjectById(projectId: string): Promise<Project | undefined>;
findProjectByCloneUrl(cloneUrl: string): Promise<Project | undefined>;
findProjectsByCloneUrls(cloneUrls: string[]): Promise<Project[]>;
findProjectsByCloneUrls(cloneUrls: string[]): Promise<(Project & { teamOwners?: string[] })[]>;
findTeamProjects(teamId: string): Promise<Project[]>;
findUserProjects(userId: string): Promise<Project[]>;
storeProject(project: Project): Promise<Project>;
Expand Down
20 changes: 18 additions & 2 deletions components/gitpod-db/src/typeorm/project-db-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,31 @@ export class ProjectDBImpl implements ProjectDB {
return repo.findOne({ cloneUrl, markedDeleted: false });
}

public async findProjectsByCloneUrls(cloneUrls: string[]): Promise<Project[]> {
public async findProjectsByCloneUrls(cloneUrls: string[]): Promise<(Project & { teamOwners?: string[] })[]> {
if (cloneUrls.length === 0) {
return [];
}
const repo = await this.getRepo();
const q = repo.createQueryBuilder("project")
.where("project.markedDeleted = false")
.andWhere(`project.cloneUrl in (${ cloneUrls.map(u => `'${u}'`).join(", ") })`)
const result = await q.getMany();
const projects = await q.getMany();

const teamIds = Array.from(new Set(projects.map(p => p.teamId).filter(id => !!id)));

const teamIdsAndOwners = teamIds.length === 0 ? [] : (await (await this.getEntityManager()).query(`
SELECT member.teamId AS teamId, user.name AS owner FROM d_b_user AS user
LEFT JOIN d_b_team_membership AS member ON (user.id = member.userId)
WHERE member.teamId IN (${teamIds.map(id => `'${id}'`).join(", ")})
AND member.deleted = 0
AND member.role = 'owner'
`)) as { teamId: string, owner: string }[];

const result: (Project & { teamOwners?: string[] })[] = [];
for (const project of projects) {
result.push({...project, teamOwners: teamIdsAndOwners.filter(i => i.teamId === project.teamId).map(i => i.owner)});
}

return result;
}

Expand Down
2 changes: 1 addition & 1 deletion components/gitpod-protocol/src/gitpod-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ export interface ProviderRepository {
installationId?: number;
installationUpdatedAt?: string;

inUse?: boolean;
inUse?: { userName: string };
}

export interface ClientHeaderFields {
Expand Down
21 changes: 19 additions & 2 deletions components/server/ee/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1440,8 +1440,25 @@ export class GitpodServerEEImpl extends GitpodServerImpl {
}
const projects = await this.projectsService.getProjectsByCloneUrls(repositories.map(r => r.cloneUrl));

const cloneUrlsInUse = new Set(projects.map(p => p.cloneUrl));
repositories.forEach(r => { r.inUse = cloneUrlsInUse.has(r.cloneUrl) });
const cloneUrlToProject = new Map(projects.map(p => [p.cloneUrl, p]));

for (const repo of repositories) {
const p = cloneUrlToProject.get(repo.cloneUrl);
if (p) {
if (p.userId) {
const owner = await this.userDB.findUserById(p.userId);
if (owner) {
repo.inUse = {
userName: owner?.name || owner?.fullName || 'somebody'
}
}
} else if (p.teamOwners && p.teamOwners[0]) {
repo.inUse = {
userName: p.teamOwners[0] || 'somebody'
}
}
}
}

return repositories;
}
Expand Down
2 changes: 1 addition & 1 deletion components/server/src/projects/projects-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class ProjectsService {
return this.projectDB.findUserProjects(userId);
}

async getProjectsByCloneUrls(cloneUrls: string[]): Promise<Project[]> {
async getProjectsByCloneUrls(cloneUrls: string[]): Promise<(Project & { teamOwners?: string[] })[]> {
return this.projectDB.findProjectsByCloneUrls(cloneUrls);
}

Expand Down