From e83d8e70b736c36920dbdfe4e4c5ed8e7bcddc4d Mon Sep 17 00:00:00 2001 From: Federico Ruggi <1081051+ruggi@users.noreply.github.com> Date: Fri, 22 Mar 2024 16:18:43 +0100 Subject: [PATCH] Projects page Github badges (#5094) * add column * update github repo * when updating settings, update the db repo * test update * test handler * github limits * remove copypasta * test helper * fix assert * bff check --- .../src/components/editor/actions/actions.tsx | 23 +++- editor/src/components/editor/server.ts | 27 ++++- server/migrations/010.sql | 3 + server/src/Utopia/Web/Database/Migrations.hs | 1 + .../app/models/project.server.spec.ts | 107 +++++++++++++++++- utopia-remix/app/models/project.server.ts | 21 +++- .../projectCollaborators.server.spec.ts | 4 +- ...ects.$id.github.repository.update.spec.tsx | 97 ++++++++++++++++ ....projects.$id.github.repository.update.tsx | 42 +++++++ utopia-remix/app/routes/projects.tsx | 77 ++++++++++--- utopia-remix/app/test-util.ts | 3 +- utopia-remix/app/types.ts | 39 +++++++ utopia-remix/app/util/github.spec.ts | 18 +++ utopia-remix/app/util/github.ts | 12 ++ utopia-remix/schema.prisma | 1 + 15 files changed, 450 insertions(+), 25 deletions(-) create mode 100644 server/migrations/010.sql create mode 100644 utopia-remix/app/routes-test/internal.projects.$id.github.repository.update.spec.tsx create mode 100644 utopia-remix/app/routes/internal.projects.$id.github.repository.update.tsx create mode 100644 utopia-remix/app/util/github.spec.ts create mode 100644 utopia-remix/app/util/github.ts diff --git a/editor/src/components/editor/actions/actions.tsx b/editor/src/components/editor/actions/actions.tsx index 4b3ac293b0f8..4187469e89e7 100644 --- a/editor/src/components/editor/actions/actions.tsx +++ b/editor/src/components/editor/actions/actions.tsx @@ -350,6 +350,7 @@ import { saveAsset as saveAssetToServer, saveUserConfiguration, updateAssetFileName, + updateGithubRepository, } from '../server' import type { CanvasBase64Blobs, @@ -366,6 +367,7 @@ import type { TrueUpTarget, TrueUpHuggingElement, CollaborativeEditingSupport, + ProjectGithubSettings, } from '../store/editor-state' import { trueUpChildrenOfGroupChanged, @@ -3612,12 +3614,25 @@ export const UPDATE_FNS = { } }, UPDATE_GITHUB_SETTINGS: (action: UpdateGithubSettings, editor: EditorModel): EditorModel => { + const newGithubSettings: ProjectGithubSettings = { + ...editor.githubSettings, + ...action.settings, + } + if (editor.id != null) { + void updateGithubRepository( + editor.id, + newGithubSettings.targetRepository == null + ? null + : { + owner: newGithubSettings.targetRepository.owner, + repository: newGithubSettings.targetRepository.repository, + branch: newGithubSettings.branchName, + }, + ) + } return normalizeGithubData({ ...editor, - githubSettings: { - ...editor.githubSettings, - ...action.settings, - }, + githubSettings: newGithubSettings, }) }, UPDATE_GITHUB_DATA: (action: UpdateGithubData, editor: EditorModel): EditorModel => { diff --git a/editor/src/components/editor/server.ts b/editor/src/components/editor/server.ts index ab2bedb59364..74f2fae9fa80 100644 --- a/editor/src/components/editor/server.ts +++ b/editor/src/components/editor/server.ts @@ -8,7 +8,12 @@ import { thumbnailURL, userConfigURL, } from '../../common/server' -import type { PersistentModel, UserConfiguration, UserPermissions } from './store/editor-state' +import type { + GithubRepo, + PersistentModel, + UserConfiguration, + UserPermissions, +} from './store/editor-state' import { emptyUserConfiguration, emptyUserPermissions } from './store/editor-state' import type { LoginState } from '../../uuiui-deps' import urljoin from 'url-join' @@ -609,3 +614,23 @@ export async function requestProjectAccess(projectId: string): Promise { throw new Error(`Request project access failed (${response.status}): ${response.statusText}`) } } + +export async function updateGithubRepository( + projectId: string, + githubRepository: (GithubRepo & { branch: string | null }) | null, +): Promise { + if (!isBackendBFF()) { + return + } + const url = urljoin(`/internal/projects/${projectId}/github/repository/update`) + const response = await fetch(url, { + method: 'POST', + credentials: 'include', + headers: HEADERS, + mode: MODE, + body: JSON.stringify({ githubRepository: githubRepository }), + }) + if (!response.ok) { + throw new Error(`Update Github repository failed (${response.status}): ${response.statusText}`) + } +} diff --git a/server/migrations/010.sql b/server/migrations/010.sql new file mode 100644 index 000000000000..27251b748368 --- /dev/null +++ b/server/migrations/010.sql @@ -0,0 +1,3 @@ +ALTER TABLE project + ADD COLUMN github_repository text; + diff --git a/server/src/Utopia/Web/Database/Migrations.hs b/server/src/Utopia/Web/Database/Migrations.hs index 48fa0fee68e8..718d753f8122 100644 --- a/server/src/Utopia/Web/Database/Migrations.hs +++ b/server/src/Utopia/Web/Database/Migrations.hs @@ -31,6 +31,7 @@ migrateDatabase verbose includeInitial pool = withResource pool $ \connection -> , MigrationFile "007.sql" "./migrations/007.sql" , MigrationFile "008.sql" "./migrations/008.sql" , MigrationFile "009.sql" "./migrations/009.sql" + , MigrationFile "010.sql" "./migrations/010.sql" ] let initialMigrationCommand = if includeInitial then [MigrationFile "initial.sql" "./migrations/initial.sql"] diff --git a/utopia-remix/app/models/project.server.spec.ts b/utopia-remix/app/models/project.server.spec.ts index 36e8a3ed215d..57f7ec1ec546 100644 --- a/utopia-remix/app/models/project.server.spec.ts +++ b/utopia-remix/app/models/project.server.spec.ts @@ -18,8 +18,15 @@ import { renameProject, restoreDeletedProject, softDeleteProject, + updateGithubRepository, } from './project.server' -import { AccessLevel, AccessRequestStatus } from '../types' +import { + AccessLevel, + MaxGithubBranchNameLength, + MaxGithubOwnerLength, + MaxGithubRepositoryLength, + AccessRequestStatus, +} from '../types' describe('project model', () => { afterEach(async () => { @@ -419,4 +426,102 @@ describe('project model', () => { expect(got.collaborators['four'].map((c) => c.id)[1]).toBe('carol') }) }) + + describe('updateGithubRepository', () => { + beforeEach(async () => { + await createTestUser(prisma, { id: 'bob' }) + await createTestUser(prisma, { id: 'alice' }) + await createTestProject(prisma, { id: 'one', ownerId: 'bob' }) + await prisma.project.update({ + where: { proj_id: 'one' }, + data: { github_repository: 'something' }, + }) + }) + + it('errors if the project is not found', async () => { + const fn = async () => + updateGithubRepository({ projectId: 'unknown', userId: 'bob', repository: null }) + await expect(fn).rejects.toThrow('not found') + }) + + it('errors if the user does not own the project', async () => { + const fn = async () => + updateGithubRepository({ projectId: 'one', userId: 'alice', repository: null }) + await expect(fn).rejects.toThrow('not found') + }) + + it('updates the repository string (null)', async () => { + await updateGithubRepository({ projectId: 'one', userId: 'bob', repository: null }) + const project = await prisma.project.findUnique({ + where: { proj_id: 'one' }, + select: { github_repository: true }, + }) + if (project == null) { + throw new Error('expected project not to be null') + } + expect(project.github_repository).toBe(null) + }) + + it('updates the repository string (without branch)', async () => { + await updateGithubRepository({ + projectId: 'one', + userId: 'bob', + repository: { owner: 'foo', repository: 'bar', branch: null }, + }) + const project = await prisma.project.findUnique({ + where: { proj_id: 'one' }, + select: { github_repository: true }, + }) + if (project == null) { + throw new Error('expected project not to be null') + } + expect(project.github_repository).toBe('foo/bar') + }) + + it('updates the repository string (with branch)', async () => { + await updateGithubRepository({ + projectId: 'one', + userId: 'bob', + repository: { owner: 'foo', repository: 'bar', branch: 'baz' }, + }) + const project = await prisma.project.findUnique({ + where: { proj_id: 'one' }, + select: { github_repository: true }, + }) + if (project == null) { + throw new Error('expected project not to be null') + } + expect(project.github_repository).toBe('foo/bar:baz') + }) + + it('updates the repository string (with branch), trimming to max lengths', async () => { + const repo = { + owner: + 'foooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo', + repository: + 'barrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr', + branch: + 'bazzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz', + } + await updateGithubRepository({ + projectId: 'one', + userId: 'bob', + repository: repo, + }) + const project = await prisma.project.findUnique({ + where: { proj_id: 'one' }, + select: { github_repository: true }, + }) + if (project == null) { + throw new Error('expected project not to be null') + } + expect(project.github_repository).toBe( + repo.owner.slice(0, MaxGithubOwnerLength) + + '/' + + repo.repository.slice(0, MaxGithubRepositoryLength) + + ':' + + repo.branch.slice(0, MaxGithubBranchNameLength), + ) + }) + }) }) diff --git a/utopia-remix/app/models/project.server.ts b/utopia-remix/app/models/project.server.ts index 108d58a15b13..0531f26ae851 100644 --- a/utopia-remix/app/models/project.server.ts +++ b/utopia-remix/app/models/project.server.ts @@ -1,10 +1,11 @@ import { prisma } from '../db.server' -import type { CollaboratorsByProject, ProjectListing } from '../types' +import type { CollaboratorsByProject, GithubRepository, ProjectListing } from '../types' import { AccessLevel, AccessRequestStatus, asAccessLevel, userToCollaborator, + githubRepositoryStringOrNull, type ProjectWithoutContentFromDB, } from '../types' import { ensure } from '../util/api.server' @@ -19,6 +20,7 @@ const selectProjectWithoutContent: Record { @@ -195,3 +197,20 @@ export async function listSharedWithMeProjectsAndCollaborators(params: { collaborators: collaboratorsByProject, } } + +export async function updateGithubRepository(params: { + projectId: string + userId: string + repository: GithubRepository | null +}) { + return prisma.project.update({ + where: { + owner_id: params.userId, + proj_id: params.projectId, + }, + data: { + github_repository: githubRepositoryStringOrNull(params.repository), + modified_at: new Date(), + }, + }) +} diff --git a/utopia-remix/app/models/projectCollaborators.server.spec.ts b/utopia-remix/app/models/projectCollaborators.server.spec.ts index 2f55ac94ef5e..9905941921b5 100644 --- a/utopia-remix/app/models/projectCollaborators.server.spec.ts +++ b/utopia-remix/app/models/projectCollaborators.server.spec.ts @@ -53,7 +53,7 @@ describe('projectCollaborators model', () => { it('returns the collaborator details by project id', async () => { const ids = ['one', 'two', 'four', 'five'] const got = await getCollaborators({ ids: ids, userId: 'bob' }) - expect(Object.keys(got)).toEqual(ids) + expect(Object.keys(got).length).toEqual(4) expect(got['one'].map((c) => c.id)).toEqual(['bob']) expect(got['two'].map((c) => c.id)).toEqual(['bob', 'wendy']) expect(got['four'].map((c) => c.id)).toEqual([]) @@ -62,7 +62,7 @@ describe('projectCollaborators model', () => { it('ignores mismatching projects', async () => { const ids = ['one', 'two', 'three'] const got = await getCollaborators({ ids: ids, userId: 'bob' }) - expect(Object.keys(got)).toEqual(['one', 'two']) + expect(Object.keys(got).length).toEqual(2) expect(got['one'].map((c) => c.id)).toEqual(['bob']) expect(got['two'].map((c) => c.id)).toEqual(['bob', 'wendy']) }) diff --git a/utopia-remix/app/routes-test/internal.projects.$id.github.repository.update.spec.tsx b/utopia-remix/app/routes-test/internal.projects.$id.github.repository.update.spec.tsx new file mode 100644 index 000000000000..130346402fbf --- /dev/null +++ b/utopia-remix/app/routes-test/internal.projects.$id.github.repository.update.spec.tsx @@ -0,0 +1,97 @@ +import { prisma } from '../db.server' +import { handleUpdateGithubRepository } from '../routes/internal.projects.$id.github.repository.update' +import { + createTestProject, + createTestSession, + createTestUser, + newTestRequest, + truncateTables, +} from '../test-util' +import { ApiError } from '../util/errors' + +describe('handleUpdateGithubRepository', () => { + afterEach(async () => { + await truncateTables([ + prisma.userDetails, + prisma.persistentSession, + prisma.project, + prisma.projectID, + ]) + }) + + beforeEach(async () => { + await createTestUser(prisma, { id: 'bob' }) + await createTestUser(prisma, { id: 'alice' }) + await createTestSession(prisma, { key: 'the-key', userId: 'bob' }) + await createTestProject(prisma, { id: 'one', ownerId: 'bob' }) + await createTestProject(prisma, { id: 'two', ownerId: 'alice' }) + }) + + it('requires a user', async () => { + const fn = async () => + handleUpdateGithubRepository(newTestRequest({ method: 'POST', authCookie: 'wrong-key' }), {}) + await expect(fn).rejects.toThrow(ApiError) + await expect(fn).rejects.toThrow('session not found') + }) + it('requires a valid id', async () => { + const fn = async () => + handleUpdateGithubRepository(newTestRequest({ method: 'POST', authCookie: 'the-key' }), {}) + await expect(fn).rejects.toThrow(ApiError) + await expect(fn).rejects.toThrow('id is null') + }) + it('requires a valid request body', async () => { + const fn = async () => { + const req = newTestRequest({ + method: 'POST', + authCookie: 'the-key', + body: JSON.stringify({}), + }) + return handleUpdateGithubRepository(req, { id: 'one' }) + } + + await expect(fn).rejects.toThrow('invalid request') + }) + it('requires a valid project', async () => { + const fn = async () => { + const req = newTestRequest({ + method: 'POST', + authCookie: 'the-key', + body: JSON.stringify({ githubRepository: null }), + }) + return handleUpdateGithubRepository(req, { id: 'doesnt-exist' }) + } + + await expect(fn).rejects.toThrow('Record to update not found') + }) + it('requires ownership of the project', async () => { + const fn = async () => { + const req = newTestRequest({ + method: 'POST', + authCookie: 'the-key', + body: JSON.stringify({ githubRepository: null }), + }) + return handleUpdateGithubRepository(req, { id: 'two' }) + } + + await expect(fn).rejects.toThrow('Record to update not found') + }) + it('updates the github repository', async () => { + const fn = async () => { + const req = newTestRequest({ + method: 'POST', + authCookie: 'the-key', + body: JSON.stringify({ + githubRepository: { owner: 'foo', repository: 'bar', branch: 'baz' }, + }), + }) + return handleUpdateGithubRepository(req, { id: 'one' }) + } + + await fn() + const got = await prisma.project.findUnique({ + where: { proj_id: 'one' }, + select: { github_repository: true }, + }) + expect(got?.github_repository).toEqual('foo/bar:baz') + }) +}) diff --git a/utopia-remix/app/routes/internal.projects.$id.github.repository.update.tsx b/utopia-remix/app/routes/internal.projects.$id.github.repository.update.tsx new file mode 100644 index 000000000000..c481631cee00 --- /dev/null +++ b/utopia-remix/app/routes/internal.projects.$id.github.repository.update.tsx @@ -0,0 +1,42 @@ +import type { ActionFunctionArgs, LoaderFunctionArgs } from '@remix-run/node' +import type { Params } from '@remix-run/react' +import { validateProjectAccess } from '../handlers/validators' +import { updateGithubRepository } from '../models/project.server' +import { UserProjectPermission, isUpdateGithubRepositoryBody } from '../types' +import { ensure, handle, handleOptions, requireUser } from '../util/api.server' +import { Status } from '../util/statusCodes' + +export async function loader(args: LoaderFunctionArgs) { + return handle(args, { + OPTIONS: handleOptions, + }) +} + +export async function action(args: ActionFunctionArgs) { + return handle(args, { + POST: { + handler: handleUpdateGithubRepository, + validator: validateProjectAccess(UserProjectPermission.CAN_MANAGE_PROJECT, { + getProjectId: (params) => params.id, + }), + }, + }) +} + +export async function handleUpdateGithubRepository(req: Request, params: Params) { + const user = await requireUser(req) + + const { id } = params + ensure(id != null, 'id is null', Status.BAD_REQUEST) + + const body = await req.json() + ensure(isUpdateGithubRepositoryBody(body), 'invalid request', Status.BAD_REQUEST) + + await updateGithubRepository({ + projectId: id, + userId: user.user_id, + repository: body.githubRepository, + }) + + return {} +} diff --git a/utopia-remix/app/routes/projects.tsx b/utopia-remix/app/routes/projects.tsx index 3ce7c2b1e699..1042692e39b0 100644 --- a/utopia-remix/app/routes/projects.tsx +++ b/utopia-remix/app/routes/projects.tsx @@ -5,6 +5,7 @@ import { AvatarIcon, CubeIcon, DashboardIcon, + GitHubLogoIcon, GlobeIcon, HamburgerMenuIcon, LockClosedIcon, @@ -49,6 +50,7 @@ import { useProjectMatchesQuery, useSortCompareProject, } from '../util/use-sort-compare-project' +import { githubRepositoryPrettyName } from '../util/github' const SortOptions = ['title', 'dateCreated', 'dateModified'] as const export type SortCriteria = (typeof SortOptions)[number] @@ -665,6 +667,8 @@ const ProjectCard = React.memo( return getOwnerName(project.owner_id, collaborators) }, [collaborators, project]) + const isDarkMode = useIsDarkMode() + const openShareDialog = useOpenShareDialog(project) const canOpenShareDialog = React.useMemo(() => { @@ -795,15 +799,34 @@ const ProjectCard = React.memo( style={{ display: 'flex', flexDirection: 'column', padding: 10, gap: 5, flex: 1 }} >
- - {projectTitle} - + + {projectTitle} + + {when( + project.github_repository != null, + + + , + )} + + store.operations.filter((op) => op.projectId === project.proj_id && !op.errored), + ) + + const projectTitle = React.useMemo(() => { + const renaming = activeOperations.find((op) => op.type === 'rename') + return renaming?.type === 'rename' ? renaming.newTitle : project.title + }, [project, activeOperations]) + + const isDarkMode = useIsDarkMode() + const openShareDialog = useOpenShareDialog(project) const canOpenShareDialog = React.useMemo(() => { @@ -913,21 +947,34 @@ const ProjectRow = React.memo( }} /> - - {project.title} - + + {projectTitle} + + {when( + project.github_repository != null, + + + , + )} + {when( isSharedWithMe && ownerName != null, diff --git a/utopia-remix/app/test-util.ts b/utopia-remix/app/test-util.ts index e5c34cfb6328..c05f2aa754f5 100644 --- a/utopia-remix/app/test-util.ts +++ b/utopia-remix/app/test-util.ts @@ -128,11 +128,12 @@ export function newTestRequest(params?: { headers?: { [key: string]: string } authCookie?: string formData?: FormData + body?: string }): Request { const path = (params?.path ?? '').replace(/^\/+/, '') const req = new Request(`http://localhost:8000/` + path, { method: params?.method, - body: params?.formData, + body: params?.formData ?? params?.body, }) if (params?.headers != null) { diff --git a/utopia-remix/app/types.ts b/utopia-remix/app/types.ts index 1bd96bf93c7d..f9f47baabe13 100644 --- a/utopia-remix/app/types.ts +++ b/utopia-remix/app/types.ts @@ -1,6 +1,8 @@ import type { ProjectAccessRequest, UserDetails } from 'prisma-client' import { Prisma } from 'prisma-client' import { assertNever } from './util/assertNever' +import { ensure } from './util/api.server' +import { Status } from './util/statusCodes' const fullProjectFromDB = Prisma.validator()({ include: { @@ -254,3 +256,40 @@ export function isProjectAccessRequestWithUserDetailsArray( maybe.every(isProjectAccessRequestWithUserDetails) ) } + +export type GithubRepository = { + owner: string + repository: string + branch: string | null +} + +export interface UpdateGithubRepositoryRequestBody { + githubRepository: GithubRepository | null +} + +export function isUpdateGithubRepositoryBody(u: unknown): u is UpdateGithubRepositoryRequestBody { + const maybe = u as UpdateGithubRepositoryRequestBody + return u != null && typeof u === 'object' && maybe.githubRepository !== undefined +} + +// Github-specific constraints +export const MaxGithubOwnerLength = 39 // https://docs.github.com/en/enterprise-cloud@latest/admin/identity-and-access-management/iam-configuration-reference/username-considerations-for-external-authentication +export const MaxGithubRepositoryLength = 100 // https://github.com/dead-claudia/github-limits +export const MaxGithubBranchNameLength = 255 // https://stackoverflow.com/questions/24014361/max-length-of-git-branch-name + +export function githubRepositoryStringOrNull(repo: GithubRepository | null): string | null { + if (repo == null) { + return null + } + + const owner = repo.owner.trim().slice(0, MaxGithubOwnerLength) + ensure(owner.length > 0, 'invalid github owner', Status.BAD_REQUEST) + + const repository = repo.repository.trim().slice(0, MaxGithubRepositoryLength) + ensure(repository.length > 0, 'invalid github repository', Status.BAD_REQUEST) + + const branch = repo.branch == null ? null : repo.branch.trim().slice(0, MaxGithubBranchNameLength) + ensure(branch == null || branch.length > 0, 'invalid github branch', Status.BAD_REQUEST) + + return branch == null ? `${owner}/${repository}` : `${owner}/${repository}:${branch}` +} diff --git a/utopia-remix/app/util/github.spec.ts b/utopia-remix/app/util/github.spec.ts new file mode 100644 index 000000000000..6fe64a1e5fdc --- /dev/null +++ b/utopia-remix/app/util/github.spec.ts @@ -0,0 +1,18 @@ +import { githubRepositoryPrettyName } from './github' + +describe('github', () => { + describe('githubRepositoryStringOrNull', () => { + it('returns empty string on null', async () => { + expect(githubRepositoryPrettyName(null)).toBe('') + }) + it('returns the string as-is when malformed', async () => { + expect(githubRepositoryPrettyName('foo')).toBe('foo') + }) + it('returns the owner+repo if branch is missing', async () => { + expect(githubRepositoryPrettyName('foo/bar')).toBe('foo/bar') + }) + it('returns the owner+repo and branch', async () => { + expect(githubRepositoryPrettyName('foo/bar:baz')).toBe('foo/bar (baz)') + }) + }) +}) diff --git a/utopia-remix/app/util/github.ts b/utopia-remix/app/util/github.ts new file mode 100644 index 000000000000..fc8f80de3e82 --- /dev/null +++ b/utopia-remix/app/util/github.ts @@ -0,0 +1,12 @@ +export function githubRepositoryPrettyName(repo: string | null): string { + if (repo == null) { + return '' + } + const parts = repo.split(':') + const name = parts[0] + if (parts.length > 1) { + const branch = parts[1] + return `${name} (${branch})` + } + return name +} diff --git a/utopia-remix/schema.prisma b/utopia-remix/schema.prisma index 59880f365c82..8a90f71b2873 100644 --- a/utopia-remix/schema.prisma +++ b/utopia-remix/schema.prisma @@ -40,6 +40,7 @@ model Project { ProjectCollaborator ProjectCollaborator[] ProjectAccess ProjectAccess? ProjectAccessRequest ProjectAccessRequest[] + github_repository String? @@map("project") }