fix: dora team filter#78
Merged
waltergalvao merged 2 commits intomainfrom Feb 19, 2026
Merged
Conversation
sweetr-dev
Bot
added
the
large
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+171
to
183
| if (filters.teamIds && filters.teamIds.length > 0) { | ||
| conditions.push( | ||
| Prisma.sql`EXISTS ( | ||
| SELECT 1 FROM "TeamMember" tm | ||
| WHERE tm."gitProfileId" = ${Prisma.raw(alias)}."authorId" | ||
| AND tm."workspaceId" = ${Prisma.raw(alias)}."workspaceId" | ||
| AND tm."teamId" = ANY(ARRAY[${Prisma.join( | ||
| filters.teamIds.map((id) => Prisma.sql`${id}`), | ||
| ", " | ||
| )}])` | ||
| ); | ||
| } | ||
|
|
||
| if (filters.repositoryIds && filters.repositoryIds.length > 0) { | ||
| conditions.push( | ||
| Prisma.sql`a."repositoryId" = ANY(ARRAY[${Prisma.join( | ||
| filters.repositoryIds.map((id) => Prisma.sql`${id}`), | ||
| ", " | ||
| )}])` | ||
| ); | ||
| } | ||
| )}]) | ||
| )` | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
EXISTS subquery doesn't handle NULL authorId. Deployments without an author will be excluded when filtering by teamIds since NULL = anything is false in SQL. Add ${Prisma.raw(alias)}."authorId" IS NOT NULL AND before the EXISTS clause to explicitly handle this edge case.
Contributor
Author
There was a problem hiding this comment.
When filtering by teams, it shouldn't bring data about deployments that has no author. The behaviour is correct.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/app/metrics/services/dora-metrics.service.ts">
<violation number="1" location="apps/api/src/app/metrics/services/dora-metrics.service.ts:175">
P2: The EXISTS subquery doesn't explicitly handle NULL `authorId` values. In SQL, `NULL = anything` evaluates to UNKNOWN (not true), so deployments without an author will be silently excluded when filtering by `teamIds`. Consider adding an explicit NULL check or documenting this as expected behavior.</violation>
<violation number="2" location="apps/api/src/app/metrics/services/dora-metrics.service.ts:533">
P2: Same NULL `authorId` handling issue exists in this EXISTS subquery. When `cd.authorId` is NULL, the comparison will evaluate to UNKNOWN in SQL, causing incidents linked to deployments without authors to be excluded when filtering by `teamIds`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
WalkthroughThe changes implement team-member-based filtering for DORA metrics by replacing application-owner-based filtering with deployment-author membership checks. This includes refactoring the metric service's filter logic to use EXISTS subqueries for team verification, adding a new seed helper function to establish team-member relationships, and updating integration tests to propagate author information through test data. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/app/metrics/services/dora-metrics.integration.test.ts`:
- Around line 508-559: Add a new integration test in
dora-metrics.integration.test.ts that verifies deployments with a NULL authorId
are included when filtering by teamIds: create context via
createTestContextWithGitProfile(), seedGitProfile(), seedRepository(),
seedTeam() and seedTeamMember() to attach the git profile to the team,
seedApplication(), seedEnvironment({ isProduction: true }), seedPullRequest()
(with createdAt), then call seedDeployment() without supplying authorId (so it
is NULL) and link it via seedDeploymentPullRequest(); finally call
getDeploymentFrequencyMetric({ workspaceId, dateRange, period: Period.DAILY,
teamIds: [team.teamId] }) and assert result.currentAmount equals BigInt(1).
Ensure the test name clearly states it “includes deployments with null authorId
when filtering by teamIds.”
In `@apps/api/src/app/metrics/services/dora-metrics.service.ts`:
- Around line 171-183: In buildDeploymentFilters, the EXISTS subquery currently
drops rows when ${Prisma.raw(alias)}."authorId" is NULL; update the WHERE clause
inside the EXISTS (the subquery referencing tm."gitProfileId" and
${Prisma.raw(alias)}."authorId") to treat NULL authorId as a match so authorless
deployments are included for any team filter (e.g., change the predicate to
check tm."gitProfileId" = alias."authorId" OR alias."authorId" IS NULL);
alternatively, if you intend to exclude authorless deployments, add an explicit
AND ${Prisma.raw(alias)}."authorId" IS NOT NULL guard before the EXISTS—adjust
the condition in buildDeploymentFilters accordingly.
- Around line 529-541: In buildIncidentFilters the EXISTS clause `WHERE
tm."gitProfileId" = cd."authorId"` causes incidents to be excluded when
cd."authorId" is NULL; change the condition to allow NULL authorIds by making
the filter accept rows where cd."authorId" IS NULL OR the EXISTS(...) matches a
TeamMember — i.e., wrap the EXISTS check with `cd."authorId" IS NULL OR
EXISTS(...)` (keeping the same INNER EXISTS body matching tm."gitProfileId" =
cd."authorId", tm."workspaceId" = cd."workspaceId" and teamId ANY of
filters.teamIds) so that incidents with no cause author are not silently dropped
when filters.teamIds is applied.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Greptile Summary
This PR changes DORA metrics team filtering from application ownership to deployment author team membership. Previously, filtering by
teamIdsmatched deployments whose applications were owned by those teams. Now it matches deployments whose authors are members of those teams via theTeamMembertable.Key Changes:
Application.teamIdjoin withTeamMemberEXISTS subquery in bothbuildDeploymentFiltersandbuildIncidentFilterscauseDeployment.authorId) instead ofIncident.teamIdorApplication.teamIdseedTeamMemberhelper to properly set up team memberships in testsrimrafdev dependency (likely for test cleanup utilities)Impact:
This is a behavioral change that aligns team metrics with who actually performed deployments rather than which team owns the application. Existing tests were updated to maintain the same assertions by properly creating team memberships.
Confidence Score: 5/5
Important Files Changed
teamIdseedTeamMembercalls and new test casesseedTeamMemberhelper function to create team member relationships in testsEntity Relationship Diagram
%%{init: {'theme': 'neutral'}}%% erDiagram Team ||--o{ TeamMember : has GitProfile ||--o{ TeamMember : member-of GitProfile ||--o{ Deployment : authors Deployment }o--|| Application : deployed-to Application }o--|| Team : owned-by Deployment ||--o{ Incident : causes Team { int id PK string name int workspaceId FK } TeamMember { int id PK int teamId FK int gitProfileId FK string role } GitProfile { int id PK string handle } Deployment { int id PK int applicationId FK int authorId FK "nullable" int environmentId FK datetime deployedAt } Application { int id PK string name int teamId FK "nullable" int repositoryId FK } Incident { int id PK int causeDeploymentId FK int teamId FK "nullable" datetime detectedAt }Last reviewed commit: b7c515d