Bladeren bron

fix(qa): 9 additional QA fixes — performance, security, UX

CRITICAL performance:
- db.ts: rewrite getAnalyticsSummary with SQL COUNT GROUP BY (no more OOM full-table scan)
- db.ts: rewrite searchKnowledge with PostgreSQL ILIKE query, limit 5 (no more 1000-row scan)

MEDIUM security/logic:
- routers.ts: analytics.track validates sessionId against existing conversation
- routers.ts: logKnowledgeSuggestion only fires when KB had no match (reduce noise)
- db.ts: deleteConversations wrapped in db.transaction() (atomic delete)

LOW security/UX:
- routers.ts: remove raw invitation token from audit log (store email only)
- DataSources.tsx: replace confirm() with toast action/cancel buttons
- DataSources.tsx: add product search (model/description/collection filter)
- AgentDashboard.tsx: replace confirm() with toast action/cancel buttons

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tony T 1 dag geleden
bovenliggende
commit
4abfe123fb
4 gewijzigde bestanden met toevoegingen van 124 en 45 verwijderingen
  1. 4 3
      client/src/pages/AgentDashboard.tsx
  2. 35 2
      client/src/pages/DataSources.tsx
  3. 73 36
      server/db.ts
  4. 12 4
      server/routers.ts

+ 4 - 3
client/src/pages/AgentDashboard.tsx

