feat: search files by name - #47
Merged
Merged
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces the US-10 feature to search assets by file name. It adds a new GraphQL query endpoint with pagination, implements account-scoped, status-filtered file searches via repository methods, applies LIKE-based literal matching with proper escaping, and provides comprehensive unit and integration tests alongside updated ADR documentation. ChangesSearch Assets by File Name
Sequence DiagramsequenceDiagram
actor Client
participant GraphQL as GraphQL Handler
participant Resolver as SearchAssetsResolver
participant Service as SearchAssetsService
participant Repo as AssetRepository
participant DB as MySQL Database
Client->>GraphQL: searchAssets(query, page, pageSize)
GraphQL->>Resolver: resolve(args, context)
Resolver->>Resolver: Extract accountId from context
Resolver->>Service: searchAssets(SearchAssetsQuery)
Service->>Service: Validate & normalize input
Service->>Repo: countByFileName(accountId, query, status)
Repo->>DB: SELECT COUNT(*) WHERE account_id=? AND status=? AND file_name LIKE ?
DB-->>Repo: totalCount
Service->>Repo: searchByFileName(accountId, query, status, offset, limit)
Repo->>DB: SELECT * WHERE account_id=? AND status=? AND file_name LIKE ? ORDER BY created_at, id LIMIT ? OFFSET ?
DB-->>Repo: Asset rows
Repo-->>Service: Asset objects
Service->>Service: Map Assets to SearchAssetsFile DTOs
Service-->>Resolver: SearchAssetsResult
Resolver->>Resolver: Transform to GraphQL response array
Resolver-->>GraphQL: { files, totalCount, pageInfo, userErrors }
GraphQL-->>Client: GraphQL response (JSON)
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/Integration/Infrastructure/Persistence/MySQLAssetRepositoryTest.php (1)
316-333: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMake the cross-account fixture eligible for the same status filter.
$otherAccountMatchis a pending asset, so this test still passes ifaccount_idfiltering is accidentally removed because theUPLOADEDpredicate excludes it first. Using an uploaded asset here would make the test actually guard against cross-account leakage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Integration/Infrastructure/Persistence/MySQLAssetRepositoryTest.php` around lines 316 - 333, The cross-account fixture $otherAccountMatch is created as a pending asset which doesn't trigger the UPLOADED filter; update the fixture so $otherAccountMatch is created as an uploaded asset (use the same helper that builds fixtures - e.g., change the call to pendingAsset to produce an uploaded asset or use an uploadedAsset helper) so its status equals AssetStatus::UPLOADED before repository->save; this ensures repository->countByFileName and repository->searchByFileName (the tested methods) will actually verify cross-account filtering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/logs/10-search-files-by-name.md`:
- Around line 45-48: Clarify that the reported test counts refer to different
suites: update the text around the entries referencing
SearchAssetsServiceTest.php and GraphQLHandlerTest.php to explicitly label those
results as "unit tests" (29 tests, 217 assertions) and the MySQL run as
"integration tests" (OK 14 tests, 217 assertions), and add a brief note
explaining why the assertion count is the same across both runs (e.g.,
overlapping assertions or different subsets executed) so readers aren’t confused
by the identical assertion total.
- Line 13: Replace the current status line "**PR:** Creation in progress." with
a clear reference to the existing pull request by updating it to something like
"**PR:** Included in PR `#47`" (keeping the bold format); locate the exact string
"**PR:** Creation in progress." in docs/logs/10-search-files-by-name.md and
change it to reflect PR `#47` so the documentation accurately references the PR.
In `@src/Application/Asset/Result/SearchAssetsResult.php`:
- Around line 9-12: The docblock for the SearchAssetsResult constructor/type
hints references UserError but lacks a use/import, causing static analysers to
resolve it to the current namespace; locate the actual UserError class FQCN
(search for the class definition) and add a corresponding use statement (e.g.
use App\Path\To\UserError;) at the top of SearchAssetsResult.php so the `@param`
list<UserError> $userErrors docblock refers to the correct type; ensure the file
also imports SearchAssetsFile if not already present.
In `@src/Application/Asset/SearchAssetsService.php`:
- Around line 46-56: When totalCount returned by $this->assets->countByFileName
is 0, avoid calling $this->assets->searchByFileName and return the empty result
early: after computing $totalCount and $pageInfo in SearchAssetsService, add a
conditional that if $totalCount === 0 you construct and return the response
(using SearchAssetsPageInfo::fromTotalCount and an empty assets array) instead
of calling searchByFileName; this removes the unnecessary DB round-trip while
preserving existing pagination info.
In `@src/GraphQL/Schema/schema.graphql`:
- Around line 323-328: The GraphQL doc for SearchAssetsPageInfo::pageSize is
misleading—it's not the raw client-requested value but the effective,
validated/clamped size (capped by MAX_PAGE_SIZE). Update the pageSize field
description in the schema to state that it is the effective/validated page size
returned after server-side validation/clamping (mentioning MAX_PAGE_SIZE) so
clients understand the value may differ from their request.
In `@tests/Unit/Http/GraphQLHandlerTest.php`:
- Around line 962-967: The opening brace for the method createHandler should be
moved up to the same line as the function signature to satisfy PSR-12 (change
the current multi-line signature ending with "): array" followed by a newline
and "{" to a single-line signature with the brace). Edit the createHandler
method declaration so the opening brace is on the same line as the signature
(affects the function named createHandler and its parameter list), then run PHP
CS Fixer or your linter to confirm the signature formatting passes CI.
---
Outside diff comments:
In `@tests/Integration/Infrastructure/Persistence/MySQLAssetRepositoryTest.php`:
- Around line 316-333: The cross-account fixture $otherAccountMatch is created
as a pending asset which doesn't trigger the UPLOADED filter; update the fixture
so $otherAccountMatch is created as an uploaded asset (use the same helper that
builds fixtures - e.g., change the call to pendingAsset to produce an uploaded
asset or use an uploadedAsset helper) so its status equals AssetStatus::UPLOADED
before repository->save; this ensures repository->countByFileName and
repository->searchByFileName (the tested methods) will actually verify
cross-account filtering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9f148dc-8449-4f6f-8165-7e2c30b4b4fb
📒 Files selected for processing (17)
docs/adr/04-asset-domain-contracts.mddocs/logs/10-search-files-by-name.mdmkdocs.ymlpublic/index.phpsrc/Application/Asset/Command/SearchAssetsQuery.phpsrc/Application/Asset/Result/SearchAssetsFile.phpsrc/Application/Asset/Result/SearchAssetsPageInfo.phpsrc/Application/Asset/Result/SearchAssetsResult.phpsrc/Application/Asset/SearchAssetsService.phpsrc/Domain/Asset/AssetRepositoryInterface.phpsrc/GraphQL/Resolver/SearchAssetsResolver.phpsrc/GraphQL/Schema/schema.graphqlsrc/GraphQL/SchemaFactory.phpsrc/Infrastructure/Persistence/MySQLAssetRepository.phptests/Integration/Infrastructure/Persistence/MySQLAssetRepositoryTest.phptests/Unit/Application/Asset/SearchAssetsServiceTest.phptests/Unit/Http/GraphQLHandlerTest.php
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
1 participant
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
This PR implements US-10: Search Files by Name, adding a GraphQL
searchAssetsquery for account-scoped file-name search across uploaded digital assets. The feature enables users to search their uploaded files with pagination support, friendly error handling, and deterministic result ordering.Core Changes
Application Layer
Domain Contracts (ADR-04)
Extended
AssetRepositoryInterfacewith:searchByFileName(AccountId, string, AssetStatus, int, int): array— account-scoped, status-filtered, paginated file-name search with deterministic ordering (created_at DESC, id ASC)countByFileName(AccountId, string, AssetStatus): int— count matching assets for pagination metadata without filtering in PHPInfrastructure (MySQL)
%and_charactersGraphQL Integration
searchAssets(query: String!, page: Int = 1, pageSize: Int = 10)query with SearchAssetsPayload, SearchAssetsFile, and SearchAssetsPageInfo typesBehavior
Query:
Key Features:
RepositoryUnavailableExceptionTesting
%/_search semanticsDocumentation
Architecture Alignment
Follows clean architecture (Domain → Application → Infrastructure → GraphQL) and DDD principles: