Skip to content

Conversation

@hmacr
Copy link
Contributor

@hmacr hmacr commented Jan 26, 2026

Resolves SER-642

Various improvements to repositories UX

  • Disable connect button on click
  • Fix GitHub installation links
  • Fix broken navigation of "create function"
  • Race condition in search
  • Opening a modal shows stale data initially

Summary by CodeRabbit

  • Improvements
    • Repository loading now shows clear loading states, prevents stale responses, and keeps pagination totals accurate.
    • Repository search stays enabled at all times.
    • Connect button disables while a repository is being connected to prevent duplicate actions.
    • Navigation and links for repository/site creation, templates, starters, and manual creation have been standardized to reliably open correct destinations.
    • GitHub permissions/settings links open externally when available.

✏️ Tip: You can customize this high-level summary in your review settings.

@appwrite
Copy link

appwrite bot commented Jan 26, 2026

Console (appwrite/console)

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Ready Ready View Logs Preview URL QR Code

Tip

Global CDN and DDoS protection come free with every Sites deployment

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 26, 2026

Walkthrough

This PR changes repository-list UI and two creation pages. In the repositories component it adds internal UI state (connectingRepositoryId, loadRepositoriesRequestId), marks loading on mount, implements per-call request IDs to ignore stale load responses, keeps the search input enabled, disables the Connect button while connecting, and adjusts pagination total/offset handling. In two create pages it replaces manual base-path construction with a resolveRoute utility for internal navigation and updates multiple route targets and conditional rendering to use reactive installation data.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Improve UX for repositories modal' is vague and generic. While the PR does address repositories UX improvements, the title doesn't convey specific changes like fixing navigation, disabling buttons, or preventing stale data that define the actual work. Consider a more specific title such as 'Fix repositories UX: disable connect button, fix navigation, and prevent stale data' to better reflect the actual changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ser-642

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/components/git/repositories.svelte (1)

121-137: Prevent stale requests from clearing the loading state early.

The stale-response guard prevents old data writes, but isLoadingRepositories is still flipped to false by earlier requests in the callers. That can hide loading while a newer request is still in flight. Consider tying loading-state updates to the same requestId.

🛠️ Suggested fix to tie loading state to the latest request
-    const debouncedLoadRepositories = debounce(
-        async (installationId: string, searchTerm: string) => {
-            isLoadingRepositories = true;
-            try {
-                await loadRepositories(installationId, searchTerm);
-            } finally {
-                isLoadingRepositories = false;
-            }
-        },
-        300
-    );
+    const debouncedLoadRepositories = debounce(
+        async (installationId: string, searchTerm: string) => {
+            await loadRepositories(installationId, searchTerm);
+        },
+        300
+    );

-    const loadRepositoryPage = async () => {
-        isLoadingRepositories = true;
-        try {
-            await loadRepositories(selectedInstallation, search);
-        } finally {
-            isLoadingRepositories = false;
-        }
-    };
+    const loadRepositoryPage = async () => {
+        await loadRepositories(selectedInstallation, search);
+    };

     async function loadRepositories(installationId: string, search: string) {
         const requestId = ++loadRepositoriesRequestId;
+        isLoadingRepositories = true;

-        const result = await sdk
-            .forProject(page.params.region, page.params.project)
-            .vcs.listRepositories({
-                installationId,
-                type:
-                    product === 'functions' ? VCSDetectionType.Runtime : VCSDetectionType.Framework,
-                search: search || undefined,
-                queries: [Query.limit(limit), Query.offset(offset)]
-            });
-
-        // Stale request
-        if (requestId !== loadRepositoriesRequestId) {
-            return;
-        }
-
-        $repositories.repositories =
-            product === 'functions'
-                ? (result as unknown as Models.ProviderRepositoryRuntimeList)
-                      .runtimeProviderRepositories
-                : (result as unknown as Models.ProviderRepositoryFrameworkList)
-                      .frameworkProviderRepositories; //TODO: remove forced cast after backend fixes
-        $repositories.total = result.total;
-        $repositories.search = search;
-        $repositories.installationId = installationId;
-        return $repositories.repositories;
+        try {
+            const result = await sdk
+                .forProject(page.params.region, page.params.project)
+                .vcs.listRepositories({
+                    installationId,
+                    type:
+                        product === 'functions'
+                            ? VCSDetectionType.Runtime
+                            : VCSDetectionType.Framework,
+                    search: search || undefined,
+                    queries: [Query.limit(limit), Query.offset(offset)]
+                });
+
+            // Stale request
+            if (requestId !== loadRepositoriesRequestId) {
+                return;
+            }
+
+            $repositories.repositories =
+                product === 'functions'
+                    ? (result as unknown as Models.ProviderRepositoryRuntimeList)
+                          .runtimeProviderRepositories
+                    : (result as unknown as Models.ProviderRepositoryFrameworkList)
+                          .frameworkProviderRepositories; //TODO: remove forced cast after backend fixes
+            $repositories.total = result.total;
+            $repositories.search = search;
+            $repositories.installationId = installationId;
+            return $repositories.repositories;
+        } finally {
+            if (requestId === loadRepositoriesRequestId) {
+                isLoadingRepositories = false;
+            }
+        }
     }
🤖 Fix all issues with AI agents
In `@src/lib/components/git/repositories.svelte`:
- Around line 276-280: The click handler sets connectingRepositoryId (used to
disable buttons) but never clears it, so failures block all buttons; update the
connect function to return a Promise and ensure connectingRepositoryId is reset
in a finally block (e.g., inside connect or in the caller) so it is cleared
whether connect succeeds or throws; update all callers of connect to
return/await that Promise so the finally runs and the disabled state (tied to
connectingRepositoryId) is released.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/functions/create-function/+page.svelte:
- Around line 52-64: The connect function can throw when $installation is
null/undefined because it directly accesses $installation.$id; add a guard at
the start of connect (function connect) to verify $installation exists (and
optionally $installation.$id) and either return early or handle the missing
installation (e.g., show error/redirect) before calling repository.set,
resolveRoute, or goto; ensure the constructed URL only uses $installation.$id
when it is defined to prevent a TypeError.

@hmacr hmacr merged commit ab39fb9 into main Jan 30, 2026
4 checks passed
@hmacr hmacr deleted the ser-642 branch January 30, 2026 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants