added types, services and repository api route #4
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
There was a problem hiding this comment.
@Dhruwang thanks for the PR 💪 💪💪 Here is some quick feedback ;-) :
| import { TApiKeyCreateInput } from "@/lib/types/apiKey" | ||
|
|
||
| export async function createApiKeyAction( | ||
| accountId: string, |
There was a problem hiding this comment.
in the service you use repositoryId. I would also use this here.
| const session = await getServerSession(authOptions) | ||
| if (!session) return | ||
|
|
||
| return await createApiKey(accountId, apiKeyData) |
There was a problem hiding this comment.
We need an additional check here if the current user has access rights to the repository so he doesn't create an api key for a repo he doesn't belong to
app/api/repository/route.ts
Outdated
There was a problem hiding this comment.
We could get the repository Id also from the API key but I like that it's explicit. But if params in REST APIs are required I love to have them as url parameters. So I would prefer:
/api/repositories/[repositoryId]
app/api/repository/route.ts
Outdated
| import { isApiKeyValid } from "../auth" | ||
|
|
||
| export async function GET(request) { | ||
| if (!isApiKeyValid(request)) return |
There was a problem hiding this comment.
should we add a check if the repository Id of the API key is the same as the requested repository ID?
app/api/repository/route.ts
Outdated
| }) | ||
| } | ||
|
|
||
| //this route is for teting purpose |
There was a problem hiding this comment.
I would uncomment this for now just to make sure we don't forget it later 🙈
lib/repository.ts
Outdated
| import { TRepository, TRepositoryCreateInput } from "./types/repository" | ||
|
|
||
| export const getRepository = async ( | ||
| repositoryId: string |
lib/repository.ts
Outdated
| } | ||
| } | ||
|
|
||
| export const getRepositories = async ( |
There was a problem hiding this comment.
Repositories doesn't have an owner currently. Can we remove this for now?
lib/types/session.ts
Outdated
There was a problem hiding this comment.
Since I just removed the session we can remove this :-)
|
@mattinannt is attempting to deploy a commit to the formbricks Team on Vercel. A member of the Team first needs to authorize it. |
What does this PR do?
Fixes # (issue)
How should this be tested?
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated