feat: org projects pagination and add archived pagination #2312
Conversation
ConsoleProject ID: Sites (2)
Note You can use Avatars API to generate QR code for any text or URLs. |
WalkthroughThe load function now fetches active and archived projects separately and returns per-type pages, totals, and archived paging state (archivedPage, archivedOffset). The organization page uses these fields to bind lists, totals, and pagination; it passes archivedTotalOverall, archivedOffset, and limit into ArchiveProject. ArchiveProject now reads those props, shows archived totals, and renders PaginationWithLimit with pageParam="archivedPage" and removeOnFirstPage. Pagination, Limit, and PaginationWithLimit accept pageParam and removeOnFirstPage; paginator and paginationWithLimit forward extra props to their layout containers. Minor UI tweaks include name truncation and pagination spacing. Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
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 |
HarshMN2345
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
src/routes/(console)/organization-[organization]/+page.svelte (3)
107-109: Avoid redundant filtering when server already provides archived setUse the pre-filtered
data.archivedProjectsPagedirectly; fall back to filtering only when it’s absent.- $: projectsToArchive = (data.archivedProjectsPage ?? data.projects.projects).filter( - (project) => project.status !== 'active' - ); + $: projectsToArchive = data.archivedProjectsPage + ?? data.projects.projects.filter((project) => project.status !== 'active');
111-115: Prefer activeTotal fallback order and align all consumers to it
- Reorder fallbacks so we prefer
data.projects.totalbeforedata.organization.projects.length(the latter may include archived).- Also update any plan checks and props (e.g., Line 140 condition and Line 261 prop to CreateProjectCloud) to use
activeTotalOverallfor consistency.- $: activeProjects = (data.activeProjectsPage ?? data.projects.projects).filter( - (project) => project.status === 'active' - ); - $: activeTotalOverall = data?.activeTotalOverall ?? data?.organization?.projects?.length ?? data?.projects?.total ?? 0; + $: activeProjects = data.activeProjectsPage + ?? data.projects.projects.filter((project) => project.status === 'active'); + $: activeTotalOverall = + data?.activeTotalOverall + ?? data?.projects?.total + ?? data?.organization?.projects?.length + ?? 0;
1-1: Fix Prettier issuesCI flagged a Prettier formatting warning for this file. Please run:
pnpm prettier --write src/routes/(console)/organization-[organization]/+page.svelte.src/lib/components/archiveProject.svelte (3)
149-151: Name truncation tightened to 16 chars — confirm UX parityActive projects use 19/25 (small/regular). Dropping archived to 16 may create inconsistent card widths. Confirm with design or align to active logic.
-function formatName(name: string, limit: number = 16) { +function formatName(name: string, limit: number = 19) { // or make it responsive like the active list
163-168: Paginator config: hide UI affordances when not needed and avoid magic numbers
- With ≤6 items, you already hide the footer; consider also hiding page numbers.
- Extract
6into a local constant for clarity and future tuning.- <Paginator - items={projectsToArchive} - limit={6} - hidePages={false} - hideFooter={projectsToArchive.length <= 6}> + {#key projectsToArchive}<!-- reset page when data changes --> + <Paginator + items={projectsToArchive} + limit={ARCHIVE_PAGE_SIZE} + hidePages={projectsToArchive.length <= ARCHIVE_PAGE_SIZE} + hideFooter={projectsToArchive.length <= ARCHIVE_PAGE_SIZE}> {#snippet children(items)} <CardContainer disableEmpty={true} total={projectsToArchive.length}> {#each items as project} ... </CardContainer> {/snippet} </Paginator>Add near the script top:
const ARCHIVE_PAGE_SIZE = 6;Also applies to: 169-171, 264-267
1-1: Fix Prettier issuesCI flagged a Prettier formatting warning for this file. Please run:
pnpm prettier --write src/lib/components/archiveProject.svelte.src/routes/(console)/organization-[organization]/+page.ts (4)
56-56: Set region before deriving slices (minor)Move region normalization before computing
allActiveProjects/allArchivedProjectsto keep derived arrays fully normalized. Low impact.- // set `default` if no region! - for (const project of allProjects) { + // set `default` if no region! + for (const project of allProjects) { project.region ??= 'default'; } - const allActiveProjects = allProjects.filter((p) => p.status === 'active'); - const allArchivedProjects = allProjects.filter((p) => p.status !== 'active'); + const allActiveProjects = allProjects.filter((p) => p.status === 'active'); + const allArchivedProjects = allProjects.filter((p) => p.status !== 'active');
31-48: Reduce N+1 API calls: decouple aggregation page size from UI page sizeLooping with
Query.limit(limit)ties server fetch size to UI page size. Use a larger server-side chunk (e.g., 100) within API limits to cut requests.- const limit = getLimit(url, route, CARD_LIMIT); + const limit = getLimit(url, route, CARD_LIMIT); + const AGG_LIMIT = Math.min(100, limit); // tune to SDK/API max page size ... - Query.limit(limit), + Query.limit(AGG_LIMIT),
1-1: Fix Prettier issuesCI flagged a Prettier formatting warning for this file. Please run:
pnpm prettier --write src/routes/(console)/organization-[organization]/+page.ts.
37-42: Use explicit sort field or document defaultEmpty string in
Query.orderDesc('')is used across the codebase to invoke the SDK’s default descending order (typically by creation date). For clarity, replace''with an explicit field (e.g.'$createdAt') or add a comment noting that an empty string triggers default sorting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/lib/components/archiveProject.svelte(4 hunks)src/routes/(console)/organization-[organization]/+page.svelte(3 hunks)src/routes/(console)/organization-[organization]/+page.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/(console)/organization-[organization]/+page.ts (2)
src/routes/(console)/organization-[organization]/store.ts (1)
projects(17-17)src/lib/stores/sdk.ts (1)
sdk(142-165)
🪛 GitHub Actions: Tests
src/routes/(console)/organization-[organization]/+page.svelte
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix.
src/routes/(console)/organization-[organization]/+page.ts
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix.
src/lib/components/archiveProject.svelte
[warning] 1-1: Prettier formatting issue detected. Run 'prettier --write' to fix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
🔇 Additional comments (5)
src/routes/(console)/organization-[organization]/+page.svelte (2)
165-165: LGTM: CardContainer now bound to active totalUsing
activeTotalOverallensures paging/total reflects active projects only.
250-250: LGTM: Pagination total matches active total
PaginationWithLimit.total={activeTotalOverall}is correct and consistent with the active list.src/lib/components/archiveProject.svelte (2)
3-3: LGTM: Paginator importImporting
Paginatorfrom$lib/componentsis consistent with the component library usage.
258-261: Safe access on region already handledUsing
region?.nameavoids runtime errors when regions aren’t loaded yet. Good.src/routes/(console)/organization-[organization]/+page.ts (1)
50-54: LGTM: Server-side split of active vs archived and per-page sliceThis matches the UI’s new bindings and avoids filtering everything on the client.
| let allProjects: typeof projects.projects = []; | ||
| let fetchedCount = 0; | ||
| const total = projects.total; | ||
|
|
||
| while (fetchedCount < total) { | ||
| const next = await sdk.forConsole.projects.list({ | ||
| queries: [ | ||
| Query.offset(fetchedCount), | ||
| Query.equal('teamId', params.organization), | ||
| Query.limit(limit), | ||
| Query.orderDesc('') | ||
| ], | ||
| search: search || undefined | ||
| }); | ||
| allProjects = allProjects.concat(next.projects); | ||
| fetchedCount += next.projects.length; | ||
| if (next.projects.length === 0) break; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Seed aggregation with the first response and avoid redundant page-0 fetch
You drop the first call’s results and refetch from offset 0, doubling cost and latency. Seed allProjects with the initial page and continue from there.
- let allProjects: typeof projects.projects = [];
- let fetchedCount = 0;
+ let allProjects: typeof projects.projects = projects.projects.slice();
+ let fetchedCount = allProjects.length;
const total = projects.total;
- while (fetchedCount < total) {
+ while (fetchedCount < total) {
const next = await sdk.forConsole.projects.list({
queries: [
- Query.offset(fetchedCount),
+ Query.offset(fetchedCount),
Query.equal('teamId', params.organization),
- Query.limit(limit),
+ Query.limit(limit),
Query.orderDesc('')
],
search: search || undefined
});
allProjects = allProjects.concat(next.projects);
fetchedCount += next.projects.length;
if (next.projects.length === 0) break;
}📝 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.
| let allProjects: typeof projects.projects = []; | |
| let fetchedCount = 0; | |
| const total = projects.total; | |
| while (fetchedCount < total) { | |
| const next = await sdk.forConsole.projects.list({ | |
| queries: [ | |
| Query.offset(fetchedCount), | |
| Query.equal('teamId', params.organization), | |
| Query.limit(limit), | |
| Query.orderDesc('') | |
| ], | |
| search: search || undefined | |
| }); | |
| allProjects = allProjects.concat(next.projects); | |
| fetchedCount += next.projects.length; | |
| if (next.projects.length === 0) break; | |
| } | |
| // Seed with the initial page of projects instead of starting empty | |
| let allProjects: typeof projects.projects = projects.projects.slice(); | |
| let fetchedCount = allProjects.length; | |
| const total = projects.total; | |
| while (fetchedCount < total) { | |
| const next = await sdk.forConsole.projects.list({ | |
| queries: [ | |
| Query.offset(fetchedCount), | |
| Query.equal('teamId', params.organization), | |
| Query.limit(limit), | |
| Query.orderDesc('') | |
| ], | |
| search: search || undefined | |
| }); | |
| allProjects = allProjects.concat(next.projects); | |
| fetchedCount += next.projects.length; | |
| if (next.projects.length === 0) break; | |
| } |
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/+page.ts around lines 31 to
48, the code currently performs an initial projects fetch but then starts the
loop that refetches from offset 0, doubling cost and latency; instead, perform
the first sdk.forConsole.projects.list call once, initialize allProjects with
that response and set fetchedCount to that response.projects.length, then enter
a loop that requests subsequent pages starting at the current fetchedCount
(using Query.offset(fetchedCount)) until fetchedCount >= total or a page returns
zero items; this removes the redundant page-0 fetch while preserving the same
paging logic.
| projects: { ...projects, projects: allProjects, total: allActiveProjects.length }, | ||
| activeProjectsPage: activeProjectsForPage, | ||
| archivedProjectsPage: allArchivedProjects, | ||
| activeTotalOverall: allActiveProjects.length, | ||
| search |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Don’t mutate the projects contract; return aggregates separately
Overwriting projects.projects with all items and projects.total with “active total” breaks consumers expecting API semantics. Keep projects intact and expose new top-level fields for UI.
- projects: { ...projects, projects: allProjects, total: allActiveProjects.length },
- activeProjectsPage: activeProjectsForPage,
- archivedProjectsPage: allArchivedProjects,
- activeTotalOverall: allActiveProjects.length,
+ projects, // preserve API contract for stores and other consumers
+ activeProjectsPage: activeProjectsForPage,
+ archivedProjectsPage: allArchivedProjects,
+ activeTotalOverall: allActiveProjects.length,
+ // Optionally expose allProjects if needed elsewhere:
+ // allProjects,🏁 Script executed:
#!/bin/bash
# Audit for consumers that assume `projects.total` is "all results" (not just active).
rg -nP --type=ts --type=svelte -C2 '\bprojects\.total\b'Length of output: 89
🏁 Script executed:
#!/bin/bash
# Audit for consumers that assume `projects.total` remains the full count
rg -nP -g '*.ts' -g '*.svelte' -C2 '\bprojects\.total\b'Length of output: 4397
Preserve projects API contract and expose aggregates separately
Overriding projects.projects and projects.total with active-only values breaks UI components that rely on the full project count (plan checks, creation modals, settings pages). Undo the mutation and introduce new top-level fields for the aggregates, for example:
- projects: { ...projects, projects: allProjects, total: allActiveProjects.length },
+ projects, // preserve API contract
activeProjectsPage: activeProjectsForPage,
archivedProjectsPage: allArchivedProjects,
+ activeTotalOverall: allActiveProjects.length,
+ allProjects, // full list if neededCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/+page.ts around lines 63 to
67, the code is mutating the existing projects object by replacing
projects.projects and projects.total with active-only values which breaks
consumers expecting the full projects API; revert that mutation so the original
projects object remains intact and add new top-level fields (e.g.,
activeProjectsPage and activeProjectsTotal or activeProjectsAggregate) to expose
active-only lists and counts, update any callers that need the active-only
values to use these new fields, and ensure the returned payload preserves the
original projects structure while exposing the aggregates separately.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/routes/(console)/organization-[organization]/+page.ts (2)
31-48: Avoid double-fetching and reduce total requests while aggregating.
You re-query from offset 0 after already fetching one page; on page 1 this duplicates page-0, and on later pages you still pay an extra request. Seed aggregation or restructure to a single aggregation pass.Example approach (parallelized, larger page size):
- let allProjects: typeof projects.projects = []; - let fetchedCount = 0; - const total = projects.total; - - while (fetchedCount < total) { - const next = await sdk.forConsole.projects.list({ - queries: [ - Query.offset(fetchedCount), - Query.equal('teamId', params.organization), - Query.limit(limit), - Query.orderDesc('') - ], - search: search || undefined - }); - allProjects = allProjects.concat(next.projects); - fetchedCount += next.projects.length; - if (next.projects.length === 0) break; - } + const total = projects.total; + const pageSize = Math.max(limit, 100); // fewer round-trips + const fetches: Promise<typeof projects>[] = []; + for (let start = 0; start < total; start += pageSize) { + fetches.push( + sdk.forConsole.projects.list({ + queries: [ + Query.offset(start), + Query.equal('teamId', params.organization), + Query.limit(pageSize), + Query.orderDesc('') + ], + search: search || undefined + }) + ); + } + const pages = await Promise.all(fetches); + let allProjects: typeof projects.projects = pages.flatMap((p) => p.projects);
63-66: Preserve theprojectsAPI contract; expose aggregates separately.
Overwritingprojects.projectsandprojects.totalbreaks consumers that expect server semantics.- projects: { ...projects, projects: allProjects, total: allActiveProjects.length }, - activeProjectsPage: activeProjectsForPage, - archivedProjectsPage: allArchivedProjects, - activeTotalOverall: allActiveProjects.length, + projects, // keep original paging contract + allProjects, // optional: full list for local consumers + activeProjectsPage: activeProjectsForPage, + archivedProjectsPage: allArchivedProjects, + activeTotalOverall: allActiveProjects.length,
🧹 Nitpick comments (4)
src/lib/components/archiveProject.svelte (2)
171-171: Key your each blocks for stable DOM reconciliation.
Use project id and platform name as keys.- {#each items as project} + {#each items as project (project.$id)} ... - {#each platforms.slice(0, 2) as platform} + {#each platforms.slice(0, 2) as platform (platform.name)}Also applies to: 250-258
166-167: Nit:hidePages={false}is redundant ifhideFooterhides controls.
You can removehidePagesunless it toggles other UI.- hidePages={false} hideFooter={projectsToArchive.length <= 6}>src/routes/(console)/organization-[organization]/+page.ts (2)
56-58: Avoid mutating SDK objects in-place when normalizing region.
Safer to return normalized copies to prevent accidental shared-state mutations.- for (const project of allProjects) { - project.region ??= 'default'; - } + allProjects = allProjects.map((p) => ({ ...p, region: p.region ?? 'default' }));
31-48: Optional: cap concurrency to balance latency and load.
If you parallelize page fetches, use a small pool (e.g., 4–6) rather thanPromise.allon hundreds of pages.If helpful, I can provide a pooled fetch helper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/lib/components/archiveProject.svelte(3 hunks)src/routes/(console)/organization-[organization]/+page.svelte(3 hunks)src/routes/(console)/organization-[organization]/+page.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/organization-[organization]/+page.svelte
🔇 Additional comments (3)
src/lib/components/archiveProject.svelte (2)
3-3: Good addition: local pagination wrapper imported.
Importing Paginator here aligns with the new archived pagination flow.
149-151: Name truncation tightened (19 → 16): verify UI doesn’t regress.
Please sanity-check long archived names across viewports for truncation/tooltip behavior.src/routes/(console)/organization-[organization]/+page.ts (1)
53-54: Confirm page semantics when slicing only active projects.
offsetis derived from all projects, but you sliceallActiveProjects. If archived/active are interleaved, active pages may shift. Ensure the route’spageis intended to page active-only results.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/lib/components/archiveProject.svelte (1)
171-176: Guard against undefined platforms to avoid runtime errors.
This still calls.mapon a possibly undefinedproject.platforms. Use a safe fallback.- {@const platforms = filterPlatforms( - project.platforms.map((platform) => - getPlatformInfo(platform.type) - ) - )} + {@const platforms = filterPlatforms( + (project.platforms ?? []).map((platform) => + getPlatformInfo(platform.type) + ) + )}
🧹 Nitpick comments (3)
src/lib/components/archiveProject.svelte (3)
149-151: Truncation default lowered to 16 — confirm UX consistency.
Verify this aligns with design across breakpoints; otherwise consider keeping the previous limit or making it a shared constant.
163-170: Paginator wiring and CardContainer total — confirm contract.
- Ensure Paginator supports the
{#snippet children(items)}API withitemsas the paged slice and thathideFooterbehaves as intended.- If CardContainer’s
totalis meant to reflect items on the current page (not overall), switch toitems.lengthto avoid UI/skeleton mismatches.Proposed change if per-page is expected:
- <CardContainer disableEmpty={true} total={projectsToArchive.length}> + <CardContainer disableEmpty={true} total={items.length}>
170-170: Key the each block for stable DOM when paginating.
Helps avoid unnecessary re-renders when page content changes.- {#each items as project} + {#each items as project (project.$id)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/lib/components/archiveProject.svelte(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (3)
src/lib/components/archiveProject.svelte (3)
3-3: Importing Paginator for client-side pagination looks good.
260-265: Off-by-one fix for “+N” badge — LGTM.
Condition> 2correctly shows “+1” when there are 3 platforms.
268-271: Region rendering gated safely — LGTM.
The$regionsStore?.regionscheck prevents.findon undefined and the UI handles missing names.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/routes/(console)/organization-[organization]/+page.ts (1)
73-77: Don’t repurposeprojectsfor active-only; preserve API contractOverwriting
projects.totalwith the active count breaks consumers expecting the full “for this query” semantics (seen elsewhere in the app). Expose per-type fields separately as you already do.- projects: { - ...activeProjects, - projects: activeProjects.projects, - total: activeTotal.total - }, + // Preserve original list response for the active query without altering semantics + projects: activeProjects, activeProjectsPage: activeProjects.projects, archivedProjectsPage: archivedProjects.projects, activeTotalOverall: activeTotal.total, archivedTotalOverall: archivedTotal.total,src/lib/components/archiveProject.svelte (1)
218-221: Guard against undefined platforms array
.mapon undefined will throw for projects missing platforms.- {@const platforms = filterPlatforms( - project.platforms.map((platform) => getPlatformInfo(platform.type)) - )} + {@const platforms = filterPlatforms( + (project.platforms ?? []).map((platform) => getPlatformInfo(platform.type)) + )}
🧹 Nitpick comments (3)
src/lib/components/archivedPagination.svelte (1)
7-7: Guard currentPage against limit=0/undefinedAvoid NaN/Infinity and clamp to page >= 1.
- const currentPage = $derived(Math.floor(offset / limit + 1)); + const currentPage = $derived(Math.max(1, Math.floor(offset / Math.max(1, Number(limit) || 1)) + 1));src/lib/components/archivedPaginationWithLimit.svelte (1)
30-30: Drop unused event listener
on:pageisn’t handled; remove to avoid confusion.- <ArchivedPagination on:page {limit} {offset} sum={total} /> + <ArchivedPagination {limit} {offset} sum={total} />src/routes/(console)/organization-[organization]/+page.ts (1)
21-23: Clamp archivedPage to ≥ 1Protect against invalid/negative/zero values.
- const archivedPage = parseInt(url.searchParams.get('archivedPage') || '1'); + const archivedPage = Math.max(1, parseInt(url.searchParams.get('archivedPage') || '1', 10) || 1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/lib/components/archiveProject.svelte(6 hunks)src/lib/components/archivedLimit.svelte(1 hunks)src/lib/components/archivedPagination.svelte(1 hunks)src/lib/components/archivedPaginationWithLimit.svelte(1 hunks)src/routes/(console)/organization-[organization]/+page.svelte(3 hunks)src/routes/(console)/organization-[organization]/+page.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/(console)/organization-[organization]/+page.ts (2)
src/lib/helpers/load.ts (1)
pageToOffset(4-6)src/lib/stores/sdk.ts (1)
sdk(142-165)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: e2e
| async function limitChange() { | ||
| const url = new URL(pageStore.url); | ||
| const previousLimit = Number(url.searchParams.get('limit')); | ||
| url.searchParams.set('limit', limit.toString()); | ||
| preferences.setLimit(limit); | ||
|
|
||
| // Handle archived page pagination | ||
| if (url.searchParams.has('archivedPage')) { | ||
| const page = Number(url.searchParams.get('archivedPage')); | ||
| const newPage = Math.floor(((page - 1) * previousLimit) / limit); | ||
| if (newPage === 1) { | ||
| url.searchParams.delete('archivedPage'); | ||
| } else { | ||
| url.searchParams.set('archivedPage', newPage.toString()); | ||
| } | ||
| } | ||
|
|
||
| await goto(url.toString()); | ||
| } |
There was a problem hiding this comment.
Fix off-by-one and null handling when recalculating archivedPage on limit change
- Off-by-one: should be floor(startIndex/newLimit) + 1.
- When
limitquery is missing,Number(null)becomes 0 and can produce page 0. - Clamp to page >= 1 and defend against zero/invalid limits.
- const previousLimit = Number(url.searchParams.get('limit'));
+ const previousLimit = Number(url.searchParams.get('limit') ?? limit) || limit;
@@
- if (url.searchParams.has('archivedPage')) {
- const page = Number(url.searchParams.get('archivedPage'));
- const newPage = Math.floor(((page - 1) * previousLimit) / limit);
- if (newPage === 1) {
+ if (url.searchParams.has('archivedPage')) {
+ const page = Math.max(1, Number(url.searchParams.get('archivedPage')) || 1);
+ const startIndex = (page - 1) * Math.max(1, previousLimit);
+ const newPage = Math.max(1, Math.floor(startIndex / Math.max(1, limit)) + 1);
+ if (newPage <= 1) {
url.searchParams.delete('archivedPage');
} else {
- url.searchParams.set('archivedPage', newPage.toString());
+ url.searchParams.set('archivedPage', String(newPage));
}
}📝 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.
| async function limitChange() { | |
| const url = new URL(pageStore.url); | |
| const previousLimit = Number(url.searchParams.get('limit')); | |
| url.searchParams.set('limit', limit.toString()); | |
| preferences.setLimit(limit); | |
| // Handle archived page pagination | |
| if (url.searchParams.has('archivedPage')) { | |
| const page = Number(url.searchParams.get('archivedPage')); | |
| const newPage = Math.floor(((page - 1) * previousLimit) / limit); | |
| if (newPage === 1) { | |
| url.searchParams.delete('archivedPage'); | |
| } else { | |
| url.searchParams.set('archivedPage', newPage.toString()); | |
| } | |
| } | |
| await goto(url.toString()); | |
| } | |
| async function limitChange() { | |
| const url = new URL(pageStore.url); | |
| const previousLimit = Number(url.searchParams.get('limit') ?? limit) || limit; | |
| url.searchParams.set('limit', limit.toString()); | |
| preferences.setLimit(limit); | |
| // Handle archived page pagination | |
| if (url.searchParams.has('archivedPage')) { | |
| const page = Math.max(1, Number(url.searchParams.get('archivedPage')) || 1); | |
| const startIndex = (page - 1) * Math.max(1, previousLimit); | |
| const newPage = Math.max(1, Math.floor(startIndex / Math.max(1, limit)) + 1); | |
| if (newPage <= 1) { | |
| url.searchParams.delete('archivedPage'); | |
| } else { | |
| url.searchParams.set('archivedPage', String(newPage)); | |
| } | |
| } | |
| await goto(url.toString()); | |
| } |
| $: projectsToArchive = (data.archivedProjectsPage ?? data.projects.projects).filter( | ||
| (project) => (isCloud ? project.status === 'archived' : project.status !== 'active') // fallback for non-cloud | ||
| ); | ||
|
|
||
| $: activeProjects = data.projects.projects.filter((project) => project.status !== 'archived'); | ||
| $: activeProjects = (data.activeProjectsPage ?? data.projects.projects).filter( | ||
| (project) => project.status === 'active' | ||
| ); |
There was a problem hiding this comment.
Correct archived/active filters for non-cloud (null means active)
Per prior guidance, only status === 'archived' is archived; null is active. Current fallback misclassifies nulls.
- $: projectsToArchive = (data.archivedProjectsPage ?? data.projects.projects).filter(
- (project) => (isCloud ? project.status === 'archived' : project.status !== 'active') // fallback for non-cloud
- );
+ $: projectsToArchive = (data.archivedProjectsPage ?? data.projects.projects).filter(
+ (project) => project.status === 'archived'
+ );
@@
- $: activeProjects = (data.activeProjectsPage ?? data.projects.projects).filter(
- (project) => project.status === 'active'
- );
+ $: activeProjects = (data.activeProjectsPage ?? data.projects.projects).filter(
+ (project) => project.status !== 'archived'
+ );📝 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.
| $: projectsToArchive = (data.archivedProjectsPage ?? data.projects.projects).filter( | |
| (project) => (isCloud ? project.status === 'archived' : project.status !== 'active') // fallback for non-cloud | |
| ); | |
| $: activeProjects = data.projects.projects.filter((project) => project.status !== 'archived'); | |
| $: activeProjects = (data.activeProjectsPage ?? data.projects.projects).filter( | |
| (project) => project.status === 'active' | |
| ); | |
| $: projectsToArchive = (data.archivedProjectsPage ?? data.projects.projects).filter( | |
| (project) => project.status === 'archived' | |
| ); | |
| $: activeProjects = (data.activeProjectsPage ?? data.projects.projects).filter( | |
| (project) => project.status !== 'archived' | |
| ); |
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/+page.svelte around lines
112–118, the fallback for non-cloud currently treats null as non-active causing
misclassification; change the archived filter to only include project.status ===
'archived' (no non-cloud fallback that treats null as archived), and change the
activeProjects filter to include project.status === 'active' or, for non-cloud,
treat null as active (i.e. project.status === 'active' || project.status == null
when isCloud is false).
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/routes/(console)/organization-[organization]/+page.ts (1)
10-12: Usethrow redirect(...), notreturn redirect(...).SvelteKit expects a thrown redirect. Returning it won’t redirect.
- if (!scopes.includes('projects.read') && scopes.includes('billing.read')) { - return redirect(301, `/console/organization-${params.organization}/billing`); - } + if (!scopes.includes('projects.read') && scopes.includes('billing.read')) { + throw redirect(301, `/console/organization-${params.organization}/billing`); + }src/lib/components/archiveProject.svelte (1)
219-221: Guard against undefined platforms to prevent runtime crash
project.platformscan be undefined;.mapwould throw.- {@const platforms = filterPlatforms( - project.platforms.map((platform) => getPlatformInfo(platform.type)) - )} + {@const platforms = filterPlatforms( + (project.platforms ?? []).map((platform) => + getPlatformInfo(platform.type) + ) + )}
♻️ Duplicate comments (2)
src/routes/(console)/organization-[organization]/+page.ts (1)
72-76: Preserveprojectscontract; don’t override with active-only total.Overriding
projects.totaldiverges from the SDK response shape and has broken consumers before. Keepprojectsas returned by the SDK and expose active/archived aggregates separately.- projects: { - ...activeProjects, - projects: activeProjects.projects, - total: activeTotal.total - }, + projects: activeProjects, // preserve SDK response as-isIf callers need active total, use the already exposed
activeTotalOverall.src/routes/(console)/organization-[organization]/+page.svelte (1)
116-118: Active filter should include null/undefined (treat anything not archived as active)Self-hosted often returns no
statusfor active items. Safer filter:- $: activeProjects = (data.activeProjectsPage ?? data.projects.projects).filter( - (project) => project.status === 'active' - ); + $: activeProjects = (data.activeProjectsPage ?? data.projects.projects).filter( + (project) => project.status !== 'archived' + );
🧹 Nitpick comments (5)
src/lib/components/pagination.svelte (1)
17-24: Optional: sanitize target page input.Defensive clamp avoids accidental 0/negative pages if upstream emits bad values.
- if (page === 1) { + const p = Math.max(1, Math.floor(Number(page))); + if (p === 1) { if (removeOnFirstPage) { url.searchParams.delete(pageParam); } else { - url.searchParams.set(pageParam, '1'); + url.searchParams.set(pageParam, '1'); } } else { - url.searchParams.set(pageParam, page.toString()); + url.searchParams.set(pageParam, p.toString()); }src/routes/(console)/organization-[organization]/+page.ts (2)
24-59: Reduce payload for total-only queries.Add a small
limitto the total-only requests to avoid fetching full pages unnecessarily.sdk.forConsole.projects.list({ queries: [ Query.equal('teamId', params.organization), - Query.or([Query.equal('status', 'active'), Query.isNull('status')]) + Query.or([Query.equal('status', 'active'), Query.isNull('status')]), + Query.limit(1) ], search: search || undefined }), sdk.forConsole.projects.list({ queries: [ Query.equal('teamId', params.organization), - Query.equal('status', 'archived') + Query.equal('status', 'archived'), + Query.limit(1) ], search: search || undefined })If Appwrite totals are independent of
limit,Query.limit(1)will still return correcttotalwhile reducing payload.
61-67: Mutation is fine, but consider immutability.Setting
project.regionin-place is convenient but can surprise consumers if they rely on raw SDK objects. Consider mapping to new objects if immutability is preferred project-wide.src/lib/components/archiveProject.svelte (2)
203-203: Name truncation defaultDefault 16 is fine; consider centralizing this constant to avoid drift with other views.
404-406: Pagination spacing styleUsing
:globalis acceptable here; ifPaginationWithLimitforwards classes, you could also keep it scoped by styling a local wrapper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/lib/components/archiveProject.svelte(6 hunks)src/lib/components/limit.svelte(2 hunks)src/lib/components/pagination.svelte(1 hunks)src/lib/components/paginationWithLimit.svelte(1 hunks)src/lib/components/paginator.svelte(2 hunks)src/routes/(console)/organization-[organization]/+page.svelte(3 hunks)src/routes/(console)/organization-[organization]/+page.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/(console)/organization-[organization]/+page.ts (2)
src/lib/helpers/load.ts (1)
pageToOffset(4-6)src/lib/stores/sdk.ts (1)
sdk(142-165)
🪛 ESLint
src/lib/components/paginationWithLimit.svelte
[error] 23-23: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/lib/components/paginator.svelte
[error] 30-30: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (12)
src/lib/components/limit.svelte (1)
11-12: Good API extension with sensible defaults.
pageParamandremoveOnFirstPageprops look correct and align with the new archived pagination flow.src/lib/components/pagination.svelte (1)
9-10: Prop plumbing for pageParam/removeOnFirstPage looks good.Matches Limit/PaginationWithLimit and keeps default behavior intact.
src/lib/components/paginator.svelte (2)
16-18: Rest-props pass-through: OK, but ensure no accidental prop shadowing.Spreading
...restPropsafter{gap}allows consumers to overridegapif needed; just confirming this is intentional.
38-38: Prop forwarding is correct.
{...restProps}onLayout.Stackunlocks styling/attrs without changing behavior.src/lib/components/paginationWithLimit.svelte (2)
11-14: New props are wired correctly through to children.
useCreateLink,pageParam, andremoveOnFirstPagepropagation is consistent with Pagination and Limit.
31-44: Wrapper wiring LGTM.
sum={total}matches Pagination API.{pageParam}and{removeOnFirstPage}are correctly passed to both Limit and Pagination.src/routes/(console)/organization-[organization]/+page.svelte (3)
112-114: Archived filter fix looks correctFiltering strictly by
project.status === 'archived'resolves the earlier misclassification.
120-124: Verify active totals propagate consistently to list and paginatorUsing
activeTotalOverallfor both CardContainer and Pagination is correct. Please confirm server returns this as the full active count (not page size), so pagination math matches limit/offset.Also applies to: 176-176, 261-261
267-270: Wiring archived pagination props to ArchiveProjectProps passed match the component’s new API.
src/lib/components/archiveProject.svelte (3)
52-55: Prop surface extension is coherentNew props (
archivedTotalOverall,archivedOffset,limit) are clearly typed and destructured.Also applies to: 57-64
2-2: Archived totals + pagination wiringImport + usage of
PaginationWithLimitand switching badges/totals toarchivedTotalOveralllook good. Please confirm:
PaginationWithLimitacceptspageParamandremoveOnFirstPage.- It forwards
$$restPropssoclass="pagination-container"applies to its root.- The loader reads
archivedPageand computesarchivedOffsetaccordingly.Also applies to: 210-210, 217-217, 323-330
306-306: Off-by-one fix for “+N” badgeCondition
> 2is correct.
| if (url.searchParams.has(pageParam)) { | ||
| const page = Number(url.searchParams.get(pageParam)); | ||
| const newPage = Math.floor(((page - 1) * previousLimit) / limit); | ||
| url.searchParams.set('page', newPage.toString()); | ||
| const safePage = Math.max(1, Number.isFinite(newPage) ? newPage : 1); | ||
| if (removeOnFirstPage && safePage === 1) { | ||
| url.searchParams.delete(pageParam); | ||
| } else { | ||
| url.searchParams.set(pageParam, safePage.toString()); | ||
| } |
There was a problem hiding this comment.
Fix off-by-one when recomputing page after limit change.
newPage needs a +1 to convert from 0-based index to 1-based page. Without it, unchanged limits incorrectly drop page N to page N-1, and scaling limits can jump to the wrong page.
Apply:
- if (url.searchParams.has(pageParam)) {
- const page = Number(url.searchParams.get(pageParam));
- const newPage = Math.floor(((page - 1) * previousLimit) / limit);
- const safePage = Math.max(1, Number.isFinite(newPage) ? newPage : 1);
+ if (url.searchParams.has(pageParam)) {
+ const page = Number(url.searchParams.get(pageParam));
+ const prev = Number.isFinite(previousLimit) && previousLimit > 0 ? previousLimit : limit;
+ const newPage = Math.floor(((page - 1) * prev) / limit) + 1;
+ const safePage = Math.max(1, Number.isFinite(newPage) ? newPage : 1);
if (removeOnFirstPage && safePage === 1) {
url.searchParams.delete(pageParam);
} else {
url.searchParams.set(pageParam, safePage.toString());
}
}📝 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.
| if (url.searchParams.has(pageParam)) { | |
| const page = Number(url.searchParams.get(pageParam)); | |
| const newPage = Math.floor(((page - 1) * previousLimit) / limit); | |
| url.searchParams.set('page', newPage.toString()); | |
| const safePage = Math.max(1, Number.isFinite(newPage) ? newPage : 1); | |
| if (removeOnFirstPage && safePage === 1) { | |
| url.searchParams.delete(pageParam); | |
| } else { | |
| url.searchParams.set(pageParam, safePage.toString()); | |
| } | |
| if (url.searchParams.has(pageParam)) { | |
| const page = Number(url.searchParams.get(pageParam)); | |
| const prev = Number.isFinite(previousLimit) && previousLimit > 0 ? previousLimit : limit; | |
| const newPage = Math.floor(((page - 1) * prev) / limit) + 1; | |
| const safePage = Math.max(1, Number.isFinite(newPage) ? newPage : 1); | |
| if (removeOnFirstPage && safePage === 1) { | |
| url.searchParams.delete(pageParam); | |
| } else { | |
| url.searchParams.set(pageParam, safePage.toString()); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/lib/components/limit.svelte around lines 28 to 36, the recomputation of
newPage when adjusting limits uses a 0-based index but the app uses 1-based page
numbers; add +1 to the newPage calculation so it converts back to 1-based pages
(i.e., compute newPage = Math.floor(((page - 1) * previousLimit) / limit) + 1),
then keep the existing safePage logic and pageParam removal/setting unchanged.
| const archivedPage = parseInt(url.searchParams.get('archivedPage') || '1'); | ||
| const archivedOffset = pageToOffset(archivedPage, limit); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clamp and sanitize archivedPage to avoid negative/NaN offsets.
Negative pages produce invalid negative offsets.
- const archivedPage = parseInt(url.searchParams.get('archivedPage') || '1');
+ const archivedPageRaw = parseInt(url.searchParams.get('archivedPage') || '1', 10);
+ const archivedPage = Number.isFinite(archivedPageRaw) && archivedPageRaw > 0 ? archivedPageRaw : 1;📝 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.
| const archivedPage = parseInt(url.searchParams.get('archivedPage') || '1'); | |
| const archivedOffset = pageToOffset(archivedPage, limit); | |
| const archivedPageRaw = parseInt(url.searchParams.get('archivedPage') || '1', 10); | |
| const archivedPage = Number.isFinite(archivedPageRaw) && archivedPageRaw > 0 ? archivedPageRaw : 1; | |
| const archivedOffset = pageToOffset(archivedPage, limit); |
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/+page.ts around lines 21 to
23, the parsed archivedPage can be NaN or negative which leads to invalid
negative offsets; sanitize the value after parseInt by checking isNaN and
clamping to a minimum of 1 (e.g., if parsed is NaN or <1 set archivedPage = 1)
or alternatively compute archivedOffset as Math.max(0,
pageToOffset(sanitizedPage, limit)); update subsequent uses to rely on this
sanitized archivedPage/archivedOffset.
| const [activeProjects, archivedProjects, activeTotal, archivedTotal] = await Promise.all([ | ||
| sdk.forConsole.projects.list({ | ||
| queries: [ | ||
| Query.offset(offset), | ||
| Query.equal('teamId', params.organization), | ||
| Query.or([Query.equal('status', 'active'), Query.isNull('status')]), | ||
| Query.limit(limit), | ||
| Query.orderDesc('') | ||
| ], | ||
| search: search || undefined | ||
| }), | ||
| sdk.forConsole.projects.list({ | ||
| queries: [ | ||
| Query.offset(archivedOffset), | ||
| Query.equal('teamId', params.organization), | ||
| Query.equal('status', 'archived'), | ||
| Query.limit(limit), | ||
| Query.orderDesc('') | ||
| ], | ||
| search: search || undefined | ||
| }), | ||
| sdk.forConsole.projects.list({ | ||
| queries: [ | ||
| Query.equal('teamId', params.organization), | ||
| Query.or([Query.equal('status', 'active'), Query.isNull('status')]) | ||
| ], | ||
| search: search || undefined | ||
| }), | ||
| sdk.forConsole.projects.list({ | ||
| queries: [ | ||
| Query.equal('teamId', params.organization), | ||
| Query.equal('status', 'archived') | ||
| ], | ||
| search: search || undefined | ||
| }) | ||
| ]); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Optional: verify Query.orderDesc('') usage.
Empty field name may be ignored or error depending on SDK version. Confirm intended default ordering or set an explicit field (e.g., createdAt/updatedAt).
🌐 Web query:
Is an empty string a valid field for Appwrite `Query.orderDesc('')`? What field should be used to order projects by most recently created in the Appwrite Console SDK?
💡 Result:
No — an empty string is not a valid field for Query.orderDesc(''). To sort by most-recently-created use the created timestamp field (Appwrite exposes $createdAt). Example: Query.orderDesc('$createdAt') — (escape the $ if your language/string parser requires, e.g. '$createdAt' in Flutter). You can also use $sequence for insertion order if preferred. [1][2][3]
Use explicit timestamp field in Query.orderDesc
Replace both instances of Query.orderDesc('') with Query.orderDesc('$createdAt') to ensure projects are sorted by most-recently-created timestamp.
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/+page.ts around lines 24 to
59, the two calls using Query.orderDesc('') should explicitly sort by creation
timestamp; replace both Query.orderDesc('') occurrences with
Query.orderDesc('$createdAt') so projects are ordered by most-recently-created
items.
What does this PR do?
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
yes
Summary by CodeRabbit
New Features
Improvements