-
Notifications
You must be signed in to change notification settings - Fork 203
fix: filter non archived project #2729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Console (appwrite/console)Project ID: Tip Silent mode disables those chatty PR comments if you prefer peace and quiet |
WalkthroughReworks project listing for the organization console: the page component now derives Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/routes/(console)/organization-[organization]/+page.svelte (3)
243-248: Fix the mismatch between displayed projects and total count.The
CardContaineriterates overactiveProjects(line 248) but receivestotal={data.projects.total}(line 245). When archived projects exist,data.projects.totalincludes them whileactiveProjectsexcludes them, creating a mismatch between the number of items displayed and the total count shown in the UI.This will cause:
- Incorrect pagination indicators
- Misleading item counts
- Poor user experience
🔎 Proposed fix
<CardContainer disableEmpty={!$canWriteProjects} - total={data.projects.total} + total={activeProjects.length} offset={data.offset} on:click={handleCreateProject}> {#each activeProjects as project}
242-242: Consider checkingactiveProjects.lengthinstead ofdata.projects.total.When all projects are archived,
data.projects.total > 0will be true butactiveProjectswill be empty, potentially causing theCardContainerto render with no items. Consider checkingactiveProjects.length > 0for consistency with the filtered list being displayed.🔎 Proposed fix
- {#if data.projects.total > 0} + {#if activeProjects.length > 0} <CardContainer
326-330: Fix pagination to reflect active projects count.The
PaginationWithLimitcomponent usestotal={data.projects.total}, which includes archived projects. Since the UI now displays onlyactiveProjects, the pagination will be incorrect when archived projects exist.🔎 Proposed fix
<PaginationWithLimit name="Projects" limit={data.limit} offset={data.offset} - total={data.projects.total} /> + total={activeProjects.length} />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/organization-[organization]/+page.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/organization-[organization]/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/organization-[organization]/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/organization-[organization]/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/organization-[organization]/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/organization-[organization]/+page.svelte
🧠 Learnings (1)
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/organization-[organization]/+page.svelte
⏰ 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: build
- GitHub Check: e2e
🔇 Additional comments (1)
src/routes/(console)/organization-[organization]/+page.svelte (1)
80-82: LGTM: Clean derived store implementation.The
activeProjectsderived store correctly filters out archived projects and follows the same pattern as the existingprojectsToArchivestore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/organization-[organization]/+page.svelte (1)
244-332: Critical: Client-side filtering breaks server-side pagination.Filtering archived projects client-side while using server-side pagination creates a fundamental mismatch:
- The server returns paginated results (e.g., 25 projects) that include archived projects based on
data.offsetanddata.limit- Client-side filtering reduces the displayed count (e.g., to 22 if 3 are archived)
PaginationWithLimitreceivestotal={activeProjectsCount}(client-filtered count) butoffset={data.offset}andlimit={data.limit}(server-controlled values)- This causes incorrect pagination: the displayed count doesn't match the server's pagination window, making navigation unreliable and potentially hiding projects
Example scenario:
- Backend: 100 total projects (10 archived), returns projects 1-25
- Client displays: 22 active projects (filtered from the 25)
- Pagination shows: "1-22 of 90"
- Clicking "Next" uses server offset=25, potentially skipping or duplicating projects
🔎 Recommended fix
Move the archived project filtering to the server-side data loader (
+page.tsor+page.server.ts) where the query is constructed. This ensures:
- Pagination counts are accurate
data.projects.projectscontains only active projectsdata.projects.totalreflects the correct total- Client-side pagination calculations remain consistent
Example approach in the loader:
// In +page.ts or +page.server.ts const projects = await databases.listProjects({ queries: [ Query.notEqual('status', 'archived'), Query.limit(limit), Query.offset(offset) ] });Keep the client-side
projectsToArchivefilter for the separate archived projects section, but fetch active projects from the server.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/organization-[organization]/+page.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/organization-[organization]/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/organization-[organization]/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/organization-[organization]/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/organization-[organization]/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/organization-[organization]/+page.svelte
🧠 Learnings (2)
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/organization-[organization]/+page.svelte
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.
Applied to files:
src/routes/(console)/organization-[organization]/+page.svelte
⏰ 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 (2)
src/routes/(console)/organization-[organization]/+page.svelte (2)
80-85: Client-side filtering logic is correct, but creates pagination issues.The derived values correctly filter out archived projects using proper Svelte 5 syntax. However, applying this client-side filtering to server-paginated data will cause incorrect pagination behavior (detailed in the next comment).
55-68: Verify: Should project limits count archived projects?The
projectCreationDisabledandreachedProjectLimitchecks still usedata.projects.total, which includes archived projects, while the UI now displays onlyactiveProjectsCount. This could create confusion:
- A user might see 8 active projects displayed
- But be blocked from creating a 9th project because the limit is 10 and 2 archived projects exist
- The alert at line 199 mentions this scenario, suggesting it may be intentional
Please confirm whether:
- Intended behavior: Project limits should include archived projects (current implementation), OR
- Needs fixing: Project limits should only count active projects (change lines 58 and 66 to use
activeProjectsCount)If archived projects should count against limits, consider enhancing the tooltip at line 184 to clarify that archived projects are included in the count.
Based on coding guidelines: This review considers Svelte 5 + SvelteKit 2 conventions and proper usage of derived values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/routes/(console)/organization-[organization]/+page.ts (2)
59-64: Consider consolidating region normalization.The region normalization is performed in two separate loops for active and archived projects. While this works correctly, combining them would reduce code duplication.
🔎 Proposed consolidation
- for (const project of activeProjects.projects) { - project.region ??= 'default'; - } - for (const project of archivedProjects.projects) { - project.region ??= 'default'; - } + const allProjects = [...activeProjects.projects, ...archivedProjects.projects]; + for (const project of allProjects) { + project.region ??= 'default'; + }Alternatively, extract to a helper function:
const normalizeRegions = (projects: Array<Project>) => { for (const project of projects) { project.region ??= 'default'; } }; normalizeRegions(activeProjects.projects); normalizeRegions(archivedProjects.projects);
66-79: Clarify the object spread pattern in the return payload.Lines 69-72 spread the
activeProjectsobject but then explicitly override theprojectsandtotalfields. This pattern is confusing because:
- The spread may include fields from
activeProjectsthat are not needed (e.g., its originaltotalvalue)- The explicit override makes the spread partially redundant
Consider simplifying to only include the fields you need.
🔎 Proposed simplification
return { offset, limit, projects: { - ...activeProjects, projects: activeProjects.projects, total: activeTotal.total }, activeProjectsPage: activeProjects.projects, archivedProjectsPage: archivedProjects.projects, activeTotalOverall: activeTotal.total, archivedTotalOverall: archivedTotal.total, search };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/routes/(console)/organization-[organization]/+page.sveltesrc/routes/(console)/organization-[organization]/+page.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/organization-[organization]/+page.svelte
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/organization-[organization]/+page.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/routes/(console)/organization-[organization]/+page.ts
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/organization-[organization]/+page.ts
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/organization-[organization]/+page.ts
🧬 Code graph analysis (1)
src/routes/(console)/organization-[organization]/+page.ts (2)
src/lib/system.ts (1)
isCloud(26-26)src/lib/stores/sdk.ts (1)
sdk(173-196)
🔇 Additional comments (1)
src/routes/(console)/organization-[organization]/+page.ts (1)
81-89: Verify: Non-cloud path lacks status filtering.The non-cloud implementation doesn't filter projects by status (active/archived), which means it will return all projects mixed together. This is inconsistent with the cloud path that explicitly filters for active projects.
Confirm whether this is intentional or if the non-cloud path should also filter by status.
If non-cloud should also filter active projects, apply this change:
🔎 Proposed fix to add status filtering
const projects = await 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 });
| 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.equal('teamId', params.organization), | ||
| Query.equal('status', 'archived'), | ||
| Query.limit(1000) | ||
| ], | ||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Hardcoded limit will silently truncate archived projects.
Line 39 hardcodes Query.limit(1000) for archived projects. If an organization has more than 1000 archived projects, they will be silently truncated without any indication to the user.
Additionally, the activeTotal and archivedTotal queries (lines 43-56) fetch all matching projects without pagination just to obtain counts, which could be expensive for organizations with many projects.
Finally, Line 31 uses Query.orderDesc('') with an empty string parameter, which appears incorrect. This should either specify a field name (e.g., '$createdAt') or be removed entirely.
🔎 Proposed fixes
Fix 1: Remove hardcoded limit and use pagination for archived projects
If the UI needs all archived projects, consider removing the limit or using a much higher value with proper pagination:
sdk.forConsole.projects.list({
queries: [
Query.equal('teamId', params.organization),
Query.equal('status', 'archived'),
- Query.limit(1000)
+ Query.limit(5000) // Or implement pagination for archived projects
],
search: search || undefined
}),Fix 2: Use count-only queries for totals
If the SDK supports count queries, use them instead of fetching all records:
- sdk.forConsole.projects.list({
+ sdk.forConsole.projects.count({
queries: [
Query.equal('teamId', params.organization),
Query.or([Query.equal('status', 'active'), Query.isNull('status')])
],
search: search || undefined
}),
- sdk.forConsole.projects.list({
+ sdk.forConsole.projects.count({
queries: [
Query.equal('teamId', params.organization),
Query.equal('status', 'archived')
],
search: search || undefined
})Fix 3: Correct or remove the empty orderDesc parameter
Query.limit(limit),
- Query.orderDesc('')
+ Query.orderDesc('$createdAt')
],📝 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 [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.equal('teamId', params.organization), | |
| Query.equal('status', 'archived'), | |
| Query.limit(1000) | |
| ], | |
| 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 | |
| }) | |
| ]); | |
| 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('$createdAt') | |
| ], | |
| search: search || undefined | |
| }), | |
| sdk.forConsole.projects.list({ | |
| queries: [ | |
| Query.equal('teamId', params.organization), | |
| Query.equal('status', 'archived'), | |
| Query.limit(5000) | |
| ], | |
| 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 | |
| }) | |
| ]); |
🤖 Prompt for AI Agents
In src/routes/(console)/organization-[organization]/+page.ts around lines 24 to
57: the archived projects list uses a hardcoded Query.limit(1000) which can
silently truncate results, the activeTotal/archivedTotal requests fetch full
records to compute counts (expensive) and Query.orderDesc('') is invalid.
Replace the hardcoded limit by implementing proper pagination for archived
projects (accept page/limit params and request only that page) or remove the
limit if the UI truly needs all results and handle streaming/paging; change the
total queries to use the SDK's count-only endpoint/query (or a
Query.count()/meta flag) instead of listing all records; and fix
Query.orderDesc('') by supplying a valid field name (e.g. '$createdAt') or
remove the orderDesc call entirely.

What does this PR do?
(Provide a description of what this PR does.)
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?
(Write your answer here.)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.