feat: Enhanced file filtering and grouping by file paths#265
Merged
JamesGeorg merged 2 commits intomainfrom Mar 9, 2026
Merged
feat: Enhanced file filtering and grouping by file paths#265JamesGeorg merged 2 commits intomainfrom
JamesGeorg merged 2 commits intomainfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces a grouped, paginated file management system across the dashboard and backend. It adds a new FileChooser component for browsing and selecting files with tag filtering and search, integrates this component into package creation and release builder workflows, adds new backend endpoints for file groups and tags, and updates the files page to display grouped file views. Changes
Sequence Diagram(s)sequenceDiagram
actor User as User
participant UI as FileChooser UI
participant API as API Client
participant Server as Backend Server
participant DB as Database
User->>UI: Enter search term or select tag
UI->>API: Debounce & fetch groups (search + tag filters)
API->>Server: GET /file/groups?search=...&tags=...&page=1
Server->>DB: Query file groups with tag/search filters
DB-->>Server: Return matching groups + pagination metadata
Server-->>API: FileGroupsResponse {groups, total_items, total_pages}
API-->>UI: Render groups, versions, tags
User->>UI: Expand file group & select version
UI->>UI: Toggle version in selected files
UI->>UI: Convert SelectedFile[] to resource IDs
User->>UI: Click confirm/create
UI->>API: POST /packages or /releases with selected file IDs
API->>Server: Create package/release with resource references
Server-->>API: Success response
API-->>User: Confirm & navigate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
yuvrajjsingh0
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (14)
airborne_dashboard/app/dashboard/[orgId]/[appId]/files/page.tsx (1)
68-73: Hardcoded limit of 100 tags may truncate available options.The tags endpoint fetches with
count: 100, which could miss tags if an application has more than 100 unique tags. Consider fetching all tags (all: true) or implementing a searchable tag selector for larger datasets.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/files/page.tsx around lines 68 - 73, The current useSWR call that populates tagsData uses apiFetch("/file/tags", ...) with a hardcoded query of { page: 1, count: 100 } which can truncate results; update the query passed to apiFetch in the useSWR block (the call inside useSWR that returns apiFetch<TagsResponse>("/file/tags", ...)) to request all tags (e.g., replace count: 100 with all: true or add all: true depending on backend support) or implement a paginated/searchable approach (e.g., switch to server-side search or incremental loading in the tag selector) so the "/file/tags" endpoint returns the full set instead of only the first 100; ensure the downstream handling of tagsData in the component accommodates the full result shape.airborne_server/src/types.rs (1)
370-377: Consider handling empty string query parameters gracefully.If a query parameter is present but empty (e.g.,
?page=), this will deserialize asSome("")andparse()will fail with an error. Depending on desired behavior, you may want to treat empty strings asNone:♻️ Optional: treat empty strings as absent
fn de_u32_from_str<'de, D>(d: D) -> std::result::Result<Option<u32>, D::Error> where D: Deserializer<'de>, { let opt: Option<String> = Option::deserialize(d)?; - opt.map(|s| s.parse().map_err(serde::de::Error::custom)) + opt.filter(|s| !s.is_empty()) + .map(|s| s.parse().map_err(serde::de::Error::custom)) .transpose() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_server/src/types.rs` around lines 370 - 377, The deserializer de_u32_from_str currently treats an empty query value (Some("")) as a parse error; update de_u32_from_str so that when Option<String> is Some(s) and s.trim().is_empty() it returns Ok(None) instead of attempting to parse, otherwise parse the string to u32 and propagate parse errors via serde::de::Error::custom; refer to the function name de_u32_from_str and the existing Option<String> handling to locate where to add the empty-string check and branch.airborne_server/src/file/groups.rs (1)
128-175: N+1 query pattern may cause performance issues at scale.For each
file_pathin the page, two additional queries are executed (fetch versions + count total). With a page of 15 items, this results in 30+ queries per request. Consider batching these queries or using a single query with window functions for better performance as data grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_server/src/file/groups.rs` around lines 128 - 175, The loop over file_paths causes an N+1 query pattern: for each fp you run two queries (the file_versions query and total_versions count) which scales poorly; change to batch queries by (1) fetching all relevant versions for the page in one query (filter files.filter(org_id.eq(&organisation)).filter(app_id.eq(&application)).filter(file_path.eq_any(&file_paths)).order(file_path.asc()).order(version.desc()).select(DbFile::as_select()).load(&mut conn)?) and grouping results in-memory by file_path to build the versions and tags vectors for each FileGroupResponse, and (2) getting total_versions for all file_paths in one grouped count query (files.filter(...).filter(file_path.eq_any(&file_paths)).select((file_path, diesel::dsl::count_star())).group_by(file_path).load(...)?) then map counts back to each fp; update the code that currently uses file_versions, total_versions and the for fp in file_paths loop to use these batched results.airborne_dashboard/components/release/ReleaseBuilder.tsx (1)
133-133: Hardcoded sidebar offset may break if layout changes.The
left-64assumes a fixed sidebar width of 256px. If the sidebar width changes (e.g., for responsive design or a collapsed state), this bar will misalign. Consider using a CSS variable or a layout context to synchronize the offset.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/components/release/ReleaseBuilder.tsx` at line 133, The fixed left-64 on the bottom bar in ReleaseBuilder.tsx will misalign if the sidebar width changes; replace the hardcoded class on the <div> (the element with className="fixed bottom-0 left-64 right-0 bg-background border-t p-4 z-50") with a responsive offset driven by a CSS variable or layout context: remove left-64 and apply a left style like left: var(--sidebar-width) (or compute using a sidebarWidth prop from your layout context and set style={{ left: sidebarWidth }}), and ensure the sidebar component sets --sidebar-width (e.g., --sidebar-width: 16rem or toggles for collapsed state) so the bar always aligns with the sidebar across breakpoints and collapsed/expanded states.airborne_dashboard/components/file-chooser.tsx (3)
307-323: Redundant condition fortotalPages > 1.The check
totalPages > 1on Line 307 is always true in this branch because we're inside theelseblock wheretotalPages > maxVisible (5). This is harmless but could be simplified.♻️ Simplified condition
- if (totalPages > 1) { - items.push( + items.push(Then remove the corresponding closing brace.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/components/file-chooser.tsx` around lines 307 - 323, The extra check "if (totalPages > 1)" around the final PaginationItem is redundant because this branch already only runs when totalPages > maxVisible (5); remove that outer conditional and its matching closing brace so the PaginationItem block (using PaginationItem, PaginationLink, currentPage and setCurrentPage) is unconditionally rendered in this branch, keeping the inner onClick, isActive and key logic unchanged.
117-129: Consider using a more stable dependency for tag accumulation.The effect uses
tagsData?.dataas a dependency, which may trigger re-runs when SWR returns a new array reference even if the content is the same. For this use case it's likely fine since SWR caches responses, but you could consider usingJSON.stringify(tagsData?.data)or a custom comparison if you observe unnecessary re-renders.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/components/file-chooser.tsx` around lines 117 - 129, The useEffect depends on tagsData?.data which can change reference without content changes; update the dependency to a stable representation (e.g., JSON.stringify(tagsData?.data) or a derived key) so the effect only re-runs when actual tag content changes; modify the useEffect dependency array (and keep logic inside useEffect using tagPage, tagsData?.data and setAccumulatedTags) to use that stable dependency instead of raw tagsData?.data to prevent unnecessary accumulations in the handlers that update accumulated tags.
412-419: Add aria-label to remove button for accessibility.The remove button inside the tag badge lacks an accessible label, which makes it unclear to screen reader users what the button does.
♿ Add aria-label
<button onClick={() => handleTagFilterToggle(tag)} className="hover:bg-muted rounded-sm p-0.5" disabled={disabled} + aria-label={`Remove ${tag} filter`} > <X className="h-3 w-3" /> </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/components/file-chooser.tsx` around lines 412 - 419, The remove button inside the Badge lacks an accessible label; update the button in the Badge where onClick calls handleTagFilterToggle(tag) to include an aria-label describing its action (e.g., "Remove tag {tag}" or "Remove filter {tag}") so screen readers can convey purpose; keep the existing onClick, className, and disabled props and ensure the label incorporates the tag variable to be unique and descriptive for each badge.airborne_server/src/file.rs (2)
590-594: Redundant null filtering in tag results.The
filter_mapon Line 591-593 filters outNonetags, but the base query already includes.filter(tag.is_not_null())(Line 551). While this defensive coding is harmless, it's technically unnecessary given the query constraints.♻️ Simplified mapping (optional)
- let tags: Vec<FileTagInfo> = tag_results - .into_iter() - .filter_map(|(tag_opt, count)| tag_opt.map(|t| FileTagInfo { tag: t, count })) - .collect(); + let tags: Vec<FileTagInfo> = tag_results + .into_iter() + .map(|(tag_opt, count)| FileTagInfo { + tag: tag_opt.expect("base query filters nulls"), + count + }) + .collect();Note: Keeping
filter_mapis safer if query logic changes later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_server/src/file.rs` around lines 590 - 594, The mapping from query results redundantly filters out None values even though the DB query already applies .filter(tag.is_not_null()); remove the unnecessary filter_map by directly mapping tag_results into Vec<FileTagInfo> (use tag_results.into_iter().map(|(tag, count)| FileTagInfo { tag, count }).collect()) to simplify code around the tag_results handling and construction of FileTagInfo while keeping the same behavior; update references in this block where FileTagInfo is produced to use the direct map and ensure types still match.
444-494: Code duplication between count and data queries.The search and tag filtering logic is duplicated between the count query (Lines 446-466) and the data query (Lines 471-494). While this is somewhat unavoidable with Diesel's boxed query pattern, consider extracting a helper macro or function to build the filter conditions, which would improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_server/src/file.rs` around lines 444 - 494, The count_query and data_query blocks duplicate the same search/tag filter construction (using files, org_id, app_id, search_term -> file_path.ilike/url.ilike, and tags_filter -> tag.eq/tag.eq_any) before executing count() or load::<DbFile>(); refactor by extracting a helper that accepts the base boxed query (e.g., files.into_boxed()), the optional &search_term, and &tags_filter and returns the boxed query with filters applied, then call that helper to produce the query for both total (count_query) and file_list (data_query) to eliminate duplication while keeping Diesel types consistent.airborne_dashboard/app/dashboard/[orgId]/[appId]/packages/create/page.tsx (2)
186-192: Hardcoded light-mode colors won't work in dark mode.The selected index file indicator uses
bg-green-50andborder-green-200which are light-mode specific. Consider using theme-aware colors or adding dark mode variants.🎨 Add dark mode support
- <div className="mt-4 p-3 bg-green-50 border border-green-200 rounded-lg"> + <div className="mt-4 p-3 bg-green-50 dark:bg-green-950 border border-green-200 dark:border-green-800 rounded-lg">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/packages/create/page.tsx around lines 186 - 192, The selected index file card uses hardcoded light-mode classes (bg-green-50, border-green-200, text-green-600) which will look wrong in dark mode; update the JSX for the card around selectedIndexFile (the div with classes including bg-green-50 and border-green-200 and the FileText span using text-green-600) to use Tailwind dark variants (for example: bg-green-50 dark:bg-green-900, border-green-200 dark:border-green-700, text-green-600 dark:text-green-300) or replace with theme-aware utility tokens so the background, border and icon/text colors change appropriately in dark mode while preserving accessibility.
220-221: Hardcoded sidebar offset may cause layout issues.The fixed bottom bar uses
left-64which assumes a sidebar width of 16rem (256px). If the sidebar width changes or is collapsible, this will cause misalignment. Consider using a CSS custom property or layout context to maintain consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/packages/create/page.tsx around lines 220 - 221, The fixed bottom bar currently hardcodes the sidebar offset via the class "left-64" in the JSX element with className "fixed bottom-0 left-64 right-0 bg-background border-t p-4 z-50"; replace that hardcoded offset with a responsive approach such as using a CSS custom property (e.g. left: var(--sidebar-width)) or reading the sidebar width from a shared layout context/prop so the bottom bar aligns when the sidebar changes/collapses, and ensure the corresponding CSS/root style or context provider sets/updates that --sidebar-width value (or prop) used by the bottom bar.airborne_dashboard/components/filter-dropdown.tsx (3)
76-82: Missing cleanup whenonSearchchanges during debounce.The debounce effect includes
onSearchin the dependency array. IfonSearchis not memoized by the parent component, this could cause the timeout to be reset on every render, potentially delaying or preventing the search callback from firing.♻️ Use ref to avoid dependency on onSearch
+ const onSearchRef = useRef(onSearch); + onSearchRef.current = onSearch; + // Handle search with debounce useEffect(() => { const timer = setTimeout(() => { - onSearch?.(searchQuery); + onSearchRef.current?.(searchQuery); }, 300); return () => clearTimeout(timer); - }, [searchQuery, onSearch]); + }, [searchQuery]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/components/filter-dropdown.tsx` around lines 76 - 82, The debounce effect currently depends on onSearch which can change each render and reset the timer; change to store the callback in a ref (e.g., onSearchRef) and update that ref when onSearch changes, then have the debounce useEffect reference only searchQuery (not onSearch) so the timeout is not restarted on every parent render; keep the existing timer creation/clearTimeout cleanup in the effect that triggers on searchQuery, and invoke onSearchRef.current(searchQuery) inside the timeout to call the latest callback.
163-179: Missing keyboard navigation and ARIA attributes for options.The option items use
divelements withonClickbut lack keyboard navigation support (arrow keys, Enter to select) and proper ARIA attributes (role="option",aria-selected). This impacts accessibility for keyboard and screen reader users.♿ Add accessibility attributes
<div key={`${id}-${option.value}`} onClick={() => toggleValue(option.value)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + toggleValue(option.value); + } + }} + role="option" + aria-selected={isSelected} + tabIndex={0} className={cn(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/components/filter-dropdown.tsx` around lines 163 - 179, The option elements lack keyboard and ARIA support; update the option container (the element keyed by `${id}-${option.value}` used in the map that calls toggleValue) to include role="option", aria-selected={isSelected}, and tabIndex={0}, and add an onKeyDown handler that triggers toggleValue(option.value) on Enter or Space and implements ArrowUp/ArrowDown focus movement (e.g., focus previousSibling/nextSibling or delegate to a provided focus management callback) so keyboard users can navigate and select options; ensure Check and showCounts usage remains unchanged.
84-92: Potential for multipleonLoadMorecalls before loading state updates.The scroll handler checks
!isLoadingbefore callingonLoadMore, but if the parent doesn't immediately updateisLoadingtotrue, rapid scroll events could trigger multiple calls. Consider adding local state or a ref to track if a load is already in progress.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@airborne_dashboard/components/filter-dropdown.tsx` around lines 84 - 92, The scroll handler can call onLoadMore multiple times before the parent flips isLoading; add a local ref (e.g., loadingRef) to guard re-entrancy in handleScroll: check loadingRef.current || !hasMore || isLoading before calling, set loadingRef.current = true immediately before invoking onLoadMore, then clear it after the load finishes (if onLoadMore returns a Promise, await it and clear in finally; otherwise clear when you get a callback/prop update). Update handleScroll to reference loadingRef and ensure the ref is created via useRef(false) so rapid scroll events cannot trigger duplicate onLoadMore calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/files/page.tsx:
- Around line 311-391: The mapping fragment currently lacks the key (it's placed
on the inner TableRow), causing React's missing key warning; move the key to the
outer fragment by replacing the shorthand <>...</> with a keyed fragment like
<React.Fragment key={group.file_path}> (or import { Fragment } from 'react' and
use <Fragment key={group.file_path}>), then remove the key prop from the inner
TableRow (the elements referenced are the fragment around the group block and
the TableRow that currently has key={group.file_path}).
In `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/packages/create/page.tsx:
- Around line 34-38: The useEffect around access-checking (useEffect in
page.tsx) currently only depends on loadingAccess but references getOrgAccess,
getAppAccess, org, app, and hasAppAccess, risking stale closures; update the
dependency array to include getOrgAccess, getAppAccess, org, app, hasAppAccess
(or instead memoize those values/results before the effect) so the effect
re-runs whenever any of those change and still calls notFound() appropriately.
In `@airborne_dashboard/components/release/steps/ResourcesStep.tsx`:
- Around line 14-23: The mapping for selectedFiles in the useMemo uses `version
|| 1` which incorrectly treats version 0 as falsy; update the mapping that
builds each SelectedFile (inside the Array.from(selectedResources).map callback
where parseFileRef is used) to use nullish coalescing so a valid 0 is preserved
(i.e., use version ?? 1 instead of version || 1) so that SelectedFile.file_path,
version, and url are produced correctly.
In `@airborne_server/src/file/groups.rs`:
- Around line 63-66: The search term is being interpolated into the LIKE pattern
without escaping SQL wildcard characters, so escape '%' and '_' (and the escape
char itself) on the input before building the pattern used with file_path.ilike;
e.g., replace '\' with '\\', '%' with '\%', and '_' with '\_' (or another
consistent escape char), then build pattern = format!("%{}%", escaped_search)
and pass that to count_query.filter(file_path.ilike(pattern).escape('\\')) (or
use your ORM's ESCAPE mechanism) so the literal characters are matched; apply
the same escaping wherever search_term is used in other queries (e.g., any other
`.ilike(pattern)` calls).
---
Nitpick comments:
In `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/files/page.tsx:
- Around line 68-73: The current useSWR call that populates tagsData uses
apiFetch("/file/tags", ...) with a hardcoded query of { page: 1, count: 100 }
which can truncate results; update the query passed to apiFetch in the useSWR
block (the call inside useSWR that returns apiFetch<TagsResponse>("/file/tags",
...)) to request all tags (e.g., replace count: 100 with all: true or add all:
true depending on backend support) or implement a paginated/searchable approach
(e.g., switch to server-side search or incremental loading in the tag selector)
so the "/file/tags" endpoint returns the full set instead of only the first 100;
ensure the downstream handling of tagsData in the component accommodates the
full result shape.
In `@airborne_dashboard/app/dashboard/`[orgId]/[appId]/packages/create/page.tsx:
- Around line 186-192: The selected index file card uses hardcoded light-mode
classes (bg-green-50, border-green-200, text-green-600) which will look wrong in
dark mode; update the JSX for the card around selectedIndexFile (the div with
classes including bg-green-50 and border-green-200 and the FileText span using
text-green-600) to use Tailwind dark variants (for example: bg-green-50
dark:bg-green-900, border-green-200 dark:border-green-700, text-green-600
dark:text-green-300) or replace with theme-aware utility tokens so the
background, border and icon/text colors change appropriately in dark mode while
preserving accessibility.
- Around line 220-221: The fixed bottom bar currently hardcodes the sidebar
offset via the class "left-64" in the JSX element with className "fixed bottom-0
left-64 right-0 bg-background border-t p-4 z-50"; replace that hardcoded offset
with a responsive approach such as using a CSS custom property (e.g. left:
var(--sidebar-width)) or reading the sidebar width from a shared layout
context/prop so the bottom bar aligns when the sidebar changes/collapses, and
ensure the corresponding CSS/root style or context provider sets/updates that
--sidebar-width value (or prop) used by the bottom bar.
In `@airborne_dashboard/components/file-chooser.tsx`:
- Around line 307-323: The extra check "if (totalPages > 1)" around the final
PaginationItem is redundant because this branch already only runs when
totalPages > maxVisible (5); remove that outer conditional and its matching
closing brace so the PaginationItem block (using PaginationItem, PaginationLink,
currentPage and setCurrentPage) is unconditionally rendered in this branch,
keeping the inner onClick, isActive and key logic unchanged.
- Around line 117-129: The useEffect depends on tagsData?.data which can change
reference without content changes; update the dependency to a stable
representation (e.g., JSON.stringify(tagsData?.data) or a derived key) so the
effect only re-runs when actual tag content changes; modify the useEffect
dependency array (and keep logic inside useEffect using tagPage, tagsData?.data
and setAccumulatedTags) to use that stable dependency instead of raw
tagsData?.data to prevent unnecessary accumulations in the handlers that update
accumulated tags.
- Around line 412-419: The remove button inside the Badge lacks an accessible
label; update the button in the Badge where onClick calls
handleTagFilterToggle(tag) to include an aria-label describing its action (e.g.,
"Remove tag {tag}" or "Remove filter {tag}") so screen readers can convey
purpose; keep the existing onClick, className, and disabled props and ensure the
label incorporates the tag variable to be unique and descriptive for each badge.
In `@airborne_dashboard/components/filter-dropdown.tsx`:
- Around line 76-82: The debounce effect currently depends on onSearch which can
change each render and reset the timer; change to store the callback in a ref
(e.g., onSearchRef) and update that ref when onSearch changes, then have the
debounce useEffect reference only searchQuery (not onSearch) so the timeout is
not restarted on every parent render; keep the existing timer
creation/clearTimeout cleanup in the effect that triggers on searchQuery, and
invoke onSearchRef.current(searchQuery) inside the timeout to call the latest
callback.
- Around line 163-179: The option elements lack keyboard and ARIA support;
update the option container (the element keyed by `${id}-${option.value}` used
in the map that calls toggleValue) to include role="option",
aria-selected={isSelected}, and tabIndex={0}, and add an onKeyDown handler that
triggers toggleValue(option.value) on Enter or Space and implements
ArrowUp/ArrowDown focus movement (e.g., focus previousSibling/nextSibling or
delegate to a provided focus management callback) so keyboard users can navigate
and select options; ensure Check and showCounts usage remains unchanged.
- Around line 84-92: The scroll handler can call onLoadMore multiple times
before the parent flips isLoading; add a local ref (e.g., loadingRef) to guard
re-entrancy in handleScroll: check loadingRef.current || !hasMore || isLoading
before calling, set loadingRef.current = true immediately before invoking
onLoadMore, then clear it after the load finishes (if onLoadMore returns a
Promise, await it and clear in finally; otherwise clear when you get a
callback/prop update). Update handleScroll to reference loadingRef and ensure
the ref is created via useRef(false) so rapid scroll events cannot trigger
duplicate onLoadMore calls.
In `@airborne_dashboard/components/release/ReleaseBuilder.tsx`:
- Line 133: The fixed left-64 on the bottom bar in ReleaseBuilder.tsx will
misalign if the sidebar width changes; replace the hardcoded class on the <div>
(the element with className="fixed bottom-0 left-64 right-0 bg-background
border-t p-4 z-50") with a responsive offset driven by a CSS variable or layout
context: remove left-64 and apply a left style like left: var(--sidebar-width)
(or compute using a sidebarWidth prop from your layout context and set style={{
left: sidebarWidth }}), and ensure the sidebar component sets --sidebar-width
(e.g., --sidebar-width: 16rem or toggles for collapsed state) so the bar always
aligns with the sidebar across breakpoints and collapsed/expanded states.
In `@airborne_server/src/file.rs`:
- Around line 590-594: The mapping from query results redundantly filters out
None values even though the DB query already applies .filter(tag.is_not_null());
remove the unnecessary filter_map by directly mapping tag_results into
Vec<FileTagInfo> (use tag_results.into_iter().map(|(tag, count)| FileTagInfo {
tag, count }).collect()) to simplify code around the tag_results handling and
construction of FileTagInfo while keeping the same behavior; update references
in this block where FileTagInfo is produced to use the direct map and ensure
types still match.
- Around line 444-494: The count_query and data_query blocks duplicate the same
search/tag filter construction (using files, org_id, app_id, search_term ->
file_path.ilike/url.ilike, and tags_filter -> tag.eq/tag.eq_any) before
executing count() or load::<DbFile>(); refactor by extracting a helper that
accepts the base boxed query (e.g., files.into_boxed()), the optional
&search_term, and &tags_filter and returns the boxed query with filters applied,
then call that helper to produce the query for both total (count_query) and
file_list (data_query) to eliminate duplication while keeping Diesel types
consistent.
In `@airborne_server/src/file/groups.rs`:
- Around line 128-175: The loop over file_paths causes an N+1 query pattern: for
each fp you run two queries (the file_versions query and total_versions count)
which scales poorly; change to batch queries by (1) fetching all relevant
versions for the page in one query (filter
files.filter(org_id.eq(&organisation)).filter(app_id.eq(&application)).filter(file_path.eq_any(&file_paths)).order(file_path.asc()).order(version.desc()).select(DbFile::as_select()).load(&mut
conn)?) and grouping results in-memory by file_path to build the versions and
tags vectors for each FileGroupResponse, and (2) getting total_versions for all
file_paths in one grouped count query
(files.filter(...).filter(file_path.eq_any(&file_paths)).select((file_path,
diesel::dsl::count_star())).group_by(file_path).load(...)?) then map counts back
to each fp; update the code that currently uses file_versions, total_versions
and the for fp in file_paths loop to use these batched results.
In `@airborne_server/src/types.rs`:
- Around line 370-377: The deserializer de_u32_from_str currently treats an
empty query value (Some("")) as a parse error; update de_u32_from_str so that
when Option<String> is Some(s) and s.trim().is_empty() it returns Ok(None)
instead of attempting to parse, otherwise parse the string to u32 and propagate
parse errors via serde::de::Error::custom; refer to the function name
de_u32_from_str and the existing Option<String> handling to locate where to add
the empty-string check and branch.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
airborne_dashboard/app/dashboard/[orgId]/[appId]/files/page.tsxairborne_dashboard/app/dashboard/[orgId]/[appId]/packages/create/page.tsxairborne_dashboard/components/file-chooser.tsxairborne_dashboard/components/filter-dropdown.tsxairborne_dashboard/components/release/ReleaseBuilder.tsxairborne_dashboard/components/release/steps/ResourcesStep.tsxairborne_dashboard/types/files.tsairborne_server/src/file.rsairborne_server/src/file/groups.rsairborne_server/src/file/groups/types.rsairborne_server/src/file/types.rsairborne_server/src/types.rssmithy/models/file.smithy
yuvrajjsingh0
force-pushed
the
file-filtering-grouping
branch
from
a444df7 to
014642e
Compare
yuvrajjsingh0
force-pushed
the
file-filtering-grouping
branch
from
3d78f84 to
d0f090f
Compare
JamesGeorg
approved these changes
Mar 9, 2026
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
3 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
Release Notes
New Features
Improvements