@@ -876,9 +876,10 @@ export default function AgentDashboard() {
                 size="sm" variant="outline" className="text-xs h-7"
                 style={{ borderColor: "#dc262640", color: "#dc2626" }}
                 onClick={() => {
-                  if (confirm(`Delete ${selectedIds.size} conversation(s)? This cannot be undone.`)) {
-                    deleteMutation.mutate({ conversationIds: Array.from(selectedIds) });
-                  }
+                  toast(`Delete ${selectedIds.size} conversation(s)? This cannot be undone.`, {
+                    action: { label: "Delete", onClick: () => deleteMutation.mutate({ conversationIds: Array.from(selectedIds) }) },
+                    cancel: { label: "Cancel", onClick: () => {} },
+                  });
                 }}
               >
                 <Trash2 className="w-3 h-3 mr-1" /> Delete

+ 35 - 2
client/src/pages/DataSources.tsx

@@ -44,6 +44,7 @@ function parseCSV(text: string): string[][] {
 export default function DataSources() {
   const [activeTab, setActiveTab] = useState<TabId>("sources");
   const [search, setSearch] = useState("");
+  const [productSearch, setProductSearch] = useState("");
   const [suggestionStatus, setSuggestionStatus] = useState<string>("pending");
 
   // Modal state
@@ -155,6 +156,18 @@ export default function DataSources() {
     };
   }, [allSuggestions]);
 
+  /* ─── Filtered products ─── */
+  const filteredProducts = useMemo(() => {
+    if (!products) return [];
+    if (!productSearch.trim()) return products;
+    const q = productSearch.toLowerCase();
+    return products.filter((p: any) =>
+      (p.model || "").toLowerCase().includes(q) ||
+      (p.description || "").toLowerCase().includes(q) ||
+      (p.collection || "").toLowerCase().includes(q)
+    );
+  }, [products, productSearch]);
+
   /* ─── Filtered entries ─── */
   const filteredEntries = useMemo(() => {
     if (!entries) return [];
@@ -485,7 +498,12 @@ export default function DataSources() {
                             <button onClick={() => setEditingEntry(e)} className="p-1 rounded hover:bg-blue-50">
                               <Edit2 className="w-3 h-3" style={{ color: "#0369a1" }} />
                             </button>
-                            <button onClick={() => { if (confirm("Delete this entry?")) deleteEntry.mutate({ id: e.id }); }} className="p-1 rounded hover:bg-red-50">
+                            <button onClick={() => {
+                              toast("Delete this entry?", {
+                                action: { label: "Delete", onClick: () => deleteEntry.mutate({ id: e.id }) },
+                                cancel: { label: "Cancel", onClick: () => {} },
+                              });
+                            }} className="p-1 rounded hover:bg-red-50">
                               <Trash2 className="w-3 h-3" style={{ color: "#dc2626" }} />
                             </button>
                           </div>
@@ -506,6 +524,19 @@ export default function DataSources() {
               <StatCard label="Total Products" value={products?.length || 0} color="#059669" />
             </div>
 
+            {/* Search */}
+            <div className="relative">
+              <Search className="w-3.5 h-3.5 absolute left-3 top-2.5" style={{ color: "#a8a29e" }} />
+              <input
+                type="text"
+                value={productSearch}
+                onChange={(e) => setProductSearch(e.target.value)}
+                placeholder="Search model, description, collection..."
+                className="w-full pl-9 pr-3 py-2 text-sm rounded-lg border"
+                style={{ borderColor: "#e7e0d5", background: "#fff" }}
+              />
+            </div>
+
             {/* Import note */}
             <div className="p-3 rounded-xl border text-xs" style={{ background: "#fff", borderColor: "#e7e0d5", color: "#78716C" }}>
               <FileText className="w-3.5 h-3.5 inline mr-1" style={{ color: "#0369a1" }} />
@@ -567,6 +598,8 @@ export default function DataSources() {
               <LoadingState />
             ) : !products || products.length === 0 ? (
               <EmptyState icon={<Package className="w-10 h-10" />} title="No products yet" hint="Import an Excel/CSV catalog to populate the product knowledge base" />
+            ) : filteredProducts.length === 0 ? (
+              <EmptyState icon={<Package className="w-10 h-10" />} title="No matches" hint="Try a different search term" />
             ) : (
               <div className="rounded-xl border overflow-hidden" style={{ background: "#fff", borderColor: "#e7e0d5" }}>
                 <table className="w-full text-xs">
@@ -582,7 +615,7 @@ export default function DataSources() {
                     </tr>
                   </thead>
                   <tbody>
-                    {products.map((p: any) => (
+                    {filteredProducts.map((p: any) => (
                       <tr key={p.id} className="border-t" style={{ borderColor: "#f5f0e8" }}>
                         <Td className="font-mono font-medium">{p.model}</Td>
                         <Td className="max-w-[250px] truncate" title={p.description || ""}>{p.description || <span style={{ color: "#a8a29e" }}>—</span>}</Td>

+ 73 - 36
server/db.ts

@@ -1,4 +1,4 @@
-import { eq, desc, asc, and, sql, like, ilike, or, lt, gte, lte, isNotNull, inArray } from "drizzle-orm";
+import { eq, desc, asc, and, sql, like, ilike, or, lt, gte, lte, isNotNull, inArray, count } from "drizzle-orm";
 import { drizzle } from "drizzle-orm/postgres-js";
 import {
   InsertUser, users,
@@ -435,9 +435,11 @@ export async function deleteConversations(ids: number[]) {
   const db = await getDb();
   if (!db) throw new Error("Database not available");
   if (ids.length === 0) return { deleted: 0 };
-  // Delete messages first, then conversations — both in a single statement each
-  await db.delete(messages).where(inArray(messages.conversationId, ids));
-  await db.delete(conversations).where(inArray(conversations.id, ids));
+  // Delete messages first, then conversations — wrapped in a transaction
+  await db.transaction(async (tx) => {
+    await tx.delete(messages).where(inArray(messages.conversationId, ids));
+    await tx.delete(conversations).where(inArray(conversations.id, ids));
+  });
   return { deleted: ids.length };
 }
 
@@ -600,34 +602,59 @@ export async function getAnalyticsEvents(filters?: {
 export async function getAnalyticsSummary(startDate?: Date, endDate?: Date) {
   const db = await getDb();
   if (!db) return null;
-  const conditions: any[] = [];
-  if (startDate) conditions.push(gte(analyticsEvents.createdAt, startDate));
-  if (endDate) conditions.push(lte(analyticsEvents.createdAt, endDate));
 
-  const baseQuery = conditions.length > 0
-    ? db.select().from(analyticsEvents).where(and(...conditions))
-    : db.select().from(analyticsEvents);
+  const dateConditions: any[] = [];
+  if (startDate) dateConditions.push(gte(analyticsEvents.createdAt, startDate));
+  if (endDate) dateConditions.push(lte(analyticsEvents.createdAt, endDate));
 
-  const allEvents = await baseQuery;
-
-  const totalSessions = allEvents.filter(e => e.eventType === "session_start").length;
-  const resolvedByBot = allEvents.filter(e => e.eventType === "resolved_by_bot").length;
-  const resolvedByAgent = allEvents.filter(e => e.eventType === "resolved_by_agent").length;
-  const escalated = allEvents.filter(e => e.eventType === "escalated").length;
-  const abandoned = allEvents.filter(e => e.eventType === "abandoned").length;
-  const messagesSent = allEvents.filter(e => e.eventType === "message_sent").length;
-  const messagesReceived = allEvents.filter(e => e.eventType === "message_received").length;
-  const buttonClicks = allEvents.filter(e => e.eventType === "button_clicked").length;
-  const positiveFeedback = allEvents.filter(e => e.eventType === "feedback_positive").length;
-  const negativeFeedback = allEvents.filter(e => e.eventType === "feedback_negative").length;
+  // Single GROUP BY query for event-type counts
+  const eventRows = await db.select({
+    eventType: analyticsEvents.eventType,
+    count: sql<number>`COUNT(*)`,
+  })
+    .from(analyticsEvents)
+    .where(dateConditions.length > 0 ? and(...dateConditions) : undefined)
+    .groupBy(analyticsEvents.eventType);
+
+  const eventCounts: Record<string, number> = {};
+  let totalEvents = 0;
+  for (const row of eventRows) {
+    const c = Number(row.count);
+    eventCounts[row.eventType as string] = c;
+    totalEvents += c;
+  }
 
-  // Category breakdown
+  const totalSessions = eventCounts["session_start"] || 0;
+  const resolvedByBot = eventCounts["resolved_by_bot"] || 0;
+  const resolvedByAgent = eventCounts["resolved_by_agent"] || 0;
+  const escalated = eventCounts["escalated"] || 0;
+  const abandoned = eventCounts["abandoned"] || 0;
+  const messagesSent = eventCounts["message_sent"] || 0;
+  const messagesReceived = eventCounts["message_received"] || 0;
+  const buttonClicks = eventCounts["button_clicked"] || 0;
+  const positiveFeedback = eventCounts["feedback_positive"] || 0;
+  const negativeFeedback = eventCounts["feedback_negative"] || 0;
+
+  // Category breakdown — single GROUP BY (category, eventType) query
   const categories = ["orders", "shipping", "returning", "cancelling"];
-  const categoryBreakdown = categories.map(cat => ({
-    category: cat,
-    count: allEvents.filter(e => e.category === cat).length,
-    resolved: allEvents.filter(e => e.category === cat && (e.eventType === "resolved_by_bot" || e.eventType === "resolved_by_agent")).length,
-  }));
+  const catConditions = [...dateConditions, inArray(analyticsEvents.category, categories)];
+  const catRows = await db.select({
+    category: analyticsEvents.category,
+    eventType: analyticsEvents.eventType,
+    count: sql<number>`COUNT(*)`,
+  })
+    .from(analyticsEvents)
+    .where(and(...catConditions))
+    .groupBy(analyticsEvents.category, analyticsEvents.eventType);
+
+  const categoryBreakdown = categories.map(cat => {
+    const rowsForCat = catRows.filter(r => r.category === cat);
+    const total = rowsForCat.reduce((s, r) => s + Number(r.count), 0);
+    const resolved = rowsForCat
+      .filter(r => r.eventType === "resolved_by_bot" || r.eventType === "resolved_by_agent")
+      .reduce((s, r) => s + Number(r.count), 0);
+    return { category: cat, count: total, resolved };
+  });
 
   return {
     totalSessions,
@@ -643,7 +670,7 @@ export async function getAnalyticsSummary(startDate?: Date, endDate?: Date) {
     resolutionRate: totalSessions > 0 ? Math.round(((resolvedByBot + resolvedByAgent) / totalSessions) * 100) : 0,
     botResolutionRate: totalSessions > 0 ? Math.round((resolvedByBot / totalSessions) * 100) : 0,
     categoryBreakdown,
-    totalEvents: allEvents.length,
+    totalEvents,
   };
 }
 
@@ -777,19 +804,29 @@ export async function searchKnowledge(userQuestion: string): Promise<{ id: numbe
   const db = await getDb();
   if (!db) return null;
 
+  const stopWords = new Set(["the","a","an","is","are","do","you","have","i","can","tell","me","about","how","what","when","where","why","which","my","your","our"]);
+  const queryWords = userQuestion.toLowerCase().replace(/[^a-z0-9 ]/g, " ").split(/\s+/).filter(w => w.length > 2 && !stopWords.has(w));
+
+  if (!queryWords.length) return null;
+
+  // Push search to DB: OR of ILIKE matches against question/answer
+  const ilikeConditions = queryWords.flatMap(w => {
+    const pattern = `%${w}%`;
+    return [
+      ilike(knowledgeEntries.question, pattern),
+      ilike(knowledgeEntries.answer, pattern),
+    ];
+  });
+
   const entries = await db
     .select({ id: knowledgeEntries.id, question: knowledgeEntries.question, answer: knowledgeEntries.answer, category: knowledgeEntries.category })
     .from(knowledgeEntries)
-    .where(eq(knowledgeEntries.status, "active"))
-    .limit(1000); // safety cap — migrate to DB full-text search when KB exceeds this
+    .where(and(eq(knowledgeEntries.status, "active"), or(...ilikeConditions))!)
+    .limit(5);
 
   if (!entries.length) return null;
 
-  const stopWords = new Set(["the","a","an","is","are","do","you","have","i","can","tell","me","about","how","what","when","where","why","which","my","your","our"]);
-  const queryWords = userQuestion.toLowerCase().replace(/[^a-z0-9 ]/g, " ").split(/\s+/).filter(w => w.length > 2 && !stopWords.has(w));
-
-  if (!queryWords.length) return null;
-
+  // Score the small candidate set in JS to pick the best match
   let best: { entry: typeof entries[0]; score: number } | null = null;
   for (const entry of entries) {
     const text = entry.question.toLowerCase();

+ 12 - 4
server/routers.ts

@@ -516,8 +516,11 @@ export const appRouter = router({
             content: botReply,
           });
 
-          // Auto-log as suggestion for continuous improvement
-          logKnowledgeSuggestion(input.content).catch(() => {});
+          // Auto-log as suggestion only when KB had no match (this LLM path is reached only
+          // after knowledgeAnswer was null, so logging is gated on that condition)
+          if (!knowledgeAnswer) {
+            logKnowledgeSuggestion(input.content).catch(() => {});
+          }
 
           // Log intent for hot-topic analytics (fire-and-forget)
           trackAnalyticsEvent({
@@ -900,7 +903,7 @@ export const appRouter = router({
           actorId: ctx.user.id,
           actorName: ctx.user.name || "Admin",
           targetName: input.email,
-          details: { role: input.role, token, expiresAt: expiresAt.toISOString() },
+          details: { role: input.role, email: input.email, expiresAt: expiresAt.toISOString() },
         });
 
         // Notify owner
@@ -954,7 +957,7 @@ export const appRouter = router({
           actorId: ctx.user.id,
           actorName: ctx.user.name || "Admin",
           targetName: invitation.email,
-          details: { role: invitation.role, newToken: token },
+          details: { role: invitation.role, email: invitation.email },
         });
 
         return newInvitation;
@@ -1240,6 +1243,11 @@ Return ONLY the JSON array, no markdown or explanation.`,
         metadata: z.any().optional(),
       }))
       .mutation(async ({ input }) => {
+        // Validate sessionId corresponds to an existing conversation if provided
+        if (input.sessionId) {
+          const conv = await getConversationBySessionId(input.sessionId);
+          if (!conv) return { success: false, id: null };
+        }
         const id = await trackAnalyticsEvent(input);
         return { id };
       }),