Skip to content

[server] remove Project.userId #18391

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
Jul 31, 2023
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
30 changes: 9 additions & 21 deletions components/dashboard/src/admin/ProjectDetail.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,15 @@ export default function ProjectDetail(props: { project: Project; owner: string |
{props.project.name}
</a>
</Property>
{props.project.userId ? (
<Property name="Owner">
<Link
className="text-blue-400 dark:text-blue-600 hover:text-blue-600 dark:hover:text-blue-400 truncate"
to={"/admin/users/" + props.project.userId}
>
{props.owner}
</Link>
<span className="text-gray-400 dark:text-gray-500"> (User)</span>
</Property>
) : (
<Property name="Owner">
<Link
className="text-blue-400 dark:text-blue-600 hover:text-blue-600 dark:hover:text-blue-400 truncate"
to={"/admin/orgs/" + props.project.teamId}
>
{props.owner}
</Link>
<span className="text-gray-400 dark:text-gray-500"> (Organization)</span>
</Property>
)}
<Property name="Owner">
<Link
className="text-blue-400 dark:text-blue-600 hover:text-blue-600 dark:hover:text-blue-400 truncate"
to={"/admin/orgs/" + props.project.teamId}
>
{props.owner}
</Link>
<span className="text-gray-400 dark:text-gray-500"> (Organization)</span>
</Property>
</div>
<div className="flex w-full mt-6">
<Property name="Incremental Prebuilds">
Expand Down
10 changes: 2 additions & 8 deletions components/dashboard/src/admin/ProjectsSearch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,10 @@ export function ProjectsSearch() {
useEffect(() => {
(async () => {
if (currentProject) {
if (currentProject.userId) {
const owner = await getGitpodService().server.adminGetUser(currentProject.userId);
const owner = await getGitpodService().server.adminGetTeamById(currentProject.teamId);
if (owner) {
setCurrentProjectOwner(owner.name);
}
if (currentProject.teamId) {
const owner = await getGitpodService().server.adminGetTeamById(currentProject.teamId);
if (owner) {
setCurrentProjectOwner(owner.name);
}
}
}
})();
}, [currentProject]);
Expand Down
33 changes: 13 additions & 20 deletions components/dashboard/src/data/projects/list-projects-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,26 @@ import { useCallback } from "react";
import { listAllProjects } from "../../service/public-api";
import { useCurrentOrg } from "../organizations/orgs-query";

type TeamOrUserID = {
teamId?: string;
userId?: string;
};

export type ListProjectsQueryResults = {
projects: Project[];
};

export const useListProjectsQuery = () => {
const team = useCurrentOrg().data;

const teamId = team?.id;

const org = useCurrentOrg().data;
const orgId = org?.id;
return useQuery<ListProjectsQueryResults>({
// Projects are either tied to current team, otherwise current user
queryKey: getListProjectsQueryKey({ teamId }),
queryKey: getListProjectsQueryKey(orgId || ""),
cacheTime: 1000 * 60 * 60 * 1, // 1 hour
queryFn: async () => {
if (!teamId) {
if (!orgId) {
return {
projects: [],
latestPrebuilds: new Map(),
};
}

const projects = await listAllProjects({ teamId });
const projects = await listAllProjects({ orgId });
return {
projects,
};
Expand All @@ -49,24 +42,24 @@ export const useRefreshProjects = () => {
const queryClient = useQueryClient();

return useCallback(
({ teamId, userId }: TeamOrUserID) => {
// Don't refetch if no team/user is provided
if (!teamId && !userId) {
(orgId: string) => {
// Don't refetch if no org is provided
if (!orgId) {
return;
}

queryClient.refetchQueries({
queryKey: getListProjectsQueryKey({ teamId, userId }),
queryKey: getListProjectsQueryKey(orgId),
});
},
[queryClient],
);
};

export const getListProjectsQueryKey = ({ teamId, userId }: TeamOrUserID) => {
if (!teamId && !userId) {
throw new Error("Must provide either a teamId or userId for projects query key");
export const getListProjectsQueryKey = (orgId: string) => {
if (!orgId) {
throw new Error("Must provide either an orgId for projects query key");
}

return ["projects", "list", teamId ? { teamId } : { userId }];
return ["projects", "list", { orgId }];
};
2 changes: 1 addition & 1 deletion components/dashboard/src/projects/NewProject.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ export default function NewProject() {
});

// TODO: After converting this to a mutation, we can handle invalidating/updating the query in a side effect
refreshProjects(project.teamId ? { teamId: project.teamId } : { userId: project.userId || "" });
refreshProjects(project.teamId);

setProject(project);
} catch (error) {
Expand Down
18 changes: 4 additions & 14 deletions components/dashboard/src/projects/project-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,10 @@ export function useCurrentProject(): { project: Project | undefined; loading: bo
return;
}
(async () => {
let projects: Project[];
if (!!org.data) {
projects = await listAllProjects({ teamId: org.data?.id });
} else {
projects = await listAllProjects({ userId: user?.id });
if (!org.data) {
return;
}
let projects = await listAllProjects({ orgId: org.data.id });

// Find project matching with slug, otherwise with name
const project = projects.find((p) => Project.slug(p) === slugs.projectSlug);
Expand All @@ -86,21 +84,13 @@ export function useCurrentProject(): { project: Project | undefined; loading: bo
if (t.id === org.data?.id) {
continue;
}
const projects = await listAllProjects({ teamId: t.id });
const projects = await listAllProjects({ orgId: t.id });
const project = projects.find((p) => Project.slug(p) === slugs.projectSlug);
if (project) {
// redirect to the other org
history.push(location.pathname + "?org=" + t.id);
}
}

// check personal projects
const projects = await listAllProjects({ userId: user.id });
const project = projects.find((p) => Project.slug(p) === slugs.projectSlug);
if (project) {
// redirect to the other org
history.push(location.pathname + "?org=0");
}
}
setProject(project);
setLoading(false);
Expand Down
6 changes: 3 additions & 3 deletions components/dashboard/src/service/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ export function publicApiTeamRoleToProtocol(role: TeamRole): TeamMemberRole {
}
}

export async function listAllProjects(opts: { userId?: string; teamId?: string }): Promise<ProtocolProject[]> {
export async function listAllProjects(opts: { orgId: string }): Promise<ProtocolProject[]> {
let pagination = {
page: 1,
pageSize: 100,
};

const response = await projectsService.listProjects({
teamId: opts.teamId,
teamId: opts.orgId,
pagination,
});
const results = response.projects;
Expand All @@ -84,7 +84,7 @@ export async function listAllProjects(opts: { userId?: string; teamId?: string }
page: 1 + pagination.page,
};
const response = await projectsService.listProjects({
teamId: opts.teamId,
teamId: opts.orgId,
pagination,
});
results.push(...response.projects);
Expand Down
1 change: 0 additions & 1 deletion components/gitpod-db/src/project-db.spec.db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class ProjectDBSpec {
name: "some-project",
slug: "some-project",
cloneUrl: "some-random-clone-url",
userId: user.id,
teamId: "team-1",
appInstallationId: "app-1",
});
Expand Down
1 change: 1 addition & 0 deletions components/gitpod-db/src/redis/publisher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the GNU Affero General Public License (AGPL).
* See License.AGPL.txt in the project root for license information.
*/
import "reflect-metadata";

import { inject, injectable } from "inversify";
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
Expand Down
7 changes: 0 additions & 7 deletions components/gitpod-db/src/typeorm/entity/db-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,6 @@ export class DBProject {
@Index("ind_teamId")
teamId: string;

@Column({
default: "",
transformer: Transformer.MAP_EMPTY_STR_TO_UNDEFINED,
})
@Index("ind_userId")
userId?: string;

@Column()
appInstallationId: string;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the GNU Affero General Public License (AGPL).
* See License.AGPL.txt in the project root for license information.
*/
import "reflect-metadata";

import * as crypto from "crypto";
import { injectable } from "inversify";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export interface Project {
slug?: string;
cloneUrl: string;
teamId: string;
userId?: string;
appInstallationId: string;
settings?: ProjectSettings;
creationTime: string;
Expand Down
4 changes: 2 additions & 2 deletions components/server/src/prebuilds/prebuild-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,14 +398,14 @@ export class PrebuildManager {
commit: CommitInfo,
) {
const span = TraceContext.startSpan("storePrebuildInfo", ctx);
const { userId, teamId, name: projectName, id: projectId } = project;
const { teamId, name: projectName, id: projectId } = project;
try {
await this.workspaceDB.trace({ span }).storePrebuildInfo({
id: pws.id,
buildWorkspaceId: pws.buildWorkspaceId,
basedOnPrebuildId: ws.basedOnPrebuildId,
teamId,
userId,
userId: user.id,
projectName,
projectId,
startedAt: pws.creationTime,
Expand Down
11 changes: 5 additions & 6 deletions components/server/src/projects/projects-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ export class ProjectsService {
});

// Install the prebuilds webhook if possible
const { userId, teamId, cloneUrl } = project;
const { teamId, cloneUrl } = project;
const parsedUrl = RepoURL.parseRepoUrl(project.cloneUrl);
const hostContext = parsedUrl?.host ? this.hostContextProvider.get(parsedUrl?.host) : undefined;
const authProvider = hostContext && hostContext.authProvider.info;
Expand All @@ -286,11 +286,10 @@ export class ProjectsService {
// in the project creation flow, we only propose repositories where the user is actually allowed to
// install a webhook.
if (await repositoryService.canInstallAutomatedPrebuilds(installer, cloneUrl)) {
log.info("Update prebuild installation for project.", {
teamId,
userId,
installerId: installer.id,
});
log.info(
{ organizationId: teamId, userId: installer.id },
"Update prebuild installation for project.",
);
await repositoryService.installAutomatedPrebuilds(installer, cloneUrl);
}
}
Expand Down
12 changes: 2 additions & 10 deletions components/server/src/workspace/gitpod-server-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2843,16 +2843,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
if (!project) {
throw new ApplicationError(ErrorCodes.NOT_FOUND, "Project not found");
}
// TODO(janx): This if/else block should probably become a GuardedProject.
if (project.userId) {
if (user.id !== project.userId) {
// Projects owned by a single user can only be accessed by that user
throw new ApplicationError(ErrorCodes.PERMISSION_DENIED, "Not allowed to access this project");
}
} else {
// Anyone who can read a team's information (i.e. any team member) can manage team projects
await this.guardTeamOperation(project.teamId || "", "get", "not_implemented");
}
// Anyone who can read a team's information (i.e. any team member) can manage team projects
await this.guardTeamOperation(project.teamId, "get", "not_implemented");
}

public async deleteProject(ctx: TraceContext, projectId: string): Promise<void> {
Expand Down
16 changes: 4 additions & 12 deletions components/server/src/workspace/workspace-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,14 +240,10 @@ export class WorkspaceFactory {
};

let projectId: string | undefined;
// associate with a project, if it's the personal project of the current user
if (project?.userId && project?.userId === user.id) {
projectId = project.id;
}
// associate with a project, if the current user is a team member
if (project?.teamId) {
if (project) {
const teams = await this.teamDB.findTeamsByUser(user.id);
if (teams.some((t) => t.id === project?.teamId)) {
if (teams.some((t) => t.id === project.teamId)) {
projectId = project.id;
}
}
Expand Down Expand Up @@ -380,14 +376,10 @@ export class WorkspaceFactory {
}

let projectId: string | undefined;
// associate with a project, if it's the personal project of the current user
if (project?.userId && project?.userId === user.id) {
projectId = project.id;
}
// associate with a project, if the current user is a team member
if (project?.teamId) {
if (project) {
const teams = await this.teamDB.findTeamsByUser(user.id);
if (teams.some((t) => t.id === project?.teamId)) {
if (teams.some((t) => t.id === project.teamId)) {
projectId = project.id;
}
}
Expand Down