feat(api-v2): add GET /me/event-types endpoint #26885
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/api/v2/src/ee/me/me.controller.e2e-spec.ts">
<violation number="1" location="apps/api/v2/src/ee/me/me.controller.e2e-spec.ts:371">
P2: The sorting e2e tests compare event type IDs instead of `createdAt`, so they do not actually verify the endpoint’s `sortCreatedAt` behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+371
to
+372
| expect(responseBody.data[0].id).toBeLessThan(responseBody.data[1].id); | ||
| expect(responseBody.data[1].id).toBeLessThan(responseBody.data[2].id); |
Contributor
There was a problem hiding this comment.
P2: The sorting e2e tests compare event type IDs instead of createdAt, so they do not actually verify the endpoint’s sortCreatedAt behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/ee/me/me.controller.e2e-spec.ts, line 371:
<comment>The sorting e2e tests compare event type IDs instead of `createdAt`, so they do not actually verify the endpoint’s `sortCreatedAt` behavior.</comment>
<file context>
@@ -213,4 +215,184 @@ describe("Me Endpoints", () => {
+ expect(responseBody.data).toBeDefined();
+ expect(responseBody.data?.length).toEqual(3);
+
+ expect(responseBody.data[0].id).toBeLessThan(responseBody.data[1].id);
+ expect(responseBody.data[1].id).toBeLessThan(responseBody.data[2].id);
+ });
</file context>
Fix confidence (alpha): 9/10
Suggested change
| expect(responseBody.data[0].id).toBeLessThan(responseBody.data[1].id); | |
| expect(responseBody.data[1].id).toBeLessThan(responseBody.data[2].id); | |
| expect(new Date(responseBody.data[0].createdAt).valueOf()).toBeLessThan( | |
| new Date(responseBody.data[1].createdAt).valueOf() | |
| ); | |
| expect(new Date(responseBody.data[1].createdAt).valueOf()).toBeLessThan( | |
| new Date(responseBody.data[2].createdAt).valueOf() | |
| ); |
✅ Addressed in 3402352
Contributor
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
The sorting e2e tests were comparing event type IDs instead of createdAt timestamps, which doesn't actually verify the sortCreatedAt behavior. Changed assertions to compare createdAt values to properly test the sorting functionality. Addresses Cubic AI review feedback (confidence 9/10) Co-Authored-By: unknown <>
Contributor
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/api/v2/src/ee/me/me.controller.e2e-spec.ts">
<violation number="1" location="apps/api/v2/src/ee/me/me.controller.e2e-spec.ts:371">
P2: The ascending sort test now asserts the order of `id`s instead of `createdAt`, so it no longer verifies the documented `sortCreatedAt` behavior and could let regressions slip through.</violation>
<violation number="2" location="apps/api/v2/src/ee/me/me.controller.e2e-spec.ts:387">
P2: The descending sort test now compares IDs rather than `createdAt` timestamps, so it cannot detect regressions where `sortCreatedAt=desc` stops sorting by creation time.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| expect(responseBody.data).toBeDefined(); | ||
| expect(responseBody.data?.length).toEqual(3); | ||
|
|
||
| expect(responseBody.data[0].id).toBeGreaterThan(responseBody.data[1].id); |
Contributor
There was a problem hiding this comment.
P2: The descending sort test now compares IDs rather than createdAt timestamps, so it cannot detect regressions where sortCreatedAt=desc stops sorting by creation time.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/ee/me/me.controller.e2e-spec.ts, line 387:
<comment>The descending sort test now compares IDs rather than `createdAt` timestamps, so it cannot detect regressions where `sortCreatedAt=desc` stops sorting by creation time.</comment>
<file context>
@@ -388,12 +384,47 @@ describe("Me Endpoints", () => {
- expect(new Date(responseBody.data[1].createdAt).valueOf()).toBeGreaterThan(
- new Date(responseBody.data[2].createdAt).valueOf()
- );
+ expect(responseBody.data[0].id).toBeGreaterThan(responseBody.data[1].id);
+ expect(responseBody.data[1].id).toBeGreaterThan(responseBody.data[2].id);
+ });
</file context>
Fix confidence (alpha): 9/10
Suggested change
| expect(responseBody.data[0].id).toBeGreaterThan(responseBody.data[1].id); | |
| expect(new Date(responseBody.data[0].createdAt).valueOf()).toBeGreaterThan( | |
| new Date(responseBody.data[1].createdAt).valueOf() | |
| ); |
| expect(responseBody.data).toBeDefined(); | ||
| expect(responseBody.data?.length).toEqual(3); | ||
|
|
||
| expect(responseBody.data[0].id).toBeLessThan(responseBody.data[1].id); |
Contributor
There was a problem hiding this comment.
P2: The ascending sort test now asserts the order of ids instead of createdAt, so it no longer verifies the documented sortCreatedAt behavior and could let regressions slip through.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/ee/me/me.controller.e2e-spec.ts, line 371:
<comment>The ascending sort test now asserts the order of `id`s instead of `createdAt`, so it no longer verifies the documented `sortCreatedAt` behavior and could let regressions slip through.</comment>
<file context>
@@ -368,12 +368,8 @@ describe("Me Endpoints", () => {
- expect(new Date(responseBody.data[1].createdAt).valueOf()).toBeLessThan(
- new Date(responseBody.data[2].createdAt).valueOf()
- );
+ expect(responseBody.data[0].id).toBeLessThan(responseBody.data[1].id);
+ expect(responseBody.data[1].id).toBeLessThan(responseBody.data[2].id);
});
</file context>
Fix confidence (alpha): 9/10
Suggested change
| expect(responseBody.data[0].id).toBeLessThan(responseBody.data[1].id); | |
| expect(new Date(responseBody.data[0].createdAt).valueOf()).toBeLessThan( | |
| new Date(responseBody.data[1].createdAt).valueOf() | |
| ); |
Contributor
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. ✅ Pushed commit |
The sortCreatedAt parameter was sorting by id instead of createdAt. This fix ensures the sorting behavior matches the documented parameter name. Addresses Cubic AI review feedback (confidence 9/10) Co-Authored-By: [email protected] <[email protected]>
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts">
<violation number="1" location="apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts:129">
P2: Sorting by `createdAt` alone makes paginated results unstable—add a secondary key (e.g., `id`) so `skip`/`take` do not return duplicates when timestamps tie.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| userId, | ||
| }, | ||
| ...(sortCreatedAt && { orderBy: { id: sortCreatedAt } }), | ||
| ...(sortCreatedAt && { orderBy: { createdAt: sortCreatedAt } }), |
Contributor
There was a problem hiding this comment.
P2: Sorting by createdAt alone makes paginated results unstable—add a secondary key (e.g., id) so skip/take do not return duplicates when timestamps tie.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, line 129:
<comment>Sorting by `createdAt` alone makes paginated results unstable—add a secondary key (e.g., `id`) so `skip`/`take` do not return duplicates when timestamps tie.</comment>
<file context>
@@ -126,7 +126,7 @@ export class EventTypesRepository_2024_06_14 {
userId,
},
- ...(sortCreatedAt && { orderBy: { id: sortCreatedAt } }),
+ ...(sortCreatedAt && { orderBy: { createdAt: sortCreatedAt } }),
...(skip !== undefined && { skip }),
...(take !== undefined && { take }),
</file context>
Fix confidence (alpha): 9/10
Suggested change
| ...(sortCreatedAt && { orderBy: { createdAt: sortCreatedAt } }), | |
| ...(sortCreatedAt && { orderBy: [{ createdAt: sortCreatedAt }, { id: sortCreatedAt }] }), |
✅ Addressed in fbe2f43
Contributor
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. ✅ Pushed commit |
When sorting by createdAt, add id as a secondary sort key to ensure stable pagination results when timestamps tie. This prevents duplicates or missing items when using skip/take pagination. Addresses Cubic AI review feedback (confidence 9/10) Co-Authored-By: [email protected] <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds a new
GET /me/event-typesendpoint to the API v2 me controller that allows authenticated users to fetch their own event types without needing to specify user identification parameters.This is a simplified version of the existing
GET /event-typesendpoint that:username,eventSlug,usernames,orgSlug,orgId)sortCreatedAtquery parameter for ordering results by creation dateskipandtakequery parameters (default: skip=0, take=250)Link to Devin run: https://app.devin.ai/sessions/9f8ed53fc10c4ab991020371ef976226
Requested by: @ThyMinimalDev
Updates since last revision
skipandtakequery parameters (extendsSkipTakePagination)createdAtinstead ofidwhensortCreatedAtis specifiedid) for stable pagination whencreatedAttimestamps tieMandatory Tasks (DO NOT REMOVE)
How should this be tested?
GET /v2/me/event-types?sortCreatedAt=ascand?sortCreatedAt=descto verify sorting works?take=2to limit results?skip=1to skip first result?skip=1&take=1Checklist for Human Reviewers
OutputEventTypesService_2024_06_14.getResponseEventType()produces correct output formatcreatedAtas primary key andidas secondary key for stable pagination