-
Notifications
You must be signed in to change notification settings - Fork 196
feat: show project Go to Settings for credential queries #2526
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?
feat: show project Go to Settings for credential queries #2526
Conversation
|
This PR was not deployed automatically as @HarshMN2345 does not have access to the Railway project. In order to get automatic PR deploys, please add @HarshMN2345 to your workspace on Railway. |
ConsoleProject ID: Sites (1)
Tip Cursor pagination performs better than offset pagination when loading further pages. |
|
Warning Rate limit exceeded@HarshMN2345 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request updates the projects searcher to detect credential-related query terms and, when matched, navigate to the selected project's Settings using its region and id. The searcher now obtains current project context from the project store, computes per-project hrefs, and replaces simple name filtering with a multi-token filter built from project name, id, and region. The truncateText action's signature was extended to accept an optional text parameter (and update(newText?)), and regionEndpoint.svelte now passes Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: 1
🧹 Nitpick comments (1)
src/lib/commandCenter/searchers/projects.ts (1)
56-70: Consider usingmapinstead offlatMapfor clarity.Since each project produces exactly one navigation item, using
mapinstead offlatMapwould be more direct and slightly more efficient.- .flatMap((project) => { + .map((project) => { const href = `${base}/project-${project.region}-${project.$id}`; const label = project.name; - return [ - { - label, - callback: () => { - goto(href); - }, - group: 'projects' - } - ]; + return { + label, + callback: () => { + goto(href); + }, + group: 'projects' + }; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/commandCenter/searchers/projects.ts(1 hunks)src/lib/components/id.svelte(2 hunks)src/lib/components/regionEndpoint.svelte(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/commandCenter/searchers/projects.ts (3)
src/routes/(console)/project-[region]-[project]/store.ts (1)
project(11-11)src/lib/stores/sdk.ts (2)
sdk(153-176)getApiEndpoint(47-59)src/lib/stores/organization.ts (1)
organization(62-62)
⏰ 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 (2)
src/lib/components/regionEndpoint.svelte (1)
41-41: LGTM! Correctly leverages updatedtruncateTextsignature.Passing
region?.nameexplicitly to the action enables proper reactive truncation when the region changes, and the optional chaining safely handles undefined cases.src/lib/components/id.svelte (1)
32-33: LGTM! Backward-compatible enhancement for explicit text management.The optional parameters enable callers to explicitly provide text for truncation while maintaining backward compatibility with existing usage that relies on
node.textContent. Theupdatemethod correctly receives new text values from Svelte's action lifecycle when parameters change.Also applies to: 73-74
src/lib/components/id.svelte
Outdated
| export function truncateText(node: HTMLElement, text?: string) { | ||
| let originalText = text ?? node.textContent; |
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.
why is this needed? 🤔
actions should always work with elements and usually shouldnt have a new context passed 👍🏻
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.
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.
its fixed since we removed the regional name here , reverting 👍
| const q = query.toLowerCase().trim(); | ||
| const wantsCredentials = | ||
| q.includes('endpoint') || | ||
| q.includes('api key') || | ||
| q.includes('api-key') || | ||
| q.includes('apikey') || | ||
| q.includes('project id') || | ||
| q.includes('project-id') || | ||
| q === 'id' || | ||
| q.endsWith(' id') || | ||
| q.startsWith('end') || | ||
| q.includes('api end') || | ||
| (q.includes('api') && q.includes('end')); | ||
|
|
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.
I think we can make this simpler:
const keywords = [
'endpoint',
'api key',
'api-key',
'apikey',
'project id',
'project-id',
'api end'
];
const wantsCredentials = keywords.some(k => q.includes(k));something like this maybe
| } as const; | ||
| .filter((project) => { | ||
| const endpoint = getApiEndpoint(project.region); | ||
| const searchable = [project.name, project.$id, project.region, endpoint] |
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.
i dont think we need the endpoint here 👍🏻
| .flatMap((project) => { | ||
| const href = `${base}/project-${project.region}-${project.$id}`; | ||
|
|
||
| const label = project.name; | ||
|
|
||
| return [ | ||
| { | ||
| label, | ||
| callback: () => { | ||
| goto(href); | ||
| }, | ||
| group: 'projects' | ||
| } | ||
| ]; | ||
| }); |
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.
| .flatMap((project) => { | |
| const href = `${base}/project-${project.region}-${project.$id}`; | |
| const label = project.name; | |
| return [ | |
| { | |
| label, | |
| callback: () => { | |
| goto(href); | |
| }, | |
| group: 'projects' | |
| } | |
| ]; | |
| }); | |
| .map((project) => { | |
| const href = `${base}/project-${project.region}-${project.$id}`; | |
| const label = project.name; | |
| return { | |
| label, | |
| callback: () => { | |
| goto(href); | |
| }, | |
| group: 'projects' | |
| }; | |
| }); |
why flatmap over map?
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
🧹 Nitpick comments (1)
src/lib/commandCenter/searchers/projects.ts (1)
54-66: Consider simplifying label assignment.The
labelvariable (line 57) is assignedproject.nameand used only once. While this doesn't cause issues, you could directly useproject.namein the return object for conciseness:.map((project) => { const href = `${base}/project-${project.region}-${project.$id}`; - const label = project.name; - return { - label, + label: project.name, callback: () => { goto(href); }, group: 'projects' }; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/commandCenter/searchers/projects.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/lib/commandCenter/searchers/projects.ts
🧬 Code graph analysis (1)
src/lib/commandCenter/searchers/projects.ts (3)
src/routes/(console)/project-[region]-[project]/store.ts (1)
project(11-11)src/lib/stores/sdk.ts (1)
sdk(153-176)src/lib/stores/organization.ts (1)
organization(62-62)
🔇 Additional comments (4)
src/lib/commandCenter/searchers/projects.ts (4)
7-8: LGTM! Imports are correctly added.The new imports for
projectstore andbasepath are necessary for the credential navigation and href computation features.
11-23: Good simplification of credential detection.The keywords array with
.some()check addresses the previous review feedback and is much cleaner than the original boolean expression. While substring matching withincludes()can still produce occasional false positives (e.g., "api end" matching "api ended"), this is a reasonable tradeoff for simplicity and maintainability in a search context.Based on past review comments.
24-38: Verify behavior when no current project is available.When a user searches for credential-related terms (e.g., "api key") outside of a project context, the function returns an empty array (line 37). This means no results are shown, rather than falling through to the standard project list.
Is this the intended behavior? If users often search for credentials to navigate to a project's settings, showing the project list might be more helpful when no current project is available.
45-53: LGTM! Enhanced filtering improves search flexibility.The multi-token filter is a significant improvement, allowing users to search projects by name, id, or region. The AND logic (all query words must match) provides intuitive search behavior.


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?
(Write your answer here.)
Summary by CodeRabbit
New Features
Improvements