feat(api-key): Add the endpoints and workflows for api key module #6471
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
sradevski
force-pushed
the
feat/add-api-endpoints-api-key
branch
from
f6161e1 to
a96989d
Compare
sradevski
force-pushed
the
feat/add-api-endpoints-api-key
branch
from
a96989d to
0582545
Compare
| @@ -180,8 +178,7 @@ moduleIntegrationTestRunner({ | |||
| it("should not reflect any updates on other fields", async function () { | |||
| const createdApiKey = await service.create(createSecretKeyFixture) | |||
|
|
|||
| const updatedApiKey = await service.update({ | |||
| id: createdApiKey.id, | |||
| const updatedApiKey = await service.update(createdApiKey.id, { | |||
There was a problem hiding this comment.
q: why did we move the id here?
There was a problem hiding this comment.
@adrien2p I just followed the API pattern of the region, as I assumed that's how we want all modules to behave
| * Update an api key | ||
| * @param data | ||
| */ | ||
| update(data: UpdateApiKeyDTO[]): Promise<ApiKeyDTO[]> |
There was a problem hiding this comment.
q: missing shared context, also, is the selector based approach needed here?
There was a problem hiding this comment.
also, it would be better to have the shared context at always the same position as the index of the shared context is stored on the class through the decorator and then used to identify where to put it. Here the second argument of the method could then be data or shared context and can create issues. Maybe an huddle would be easier if you want to discuss it
There was a problem hiding this comment.
@adrien2p similar to above, I just followed the API pattern of the region, as I assumed that's how we want all modules to behave.
I think we just need to standardize how all modules behave basically, as I am not sure what the "expected" public API for each module is.
I would suggest we do the discussion separately from the PR, and we go with this one as it is, and then we modify all modules at once that don't follow the convention.
There was a problem hiding this comment.
I do agree on the standardization for sure 👍 though, you will have an issue with the context coming from a workflow for example. This should be handled now, @olivermrbl you also have the issue in the region I conclude. so it should be handled too cc @carlos-r-l-rodrigues
There was a problem hiding this comment.
@adrien2p let me merge this and I'll update it in the follow-up PR where I enforce the usage of the authentication, since this already built and passed 😄
Added endpoints for managing API keys, as well as the workflows and related necessary changes.