feat: add filters on getAllOrgs#12
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds optional filtering (name, ong_type) to the GetAllOrganizations flow: controller validates query, forwards filters to the use-case, which passes them to the repository that applies Prisma where clauses. Also moves Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as "Client (HTTP)"
participant Controller as "Controller\n(getAllOrganizations)"
participant UseCase as "GetAllOrganizationsUseCase"
participant Repo as "OrganizationsRepository"
participant DB as "Prisma / Database"
Client->>Controller: GET /organizations?name=...&ong_type=...
Controller->>Controller: validate query (getAllOrganizationsQuerySchema)
Controller->>UseCase: execute({ name, ong_type })
UseCase->>Repo: getAllOrganizations(filters)
Repo->>DB: prisma.organization.findMany(where: { organization_profile: { name: { contains, mode: "insensitive" }, ong_type: ... } })
DB-->>Repo: rows
Repo-->>UseCase: organizations[]
UseCase-->>Controller: { organizations }
Controller-->>Client: 200 OK + payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
📝 Coding Plan
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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/adapters/inbound/http/controllers/organizations/get-all-organizations/schema.ts (1)
3-5: Consider rejecting empty strings for the name filter.An empty string
?name=will pass validation and trigger the filter logic in the repository, thoughcontains: ""will match all records anyway. For cleaner semantics, consider treating empty strings as "no filter":💡 Optional: Transform empty strings to undefined
export const getAllOrganizationsQuerySchema = z.object({ - name: z.string().optional() + name: z.string().optional().transform(val => val?.trim() || undefined) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adapters/inbound/http/controllers/organizations/get-all-organizations/schema.ts` around lines 3 - 5, The query schema getAllOrganizationsQuerySchema currently accepts an empty string for name which leads to a no-op filter; update the Zod schema so empty strings are treated as no filter—either reject them with z.string().min(1).optional() or preprocess/transform empty-string to undefined (e.g. use z.preprocess to map "" -> undefined before applying z.string().optional()) so repository filtering only runs for non-empty names.src/core/use-cases/organizations/get-all-organizations/index.ts (1)
13-15: Pre-existing issue: The null check is ineffective forfindManyresults.Prisma's
findManyreturns an empty array[](notnull/undefined) when no records match. Since an empty array is truthy, this condition will never trigger. The error typeOrganizationDoesNotExistErroralso seems semantically incorrect for a list endpoint.This is pre-existing code and not introduced by this PR, but consider addressing it in a follow-up.
💡 Optional: Remove ineffective check or adjust semantics
execute = async (filters?: { name?: string }): Promise<GetAllOrganizationsUseCaseReturn> => { const data = await this.organizationsRepository.getAllOrganizations(filters) - if (!data) { - throw new OrganizationDoesNotExistError() - } - return { organizations: data } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/use-cases/organizations/get-all-organizations/index.ts` around lines 13 - 15, The null check around the Prisma findMany result is ineffective because findMany returns an array (empty array is truthy); in the get-all-organizations use case (the code that throws OrganizationDoesNotExistError when `if (!data)`), either remove the check entirely and let the caller handle an empty array, or change the condition to `if (data.length === 0)` and throw a more appropriate error (e.g., OrganizationsNotFoundError) or return an empty list; update the handling in the function that calls `findMany` (refer to the getAllOrganizations handler and the OrganizationDoesNotExistError symbol) so behavior is semantically correct for list endpoints.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/adapters/inbound/http/controllers/organizations/get-all-organizations/schema.ts`:
- Around line 3-5: The query schema getAllOrganizationsQuerySchema currently
accepts an empty string for name which leads to a no-op filter; update the Zod
schema so empty strings are treated as no filter—either reject them with
z.string().min(1).optional() or preprocess/transform empty-string to undefined
(e.g. use z.preprocess to map "" -> undefined before applying
z.string().optional()) so repository filtering only runs for non-empty names.
In `@src/core/use-cases/organizations/get-all-organizations/index.ts`:
- Around line 13-15: The null check around the Prisma findMany result is
ineffective because findMany returns an array (empty array is truthy); in the
get-all-organizations use case (the code that throws
OrganizationDoesNotExistError when `if (!data)`), either remove the check
entirely and let the caller handle an empty array, or change the condition to
`if (data.length === 0)` and throw a more appropriate error (e.g.,
OrganizationsNotFoundError) or return an empty list; update the handling in the
function that calls `findMany` (refer to the getAllOrganizations handler and the
OrganizationDoesNotExistError symbol) so behavior is semantically correct for
list endpoints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f73495a8-9b60-4865-a67b-a77c4d6a69e2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.jsonsrc/adapters/inbound/http/controllers/organizations/get-all-organizations/index.tssrc/adapters/inbound/http/controllers/organizations/get-all-organizations/schema.tssrc/adapters/outbound/prisma/repositories/organization-repository.tssrc/core/domain/ports/repositories/organization-repository.tssrc/core/use-cases/organizations/get-all-organizations/index.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/domain/ports/repositories/organization-repository.ts (1)
1-1: DefineOngCategoryin core domain instead of importing from Prisma.The port couples to
@prisma-generatedthrough both the import (line 1) and usage in the method signature (line 13). CreateOngCategoryas a core domain type and implement mapping in the Prisma adapter to maintain proper dependency direction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/domain/ports/repositories/organization-repository.ts` at line 1, Replace the direct import of OngCategory from `@prisma-generated` with a core-domain definition: add a domain-level OngCategory type alongside Organization and remove Prisma from the imports in organization-repository.ts, then update any method signatures in that file that reference OngCategory to use the new core OngCategory type; finally, implement explicit mapping functions in the Prisma adapter (e.g., in the PrismaOrganizationRepository adapter, add mapPrismaOngCategoryToDomain and mapDomainOngCategoryToPrisma) to translate between the Prisma enum and the new domain enum so the core port no longer depends on `@prisma-generated`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/adapters/outbound/prisma/repositories/organization-repository.ts`:
- Around line 70-75: The current getAllOrganizations query always includes a
where: { organization_profile: { ... } } clause which excludes organizations
without profiles; change getAllOrganizations so you only add the
organization_profile predicate when filters?.name or filters?.ong_type are
present: build the Prisma where object conditionally (omit organization_profile
entirely when no filters) and only merge ...(filters?.name && ...) and
...(filters?.ong_type && ...) into organization_profile when filters exist,
ensuring the final query omits the where key when no profile filters are
applied.
---
Nitpick comments:
In `@src/core/domain/ports/repositories/organization-repository.ts`:
- Line 1: Replace the direct import of OngCategory from `@prisma-generated` with a
core-domain definition: add a domain-level OngCategory type alongside
Organization and remove Prisma from the imports in organization-repository.ts,
then update any method signatures in that file that reference OngCategory to use
the new core OngCategory type; finally, implement explicit mapping functions in
the Prisma adapter (e.g., in the PrismaOrganizationRepository adapter, add
mapPrismaOngCategoryToDomain and mapDomainOngCategoryToPrisma) to translate
between the Prisma enum and the new domain enum so the core port no longer
depends on `@prisma-generated`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46516a76-a218-4d71-8f16-d5cbf14935ed
📒 Files selected for processing (6)
package.jsonsrc/adapters/inbound/http/controllers/organizations/get-all-organizations/index.tssrc/adapters/inbound/http/controllers/organizations/get-all-organizations/schema.tssrc/adapters/outbound/prisma/repositories/organization-repository.tssrc/core/domain/ports/repositories/organization-repository.tssrc/core/use-cases/organizations/get-all-organizations/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/adapters/inbound/http/controllers/organizations/get-all-organizations/index.ts
- package.json
Comment on lines
+70
to
+75
| where: { | ||
| organization_profile: { | ||
| ...(filters?.name && { name: { contains: filters.name, mode: 'insensitive' } }), | ||
| ...(filters?.ong_type && { ong_type: filters.ong_type }) | ||
| } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify optional relation cardinality and current repository filter shape.
rg -n "organization_profile OrganizationProfile\\?" prisma/schema/account.prisma -C3
rg -n "getAllOrganizations\\s*=\\s*async" src/adapters/outbound/prisma/repositories/organization-repository.ts -A30Repository: ifsp-projects/backend
Length of output: 1177
🏁 Script executed:
#!/bin/bash
# 1. Check if organization_profile can be null/missing (relation cardinality)
rg "organization_profile" prisma/schema/account.prisma -B5 -A5
# 2. Check if there are tests for getAllOrganizations
fd ".*test.*" -e ts -e js | xargs grep -l "getAllOrganizations" 2>/dev/null | head -5
# 3. Look for how organizations are created/seeded
fd ".*seed.*" -e ts -e js | head -5
# 4. Check for any migration or validation logic around organization_profile
rg "organization_profile" src/ -l | head -10Repository: ifsp-projects/backend
Length of output: 1197
🏁 Script executed:
#!/bin/bash
# 1. Check the interface/port definition
cat src/core/domain/ports/repositories/organization-repository.ts
# 2. Look for test files
fd "getAllOrganizations|organization.*test" . -e ts -e js
# 3. Check how this method is used in controllers
rg "getAllOrganizations" src/ -B2 -A2Repository: ifsp-projects/backend
Length of output: 9161
🏁 Script executed:
#!/bin/bash
# 1. Check the actual usage in the controller to see what return type is expected
cat src/adapters/inbound/http/controllers/organizations/get-all-organizations/index.ts
# 2. Look for any test files to understand intended behavior
find . -type f -name "*.test.*" -o -name "*.spec.*" | xargs grep -l "getAllOrganizations" 2>/dev/null | head -5
# 3. Check if there's any documentation or comments about the relation
rg "organization_profile" src/adapters/outbound/prisma/repositories/organization-repository.ts -B3 -A10Repository: ifsp-projects/backend
Length of output: 2745
🏁 Script executed:
#!/bin/bash
# Verify if organizations can actually exist without profiles
# Check creation patterns and any data about orgs without profiles
# 1. Look at how organizations are typically created
rg "prisma.organization.create|Organization.create" src/ -B3 -A5
# 2. Check if there are migrations that might shed light on this
find . -name "*.sql" -o -path "*/migrations/*" 2>/dev/null | head -10
# 3. Check the organizational structure to understand the domain
cat src/core/domain/ports/repositories/organization-profile-repository.ts | head -30
# 4. Look for any explicit checks or comments about organization without profile
rg "organization.*profile.*null|organization.*without.*profile" src/ -iRepository: ifsp-projects/backend
Length of output: 3141
🏁 Script executed:
#!/bin/bash
# Critical check: Does Prisma's empty object filter actually exclude organizations without profiles?
# Let's look at any documentation or tests that show the intended behavior
# 1. Check the return type expectations
rg "getAllOrganizations.*Promise" src/core -A1
# 2. Look for any seeders or fixtures that might show the expected data state
find . -name "*seed*" -o -name "*fixture*" | grep -E "\.(ts|js)$"
# 3. Check if there's any explicit statement about org-profile requirement
rg "organization_profile.*required|must.*profile|without.*profile" src/ -i
# 4. Check the interface return type - it returns Organization[], not OrganizationWithProfileInclude
# This is a key detail. Let's verify what Organization vs OrganizationWithProfileInclude is
rg "type Organization[^P]|interface Organization" src/ -B2 -A5Repository: ifsp-projects/backend
Length of output: 2793
🏁 Script executed:
#!/bin/bash
# Check the actual return type mismatch and Prisma behavior documentation
# The critical issue: does where: { organization_profile: {} } exclude orgs without profiles?
# 1. Verify the type mismatch - interface says Organization[], implementation includes profile
rg "getAllOrganizations.*=" src/adapters/outbound/prisma/repositories/organization-repository.ts -A25 | grep -E "return|include|where"
# 2. Check if there's any documentation or comments about Prisma relation filtering
rg "organization_profile.*where|where.*organization_profile" src/ -B2 -A2
# 3. Look for any GitHub issues or comments about this
find . -name "*.md" -o -name "*.txt" | xargs grep -i "organization.*profile.*filter" 2>/dev/null
# 4. Check the return type signature consistency
cat > /tmp/check_types.ts << 'EOF'
// The interface definition says:
// getAllOrganizations: (...) => Promise<Organization[]>
// But the implementation does:
// return await prisma.organization.findMany({
// where: { organization_profile: { ... } },
// include: { organization_profile: true }
// })
// This should return OrganizationWithProfileInclude[] or Organization[]?
// Type mismatch suggests the implementation is wrong OR the interface is wrong
EOF
# 5. Verify: when organization_profile is in where clause with empty object,
# does Prisma exclude orgs without profile? This is the core issue.
# From Prisma docs: where relation: {} on optional relation DOES match records WITH the relation
# But to get ALL records (with or without), relation filter should not be in where at all
echo "ANALYSIS: In Prisma, where: { optional_relation: {...} } only matches records WITH that relation"
echo "Therefore, the current code WILL exclude organizations without profiles"Repository: ifsp-projects/backend
Length of output: 389
🏁 Script executed:
#!/bin/bash
# Check if the return type is actually correct or if there's a mismatch
# The interface says Organization[] but implementation returns with includes
# 1. Look at what getAllOrganizations actually returns in practice
rg "getAllOrganizations" src/core/use-cases/organizations/get-all-organizations/ -A10
# 2. Check how the response is used by the controller
rg "useCase.execute" src/adapters/inbound/http/controllers/organizations/get-all-organizations/ -B3 -A5
# 3. Check if Organization type includes organization_profile or not
# This will tell us if the implementation matches the interface
rg "export.*Organization[^P].*=|type Organization[^P]" src/ -l
# 4. Let's check the generated Prisma types
find . -path "*prisma*" -name "*.ts" | grep -i "index\|types" | head -5Repository: ifsp-projects/backend
Length of output: 3621
🏁 Script executed:
#!/bin/bash
# Final verification: confirm Prisma's behavior with empty relation objects in where clauses
# and verify if this impacts actual behavior
# 1. Check if organization_profile is ACTUALLY optional or if it's always created with organization
rg "organization_profile" prisma/schema/account.prisma -B5 -A5 | grep -E "model Organization|model OrganizationProfile|organization_profile"
# 2. Look at the full getAllOrganizations implementation once more for precision
sed -n '68,80p' src/adapters/outbound/prisma/repositories/organization-repository.ts
# 3. Check if any tests actually call getAllOrganizations without filters
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) -exec grep -l "getAllOrganizations" {} \;Repository: ifsp-projects/backend
Length of output: 715
Filtering always requires organization_profile, unintentionally excluding organizations without profiles.
When no filters are provided to getAllOrganizations, the where: { organization_profile: {} } clause still matches only organizations that have a profile, silently excluding organizations without one. Since organization_profile is optional, this causes unexpected data loss for plain GET /organizations requests.
Conditionally apply the organization_profile filter only when actual filters exist (name or ong_type). When no filters are provided, omit the where clause entirely to return all organizations.
Suggested fix
getAllOrganizations = async (filters?: { name?: string; ong_type?: OngCategory }) => {
+ const name = filters?.name?.trim()
+ const hasProfileFilters = Boolean(name || filters?.ong_type)
+
return await prisma.organization.findMany({
- where: {
- organization_profile: {
- ...(filters?.name && { name: { contains: filters.name, mode: 'insensitive' } }),
- ...(filters?.ong_type && { ong_type: filters.ong_type })
- }
- },
+ where: hasProfileFilters
+ ? {
+ organization_profile: {
+ is: {
+ ...(name && { name: { contains: name, mode: 'insensitive' } }),
+ ...(filters?.ong_type && { ong_type: filters.ong_type })
+ }
+ }
+ }
+ : undefined,
include: {
organization_profile: true
}
})
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/adapters/outbound/prisma/repositories/organization-repository.ts` around
lines 70 - 75, The current getAllOrganizations query always includes a where: {
organization_profile: { ... } } clause which excludes organizations without
profiles; change getAllOrganizations so you only add the organization_profile
predicate when filters?.name or filters?.ong_type are present: build the Prisma
where object conditionally (omit organization_profile entirely when no filters)
and only merge ...(filters?.name && ...) and ...(filters?.ong_type && ...) into
organization_profile when filters exist, ensuring the final query omits the
where key when no profile filters are applied.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
32-32: Use the existing workflow variable for Node version instead of hardcoding.
NODE_VERSIONis already defined inenv, but Line 32 hardcodes a patch version. This can drift from org/repo-level configuration and makes updates harder.♻️ Suggested change
- name: Set up Node.js uses: actions/setup-node@v4 with: - node-version: 24.12.0 + node-version: ${{ env.NODE_VERSION }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 32, Replace the hardcoded Node patch version "24.12.0" with the workflow-level variable NODE_VERSION so the job uses the env-defined version; update the action configuration that currently sets node-version: 24.12.0 to reference the env var (e.g., node-version: ${{ env.NODE_VERSION }}) so the setup step (actions/setup-node usage) reads the shared NODE_VERSION instead of a hardcoded value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Line 44: The CI job in .github/workflows/ci.yml currently runs the command "mv
prisma.config.ts prisma.config.ts.bak" unconditionally which will fail if
prisma.config.ts is missing or already renamed; change the step to guard the
rename by checking for the file's existence before attempting the move (or use
the mv option that skips when target is absent) so the job no longer errors when
prisma.config.ts is not present; update the step containing the "mv
prisma.config.ts prisma.config.ts.bak" invocation to perform that existence
check or safe-move behavior.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 32: Replace the hardcoded Node patch version "24.12.0" with the
workflow-level variable NODE_VERSION so the job uses the env-defined version;
update the action configuration that currently sets node-version: 24.12.0 to
reference the env var (e.g., node-version: ${{ env.NODE_VERSION }}) so the setup
step (actions/setup-node usage) reads the shared NODE_VERSION instead of a
hardcoded value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d517f0e-9fa9-486a-9917-2efa734f5e7f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
.github/workflows/ci.ymlpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
|
|
||
| - name: Run Build | ||
| run: | | ||
| mv prisma.config.ts prisma.config.ts.bak |
There was a problem hiding this comment.
Guard the Prisma config rename to prevent avoidable CI failures.
Line 44 fails the job if prisma.config.ts is absent (or already renamed). That makes the build step brittle.
🛠️ Suggested hardening
- name: Run Build
run: |
- mv prisma.config.ts prisma.config.ts.bak
+ if [ -f prisma.config.ts ]; then
+ mv prisma.config.ts prisma.config.ts.bak
+ fi
pnpm run build📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| mv prisma.config.ts prisma.config.ts.bak | |
| if [ -f prisma.config.ts ]; then | |
| mv prisma.config.ts prisma.config.ts.bak | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml at line 44, The CI job in .github/workflows/ci.yml
currently runs the command "mv prisma.config.ts prisma.config.ts.bak"
unconditionally which will fail if prisma.config.ts is missing or already
renamed; change the step to guard the rename by checking for the file's
existence before attempting the move (or use the mv option that skips when
target is absent) so the job no longer errors when prisma.config.ts is not
present; update the step containing the "mv prisma.config.ts
prisma.config.ts.bak" invocation to perform that existence check or safe-move
behavior.
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.
Summary by CodeRabbit