ux: show error message on contributor profile; guard loading on abort

ASR1015 · ASR1015 · commit 1eff568d22bb · 2025-08-14T10:58:16.000+05:30
diff --git a/library/githubSearch.ts b/library/githubSearch.ts
@@ -11,6 +11,9 @@ export type GitHubSearchItem = {
   created_at: string;
   updated_at: string;
   repository_url?: string;
+  pull_request?: {
+    merged_at: string | null;
+  };
 };
 
 export type SearchMode = "issues" | "prs";
@@ -26,12 +29,14 @@ function midpoint(a: Date, b: Date) {
   if (yyyymmdd(m) === yyyymmdd(a)) {
     const m2 = new Date(a);
     m2.setDate(m2.getDate() + 1);
-    return m2 < b ? m2 : new Date(a.getTime() + 12 * 3600 * 1000);
+    if (m2 <= b) return m2;
+    const halfDay = new Date(a.getTime() + 12 * 3600 * 1000);
+    return halfDay < b ? halfDay : b;
   }
   return m;
 }
 
-async function gh<T>(url: string, token?: string): Promise<T> {
+async function gh<T>(url: string, token?: string, signal?: AbortSignal): Promise<T> {
   let attempt = 0;
   while (true) {
     const resp = await fetch(url, {
@@ -40,8 +45,13 @@ async function gh<T>(url: string, token?: string): Promise<T> {
         ...(token ? { Authorization: `Bearer ${token}` } : {}),
         "X-GitHub-Api-Version": "2022-11-28",
       },
+      signal,
     });
 
+    if (signal?.aborted) {
+      throw new Error("Request aborted");
+    }
+
     if (resp.ok) {
       return (await resp.json()) as T;
     }
@@ -81,27 +91,28 @@ function buildQuery(opts: {
   q.push(mode === "prs" ? "type:pr" : "type:issue");
   q.push(mode === "prs" ? `author:${username}` : `involves:${username}`);
   if (repo) q.push(`repo:${repo}`);
-  if (title) q.push(`in:title ${title}`);
-  if (state !== "all") q.push(`state:${state}`);
+  if (title) q.push(`in:title "${title.replace(/"/g, '\\"')}"`);
+  if (state !== "all") q.push(`is:${state}`);
   const s = start ? yyyymmdd(start) : "2008-01-01";
   const e = end ? yyyymmdd(end) : yyyymmdd(new Date());
   q.push(`created:${s}..${e}`);
+  // NOTE: we join with '+' for GitHub search; each token may contain quotes; the URL layer encodes the string.
   return q.join("+");
 }
 
-async function searchCount(q: string, token?: string) {
+async function searchCount(q: string, token?: string, signal?: AbortSignal) {
   const url = `${GH_API}/search/issues?q=${q}&per_page=1&page=1`;
-  const data = await gh<{ total_count: number }>(url, token);
+  const data = await gh<{ total_count: number }>(url, token, signal);
   return data.total_count;
 }
 
-async function fetchWindow(q: string, token?: string): Promise<GitHubSearchItem[]> {
+async function fetchWindow(q: string, token?: string, signal?: AbortSignal): Promise<GitHubSearchItem[]> {
   const items: GitHubSearchItem[] = [];
   let page = 1;
   while (true) {
     const url = `${GH_API}/search/issues?q=${q}&per_page=${PER_PAGE}&page=${page}`;
-    const data = await gh<{ items: GitHubSearchItem[] }>(url, token);
-    const batch = (data as any).items || [];
+    const data = await gh<{ items: GitHubSearchItem[] }>(url, token, signal);
+    const batch = data.items || [];
     if (!batch.length) break;
     items.push(...batch);
     if (batch.length < PER_PAGE) break;
@@ -123,20 +134,21 @@ export async function searchUserIssuesAndPRs(params: {
   title?: string;
   start?: Date;
   end?: Date;
+  signal?: AbortSignal;
 }): Promise<GitHubSearchItem[]> {
   const start = params.start ?? new Date("2008-01-01");
   const end = params.end ?? new Date();
 
   async function recurse(win: { start: Date; end: Date }): Promise<GitHubSearchItem[]> {
     const q = buildQuery({ ...params, start: win.start, end: win.end });
-    const count = await searchCount(q, params.token);
+    const count = await searchCount(q, params.token, params.signal);
 
     if (count === 0) return [];
-    if (count <= 1000) return fetchWindow(q, params.token);
+    if (count <= 1000) return fetchWindow(q, params.token, params.signal);
 
     const mid = midpoint(win.start, win.end);
     const left = await recurse({ start: win.start, end: mid });
-    const right = await recurse({ start: new Date(mid.getTime() + 1000), end: win.end });
+    const right = await recurse({ start: new Date(mid.getTime() + 1), end: win.end });
     return [...left, ...right];
   }
 
@@ -161,6 +173,7 @@ export async function fetchUserItems(opts: {
   title?: string;
   start?: Date;
   end?: Date;
+  signal?: AbortSignal;
 }) {
   const token =
     (opts.userProvidedToken && opts.userProvidedToken.trim()) ||
@@ -180,5 +193,6 @@ export async function fetchUserItems(opts: {
     title: opts.title,
     start: opts.start,
     end: opts.end,
+    signal: opts.signal,
   });
 }
diff --git a/src/hooks/useGitHubData.ts b/src/hooks/useGitHubData.ts
@@ -1,12 +1,12 @@
 import { useState, useCallback } from "react";
-import { searchUserIssuesAndPRs } from "../../library/githubSearch";
+import { searchUserIssuesAndPRs, GitHubSearchItem } from "../../library/githubSearch";
 
 type GhState = "open" | "closed" | "all";
 
 
-export const useGitHubData = (_getOctokit: () => any) => {
-  const [issues, setIssues] = useState<any[]>([]);
-  const [prs, setPrs] = useState<any[]>([]);
+export const useGitHubData = () => {
+  const [issues, setIssues] = useState<GitHubSearchItem[]>([]);
+  const [prs, setPrs] = useState<GitHubSearchItem[]>([]);
   const [loading, setLoading] = useState(false);
   const [error, setError] = useState("");
   const [totalIssues, setTotalIssues] = useState(0);
@@ -26,32 +26,66 @@ export const useGitHubData = (_getOctokit: () => any) => {
   };
 
   const fetchData = useCallback(
-    async (username: string, page = 1, perPage = 10, state: GhState = "all") => {
-      if (!username || rateLimited) return;
-
+    async (
+      username: string,
+      page = 1,
+      perPage = 10,
+      state: GhState = "all",
+      signal?: AbortSignal
+    ) => {
       setLoading(true);
-      setError("");
-
       try {
         const token = readToken();
 
-        // Fetch full result sets using robust date-window pagination (bypasses 1000 cap)
-        const [allIssues, allPRs] = await Promise.all([
-          searchUserIssuesAndPRs({ username, mode: "issues", token, state }),
-          searchUserIssuesAndPRs({ username, mode: "prs", token, state }),
-        ]);
+        // basic param validation
+        if (page < 1 || perPage < 1) {
+          throw new Error("Invalid pagination parameters");
+        }
+
+        // clear old error; handle username/rate-limit UX
+        setError("");
+        if (!username) {
+          setLoading(false);
+          return;
+        }
+        if (rateLimited) {
+          setError("Rate limited. Please try again later.");
+          setLoading(false);
+          return;
+        }
 
-        // Save totals for pagination controls
-        setTotalIssues(allIssues.length);
-        setTotalPrs(allPRs.length);
+        // Abortable requests to avoid setState-on-unmounted
+        const internalCtrl = signal ? null : new AbortController();
+        const activeSignal = signal ?? internalCtrl!.signal;
 
-        // Client-side slice to requested page
-        const startIdx = Math.max(0, (page - 1) * perPage);
-        const endIdx = startIdx + perPage;
-        setIssues(allIssues.slice(startIdx, endIdx));
-        setPrs(allPRs.slice(startIdx, endIdx));
-      } catch (err: any) {
-        const msg = typeof err?.message === "string" ? err.message : "Failed to fetch data";
+        try {
+          // Fetch full result sets using robust date-window pagination (bypasses 1000 cap)
+          const [allIssues, allPRs] = await Promise.all([
+            searchUserIssuesAndPRs({ username, mode: "issues", token, state, signal: activeSignal }),
+            searchUserIssuesAndPRs({ username, mode: "prs", token, state, signal: activeSignal }),
+          ]);
+
+          // Save totals for pagination controls
+          setTotalIssues(allIssues.length);
+          setTotalPrs(allPRs.length);
+
+          // Client-side slice to requested page
+          const startIdx = Math.max(0, (page - 1) * perPage);
+          const endIdx = startIdx + perPage;
+          setIssues(allIssues.slice(startIdx, endIdx));
+          setPrs(allPRs.slice(startIdx, endIdx));
+
+          // clear rate-limit if we succeeded
+          if (rateLimited) setRateLimited(false);
+        } finally {
+          // Only abort if we created the controller
+          if (internalCtrl) internalCtrl.abort();
+        }
+      } catch (err: unknown) {
+        let msg = "Failed to fetch data";
+        if (err && typeof err === "object" && "message" in err && typeof (err as any).message === "string") {
+          msg = (err as any).message as string;
+        }
         setError(msg);
         if (msg.toLowerCase().includes("rate limit") || msg.includes("403")) {
           setRateLimited(true);
diff --git a/src/pages/ContributorProfile/ContributorProfile.tsx b/src/pages/ContributorProfile/ContributorProfile.tsx
@@ -1,22 +1,32 @@
 import { useParams } from "react-router-dom";
 import { useEffect, useState } from "react";
 import toast from "react-hot-toast";
-import { searchUserIssuesAndPRs } from "../../../library/githubSearch";
+import { searchUserIssuesAndPRs, GitHubSearchItem } from "../../../library/githubSearch";
+
+// Minimal shape from the GitHub Users API we actually render
+type GitHubUser = {
+  login: string;
+  avatar_url: string;
+  bio?: string | null;
+};
 
 export default function ContributorProfile() {
   const { username } = useParams();
-  const [profile, setProfile] = useState<any>(null);
-  const [prs, setPRs] = useState<any[]>([]);
+  const [profile, setProfile] = useState<GitHubUser | null>(null);
+  const [prs, setPRs] = useState<GitHubSearchItem[]>([]);
   const [loading, setLoading] = useState(true);
+  const [errorMsg, setErrorMsg] = useState<string | null>(null);
 
   useEffect(() => {
+    const controller = new AbortController();
     let canceled = false;
     let toastId: string | undefined;
 
     async function fetchData() {
       if (!username) return;
 
       setLoading(true);
+      setErrorMsg(null);
       toastId = toast.loading("Fetching PRs…");
 
       try {
@@ -29,43 +39,72 @@ export default function ContributorProfile() {
             ...(token ? { Authorization: `Bearer ${token}` } : {}),
             "X-GitHub-Api-Version": "2022-11-28",
           },
+          signal: controller.signal,
         });
+        if (controller.signal.aborted) return;
         if (!userRes.ok) {
+          if (userRes.status === 404) {
+            setProfile(null);
+            throw new Error("User not found");
+          }
           throw new Error(`Failed to fetch user: ${userRes.status}`);
         }
         const userData = await userRes.json();
         if (canceled) return;
-        setProfile(userData);
+        setProfile(userData as GitHubUser);
 
         // Robust PR fetch: replaced with searchUserIssuesAndPRs
         const prItems = await searchUserIssuesAndPRs({
           username,
           mode: "prs",
           state: "all",
           token,
+          signal: controller.signal,
         });
         if (canceled) return;
-        setPRs(prItems);
+        setPRs([...prItems].sort((a, b) => new Date(b.updated_at).getTime() - new Date(a.updated_at).getTime()));
       } catch (error) {
         console.error(error);
-        toast.error("Failed to fetch user data.");
+        const msg = error instanceof Error ? error.message : "Failed to fetch user data.";
+        setErrorMsg(msg);
+        toast.error(msg);
       } finally {
         if (toastId) toast.dismiss(toastId);
-        setLoading(false);
+        if (!controller.signal.aborted && !canceled) {
+          setLoading(false);
+        }
       }
     }
 
     fetchData();
 
     return () => {
+      controller.abort();
       canceled = true;
-      if (toastId) toast.dismiss(toastId);
+      if (toastId) {
+        toast.dismiss(toastId);
+        toastId = undefined;
+      }
     };
   }, [username]);
 
-  const handleCopyLink = () => {
-    navigator.clipboard.writeText(window.location.href);
-    toast.success("🔗 Shareable link copied to clipboard!");
+  const handleCopyLink = async () => {
+    try {
+      if (navigator.clipboard?.writeText) {
+        await navigator.clipboard.writeText(window.location.href);
+      } else {
+        // Fallback for older browsers
+        const el = document.createElement("textarea");
+        el.value = window.location.href;
+        document.body.appendChild(el);
+        el.select();
+        document.execCommand("copy");
+        document.body.removeChild(el);
+      }
+      toast.success("🔗 Shareable link copied to clipboard!");
+    } catch {
+      toast.error("Could not copy link");
+    }
   };
 
   if (loading) return <div className="text-center mt-10">Loading...</div>;
@@ -84,7 +123,7 @@ export default function ContributorProfile() {
           className="w-24 h-24 mx-auto rounded-full"
         />
         <h2 className="text-2xl font-bold mt-2">{profile.login}</h2>
-        <p className="">{profile.bio}</p>
+        <p className="">{profile.bio ?? ""}</p>
         <button
           onClick={handleCopyLink}
           className="mt-4 px-4 py-2 bg-blue-600 rounded hover:bg-blue-800 transition text-white"
@@ -93,6 +132,12 @@ export default function ContributorProfile() {
         </button>
       </div>
 
+      {errorMsg ? (
+        <div className="mt-4 mb-2 rounded border border-red-300 bg-red-50 text-red-700 dark:border-red-700/40 dark:bg-red-900/20 dark:text-red-300 px-3 py-2">
+          {errorMsg}
+        </div>
+      ) : null}
+
       <h3 className="text-xl font-semibold mt-6 mb-2">Pull Requests</h3>
       {prs.length > 0 ? (
         <ul className="list-disc ml-6 space-y-2">
@@ -106,7 +151,10 @@ export default function ContributorProfile() {
                   rel="noopener noreferrer"
                   className="text-blue-700 dark:text-blue-400 hover:underline"
                 >
-                  [{repoName}] {pr.title}
+                  {`[${repoName}] ${pr.title}`}
+                  {pr.pull_request?.merged_at ? (
+                    <span className="ml-2 inline-block px-2 py-0.5 text-xs rounded bg-green-600/20 text-green-700 dark:text-green-300">merged</span>
+                  ) : null}
                 </a>
               </li>
             );
diff --git a/vite.config.ts b/vite.config.ts
