From 84bc5c7d9d7d7c92cf533ed6dbd67b14605977ef Mon Sep 17 00:00:00 2001 From: yun-zhi-ztl <66589705+yun-zhi-ztl@users.noreply.github.com> Date: Mon, 16 Mar 2026 19:29:26 +0800 Subject: [PATCH] fix: improve dashboard UX and session refresh handling (#51) * fix: refresh skill download counts after download * fix: limit skill search query length * fix: truncate long error messages in ui * fix: refresh auth roles promptly * fix: block disabled users with active sessions * fix: add my skills preview to dashboard * fix: align dashboard my skills layout * fix: refine dashboard my skills preview * fix: adjust dashboard my skills grid * fix: keep dashboard more tile visible * fix: refine dashboard copy tone --- .../skillhub/filter/AuthContextFilter.java | 30 +++++- .../filter/AuthContextFilterTest.java | 92 +++++++++++++++++++ web/src/api/client.ts | 40 ++++++++ web/src/app/router.tsx | 3 +- web/src/features/auth/use-auth.test.ts | 15 +++ web/src/features/auth/use-auth.ts | 17 +++- web/src/features/search/search-bar.tsx | 2 + web/src/i18n/locales/en.json | 18 ++-- web/src/i18n/locales/zh.json | 14 ++- web/src/pages/dashboard-preview.test.ts | 28 ++++++ web/src/pages/dashboard-preview.ts | 14 +++ web/src/pages/dashboard.tsx | 71 +++++++++++++- web/src/pages/device.tsx | 3 +- web/src/pages/home.tsx | 3 +- web/src/pages/landing.tsx | 3 +- web/src/pages/search.tsx | 7 +- web/src/pages/settings/accounts.tsx | 13 ++- web/src/pages/settings/security.tsx | 5 +- web/src/pages/skill-detail.tsx | 34 ++++++- web/src/shared/hooks/use-skill-queries.ts | 4 +- web/src/shared/lib/error-display.test.ts | 14 +++ web/src/shared/lib/error-display.ts | 18 ++++ web/src/shared/lib/search-query.test.ts | 15 +++ web/src/shared/lib/search-query.ts | 5 + .../shared/lib/skill-download-cache.test.ts | 85 +++++++++++++++++ web/src/shared/lib/skill-download-cache.ts | 77 ++++++++++++++++ web/src/shared/lib/toast.ts | 6 +- 27 files changed, 600 insertions(+), 36 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java create mode 100644 web/src/features/auth/use-auth.test.ts create mode 100644 web/src/pages/dashboard-preview.test.ts create mode 100644 web/src/pages/dashboard-preview.ts create mode 100644 web/src/shared/lib/error-display.test.ts create mode 100644 web/src/shared/lib/error-display.ts create mode 100644 web/src/shared/lib/search-query.test.ts create mode 100644 web/src/shared/lib/search-query.ts create mode 100644 web/src/shared/lib/skill-download-cache.test.ts create mode 100644 web/src/shared/lib/skill-download-cache.ts diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java index 60d7ac6d..aeced23f 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java @@ -4,15 +4,18 @@ import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMember; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.user.UserAccountRepository; import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; import java.io.IOException; import java.util.Map; import java.util.stream.Collectors; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.stereotype.Component; import org.springframework.web.filter.OncePerRequestFilter; @@ -20,9 +23,12 @@ import org.springframework.web.filter.OncePerRequestFilter; public class AuthContextFilter extends OncePerRequestFilter { private final NamespaceMemberRepository namespaceMemberRepository; + private final UserAccountRepository userAccountRepository; - public AuthContextFilter(NamespaceMemberRepository namespaceMemberRepository) { + public AuthContextFilter(NamespaceMemberRepository namespaceMemberRepository, + UserAccountRepository userAccountRepository) { this.namespaceMemberRepository = namespaceMemberRepository; + this.userAccountRepository = userAccountRepository; } @Override @@ -32,6 +38,11 @@ public class AuthContextFilter extends OncePerRequestFilter { FilterChain filterChain) throws ServletException, IOException { PlatformPrincipal principal = resolvePrincipal(request); if (principal != null) { + if (isInactiveUser(principal.userId())) { + clearAuthentication(request); + response.sendError(HttpServletResponse.SC_UNAUTHORIZED); + return; + } request.setAttribute("userId", principal.userId()); Map userNsRoles = namespaceMemberRepository.findByUserId(principal.userId()).stream() .collect(Collectors.toMap( @@ -44,6 +55,23 @@ public class AuthContextFilter extends OncePerRequestFilter { filterChain.doFilter(request, response); } + private boolean isInactiveUser(String userId) { + return userAccountRepository.findById(userId) + .map(user -> !user.isActive()) + .orElse(true); + } + + private void clearAuthentication(HttpServletRequest request) { + SecurityContextHolder.clearContext(); + HttpSession session = request.getSession(false); + if (session == null) { + return; + } + session.removeAttribute("platformPrincipal"); + session.removeAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY); + session.invalidate(); + } + private PlatformPrincipal resolvePrincipal(HttpServletRequest request) { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); if (authentication != null) { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java new file mode 100644 index 00000000..e6050ca4 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java @@ -0,0 +1,92 @@ +package com.iflytek.skillhub.filter; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import jakarta.servlet.FilterChain; +import jakarta.servlet.http.HttpSession; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.context.SecurityContextHolder; + +import java.util.List; +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +class AuthContextFilterTest { + + private final NamespaceMemberRepository namespaceMemberRepository = mock(NamespaceMemberRepository.class); + private final UserAccountRepository userAccountRepository = mock(UserAccountRepository.class); + private final AuthContextFilter filter = new AuthContextFilter(namespaceMemberRepository, userAccountRepository); + + @AfterEach + void clearSecurityContext() { + SecurityContextHolder.clearContext(); + } + + @Test + void disabledSessionUser_shouldInvalidateSessionAndBlockRequest() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal("user-1", "Alice", "alice@example.com", null, "local", Set.of("USER")); + UserAccount user = new UserAccount("Alice", "alice@example.com"); + user.setStatus(UserStatus.DISABLED); + + MockHttpServletRequest request = new MockHttpServletRequest(); + HttpSession session = request.getSession(true); + session.setAttribute("platformPrincipal", principal); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken(principal, null, List.of()) + ); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + when(userAccountRepository.findById("user-1")).thenReturn(java.util.Optional.of(user)); + + filter.doFilter(request, response, filterChain); + + assertEquals(401, response.getStatus()); + assertTrue(!request.isRequestedSessionIdValid() || request.getSession(false) == null); + assertNull(SecurityContextHolder.getContext().getAuthentication()); + verify(filterChain, never()).doFilter(request, response); + } + + @Test + void activeSessionUser_shouldPopulateRequestContextAndContinue() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal("user-2", "Bob", "bob@example.com", null, "local", Set.of("USER")); + UserAccount user = new UserAccount("Bob", "bob@example.com"); + user.setStatus(UserStatus.ACTIVE); + NamespaceMember member = new NamespaceMember(9L, "user-2", NamespaceRole.ADMIN); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.getSession(true).setAttribute("platformPrincipal", principal); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken(principal, null, List.of()) + ); + + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + + when(userAccountRepository.findById("user-2")).thenReturn(java.util.Optional.of(user)); + when(namespaceMemberRepository.findByUserId("user-2")).thenReturn(List.of(member)); + + filter.doFilter(request, response, filterChain); + + assertEquals("user-2", request.getAttribute("userId")); + assertEquals(NamespaceRole.ADMIN, ((java.util.Map) request.getAttribute("userNsRoles")).get(9L)); + verify(filterChain).doFilter(request, response); + } +} diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 4e08c3fa..bb0da6d9 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -37,6 +37,11 @@ export { ApiError } export const WEB_API_PREFIX = '/api/web' +export type DownloadedFile = { + blob: Blob + fileName?: string +} + type RuntimeConfig = { apiBaseUrl?: string appBaseUrl?: string @@ -273,6 +278,20 @@ function ensureTrailingSlash(value: string): string { return value.endsWith('/') ? value : `${value}/` } +function parseDownloadFileName(contentDisposition: string | null): string | undefined { + if (!contentDisposition) { + return undefined + } + + const utf8Match = contentDisposition.match(/filename\*=UTF-8''([^;]+)/i) + if (utf8Match) { + return decodeURIComponent(utf8Match[1]) + } + + const basicMatch = contentDisposition.match(/filename="?([^";]+)"?/i) + return basicMatch?.[1] +} + export async function getCurrentUser(): Promise { try { const user = await unwrap(client.GET('/api/v1/auth/me', { @@ -422,6 +441,27 @@ export const accountApi = { }, } +export const skillDownloadApi = { + async downloadVersion(namespace: string, slug: string, version: string): Promise { + const cleanNamespace = namespace.startsWith('@') ? namespace.slice(1) : namespace + const response = await fetch( + withBaseUrl(`${WEB_API_PREFIX}/skills/${cleanNamespace}/${slug}/versions/${version}/download`), + { + headers: withRequestHeaders(), + }, + ) + + if (!response.ok) { + throw new ApiError(`HTTP ${response.status}`, response.status) + } + + return { + blob: await response.blob(), + fileName: parseDownloadFileName(response.headers.get('content-disposition')), + } + }, +} + export const skillLifecycleApi = { async archiveSkill(namespace: string, slug: string, reason?: string): Promise { const cleanNamespace = namespace.startsWith('@') ? namespace.slice(1) : namespace diff --git a/web/src/app/router.tsx b/web/src/app/router.tsx index 63783b87..67422847 100644 --- a/web/src/app/router.tsx +++ b/web/src/app/router.tsx @@ -2,6 +2,7 @@ import { lazy, Suspense, type ComponentType } from 'react' import { createRouter, createRoute, createRootRoute, redirect } from '@tanstack/react-router' import { Layout } from './layout' import { getCurrentUser } from '@/api/client' +import { normalizeSearchQuery } from '@/shared/lib/search-query' function createLazyRouteComponent>( importer: () => Promise, @@ -139,7 +140,7 @@ const searchRoute = createRoute({ component: SearchPage, validateSearch: (search: Record) => { return { - q: (search.q as string) || '', + q: normalizeSearchQuery(typeof search.q === 'string' ? search.q : ''), sort: (search.sort as string) || 'newest', page: Number(search.page) || 0, starredOnly: search.starredOnly === true || search.starredOnly === 'true', diff --git a/web/src/features/auth/use-auth.test.ts b/web/src/features/auth/use-auth.test.ts new file mode 100644 index 00000000..49bb858a --- /dev/null +++ b/web/src/features/auth/use-auth.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from 'vitest' +import { getAuthQueryOptions } from './use-auth' + +describe('getAuthQueryOptions', () => { + it('keeps auth state fresh so role changes are picked up promptly', () => { + const options = getAuthQueryOptions(true) + + expect(options.queryKey).toEqual(['auth', 'me']) + expect(options.staleTime).toBe(0) + expect(options.refetchOnWindowFocus).toBe(true) + expect(options.refetchOnReconnect).toBe(true) + expect(options.refetchInterval).toBe(60_000) + expect(options.enabled).toBe(true) + }) +}) diff --git a/web/src/features/auth/use-auth.ts b/web/src/features/auth/use-auth.ts index 54fbf84f..4a9d66d5 100644 --- a/web/src/features/auth/use-auth.ts +++ b/web/src/features/auth/use-auth.ts @@ -2,14 +2,21 @@ import { useQuery } from '@tanstack/react-query' import { authApi } from '@/api/client' import type { User } from '@/api/types' -export function useAuth(enabled = true) { - const { data: user, isLoading, error } = useQuery({ - queryKey: ['auth', 'me'], +export function getAuthQueryOptions(enabled = true) { + return { + queryKey: ['auth', 'me'] as const, queryFn: authApi.getMe, retry: false, enabled, - staleTime: 5 * 60 * 1000, // 5 分钟 - }) + staleTime: 0, + refetchOnWindowFocus: true, + refetchOnReconnect: true, + refetchInterval: 60_000, + } +} + +export function useAuth(enabled = true) { + const { data: user, isLoading, error } = useQuery(getAuthQueryOptions(enabled)) return { user: user ?? null, diff --git a/web/src/features/search/search-bar.tsx b/web/src/features/search/search-bar.tsx index 9331ba0a..32a3bb49 100644 --- a/web/src/features/search/search-bar.tsx +++ b/web/src/features/search/search-bar.tsx @@ -1,6 +1,7 @@ import { useEffect, useState } from 'react' import { useTranslation } from 'react-i18next' import { Loader2, Search, X } from 'lucide-react' +import { MAX_SEARCH_QUERY_LENGTH } from '@/shared/lib/search-query' import { Input } from '@/shared/ui/input' import { Button } from '@/shared/ui/button' @@ -52,6 +53,7 @@ export function SearchBar({ defaultValue = '', value, placeholder, isSearching = type="text" value={currentQuery} onChange={(e) => handleChange(e.target.value)} + maxLength={MAX_SEARCH_QUERY_LENGTH} placeholder={placeholder || t('searchBar.placeholder')} className="pl-10 pr-10 border-0 bg-transparent focus-visible:ring-0 focus-visible:ring-offset-0 h-12" /> diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index 150a292c..a72d31ee 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -215,21 +215,27 @@ }, "dashboard": { "title": "Dashboard", - "subtitle": "Manage your account and API Tokens", + "subtitle": "View your account, skills, and access credentials in one place", "backToDashboard": "Back to Dashboard", - "userInfo": "User Info", - "userInfoDesc": "Your account details", + "userInfo": "Account Information", + "userInfoDesc": "Basic account details and platform roles", "loginVia": "Logged in via {{provider}}", "platformRoles": "Platform Roles", "starsAndRatings": "Stars & Ratings", "viewStars": "View My Stars", + "mySkillsTitle": "My Skills", + "openMySkills": "View My Skills", + "mySkillsPreviewDescription": "Showing your 5 most recent skills. Open a skill or go to My Skills to view all.", + "mySkillsPreviewEmpty": "You have not published any skills yet", "credentials": "Credentials", - "openTokens": "Open Token Page", + "openTokens": "View API Tokens", "governanceTitle": "Review & Governance", - "viewGovernance": "Open governance center", + "viewGovernance": "Open Governance Center", "viewPromotions": "View Promotions", "reportsTitle": "Report Management", - "viewReports": "View skill reports" + "viewReports": "View skill reports", + "previewMore": "...", + "previewMoreLabel": "View All" }, "mySkills": { "title": "My Skills", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index 6602a769..524ecfaf 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -215,21 +215,27 @@ }, "dashboard": { "title": "Dashboard", - "subtitle": "管理你的账户和 API Tokens", + "subtitle": "统一查看账户信息、技能资产与访问凭证", "backToDashboard": "返回控制台", "userInfo": "用户信息", - "userInfoDesc": "你的账户详情", + "userInfoDesc": "查看当前账户的基础信息与平台角色", "loginVia": "通过 {{provider}} 登录", "platformRoles": "平台角色", "starsAndRatings": "收藏与评分", "viewStars": "查看我的收藏", + "mySkillsTitle": "我的技能", + "openMySkills": "查看我的技能", + "mySkillsPreviewDescription": "展示最近的 5 个技能,可进入详情或前往“我的技能”查看全部。", + "mySkillsPreviewEmpty": "你还没有发布任何技能", "credentials": "访问凭证", - "openTokens": "打开 Token 页面", + "openTokens": "查看 API Tokens", "governanceTitle": "审核与治理", "viewGovernance": "打开治理中心", "viewPromotions": "查看提升审核", "reportsTitle": "举报管理", - "viewReports": "查看技能举报" + "viewReports": "查看技能举报", + "previewMore": "...", + "previewMoreLabel": "查看全部" }, "mySkills": { "title": "我的技能", diff --git a/web/src/pages/dashboard-preview.test.ts b/web/src/pages/dashboard-preview.test.ts new file mode 100644 index 00000000..0624595f --- /dev/null +++ b/web/src/pages/dashboard-preview.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from 'vitest' +import { limitPreviewItems } from './dashboard-preview' + +describe('limitPreviewItems', () => { + it('returns all items when the list is within the limit', () => { + expect(limitPreviewItems(['a', 'b'], 3)).toEqual({ + items: ['a', 'b'], + hasMore: false, + remainingCount: 0, + }) + }) + + it('returns only the first items and reports the remaining count', () => { + expect(limitPreviewItems(['a', 'b', 'c', 'd'], 3)).toEqual({ + items: ['a', 'b', 'c'], + hasMore: true, + remainingCount: 1, + }) + }) + + it('supports a five-item preview before the ellipsis entry', () => { + expect(limitPreviewItems(['a', 'b', 'c', 'd', 'e', 'f'], 5)).toEqual({ + items: ['a', 'b', 'c', 'd', 'e'], + hasMore: true, + remainingCount: 1, + }) + }) +}) diff --git a/web/src/pages/dashboard-preview.ts b/web/src/pages/dashboard-preview.ts new file mode 100644 index 00000000..66fd18c9 --- /dev/null +++ b/web/src/pages/dashboard-preview.ts @@ -0,0 +1,14 @@ +export function limitPreviewItems(items: T[], limit: number): { + items: T[] + hasMore: boolean + remainingCount: number +} { + const visibleItems = items.slice(0, limit) + const remainingCount = Math.max(items.length - visibleItems.length, 0) + + return { + items: visibleItems, + hasMore: remainingCount > 0, + remainingCount, + } +} diff --git a/web/src/pages/dashboard.tsx b/web/src/pages/dashboard.tsx index 12d96d56..ee696144 100644 --- a/web/src/pages/dashboard.tsx +++ b/web/src/pages/dashboard.tsx @@ -1,13 +1,19 @@ import { Link } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { useAuth } from '@/features/auth/use-auth' +import { useMySkills } from '@/shared/hooks/use-skill-queries' import { TokenList } from '@/features/token/token-list' import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card' +import { limitPreviewItems } from './dashboard-preview' + +const DASHBOARD_PREVIEW_LIMIT = 5 export function DashboardPage() { const { t } = useTranslation() const { user, hasRole } = useAuth() const governanceVisible = hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN') + const { data: skills, isLoading: isLoadingSkills } = useMySkills() + const skillPreview = limitPreviewItems(skills ?? [], DASHBOARD_PREVIEW_LIMIT) return (
@@ -59,13 +65,19 @@ export function DashboardPage() { -
+
{t('dashboard.starsAndRatings')}
{t('dashboard.viewStars')}
+ +
{t('dashboard.mySkillsTitle')}
+ + {t('dashboard.openMySkills')} + +
{t('dashboard.credentials')}
@@ -88,7 +100,62 @@ export function DashboardPage() { ) : null}
- +
+
+
+

{t('mySkills.title')}

+ + {t('dashboard.openMySkills')} + +
+

{t('dashboard.mySkillsPreviewDescription')}

+ + + {isLoadingSkills ? ( +
+ {Array.from({ length: DASHBOARD_PREVIEW_LIMIT + 1 }).map((_, index) => ( +
+ ))} +
+ ) : skillPreview.items.length > 0 ? ( +
+
+ {skillPreview.items.map((skill) => ( + +
{skill.displayName}
+
@{skill.namespace}
+ {skill.latestVersion ? ( +
+ v{skill.latestVersion} +
+ ) : null} + + ))} + + {t('dashboard.previewMore')} + {t('dashboard.previewMoreLabel')} + +
+
+ ) : ( +
{t('dashboard.mySkillsPreviewEmpty')}
+ )} + + +
+ +
+ +
+
) } diff --git a/web/src/pages/device.tsx b/web/src/pages/device.tsx index a67e189d..4de463e3 100644 --- a/web/src/pages/device.tsx +++ b/web/src/pages/device.tsx @@ -5,6 +5,7 @@ import { Button } from '@/shared/ui/button' import { Input } from '@/shared/ui/input' import { Label } from '@/shared/ui/label' import { fetchJson, getCsrfHeaders } from '@/api/client' +import { truncateErrorMessage } from '@/shared/lib/error-display' async function authorizeDevice(userCode: string): Promise { await fetchJson('/api/v1/device/authorize', { @@ -80,7 +81,7 @@ export function DeviceAuthPage() { } catch (error) { setMessage({ type: 'error', - text: error instanceof Error ? error.message : t('device.defaultError') + text: truncateErrorMessage(error instanceof Error ? error.message : t('device.defaultError')) ?? t('device.defaultError'), }) } finally { setIsSubmitting(false) diff --git a/web/src/pages/home.tsx b/web/src/pages/home.tsx index 73a66c60..3b1ef4c9 100644 --- a/web/src/pages/home.tsx +++ b/web/src/pages/home.tsx @@ -4,6 +4,7 @@ import { SearchBar } from '@/features/search/search-bar' import { SkillCard } from '@/features/skill/skill-card' import { SkeletonList } from '@/shared/components/skeleton-loader' import { useSearchSkills } from '@/shared/hooks/use-skill-queries' +import { normalizeSearchQuery } from '@/shared/lib/search-query' import { Button } from '@/shared/ui/button' import { Check, Copy, Terminal, Settings, PackageOpen } from 'lucide-react' import { useState, useMemo } from 'react' @@ -159,7 +160,7 @@ export function HomePage() { }) const handleSearch = (query: string) => { - navigate({ to: '/search', search: { q: query, sort: 'relevance', page: 0, starredOnly: false } }) + navigate({ to: '/search', search: { q: normalizeSearchQuery(query), sort: 'relevance', page: 0, starredOnly: false } }) } const handleSkillClick = (namespace: string, slug: string) => { diff --git a/web/src/pages/landing.tsx b/web/src/pages/landing.tsx index fb86f305..adc5b89f 100644 --- a/web/src/pages/landing.tsx +++ b/web/src/pages/landing.tsx @@ -3,6 +3,7 @@ import { useTranslation } from 'react-i18next' import { SearchBar } from '@/features/search/search-bar' import { useAuth } from '@/features/auth/use-auth' import { LanguageSwitcher } from '@/shared/components/language-switcher' +import { normalizeSearchQuery } from '@/shared/lib/search-query' import { UserMenu } from '@/shared/components/user-menu' import { Button } from '@/shared/ui/button' import { Check, Copy, Terminal, Settings, PackageOpen } from 'lucide-react' @@ -250,7 +251,7 @@ export function LandingPage() { }, []) const handleSearch = (query: string) => { - navigate({ to: '/search', search: { q: query, sort: 'relevance', page: 0, starredOnly: false } }) + navigate({ to: '/search', search: { q: normalizeSearchQuery(query), sort: 'relevance', page: 0, starredOnly: false } }) } const features = [ diff --git a/web/src/pages/search.tsx b/web/src/pages/search.tsx index 5556dfeb..7c47ba89 100644 --- a/web/src/pages/search.tsx +++ b/web/src/pages/search.tsx @@ -10,6 +10,7 @@ import { SkeletonList } from '@/shared/components/skeleton-loader' import { EmptyState } from '@/shared/components/empty-state' import { Pagination } from '@/shared/components/pagination' import { useMyStars, useSearchSkills } from '@/shared/hooks/use-skill-queries' +import { normalizeSearchQuery } from '@/shared/lib/search-query' import { Button } from '@/shared/ui/button' const PAGE_SIZE = 12 @@ -44,7 +45,7 @@ export function SearchPage() { const searchParams = useSearch({ from: '/search' }) const { isAuthenticated } = useAuth() - const q = searchParams.q || '' + const q = normalizeSearchQuery(searchParams.q || '') const sort = searchParams.sort || 'newest' const page = searchParams.page ?? 0 const starredOnly = searchParams.starredOnly ?? false @@ -68,7 +69,7 @@ export function SearchPage() { } = useMyStars(starredOnly && isAuthenticated) useEffect(() => { - const normalizedQuery = queryInput.trim() + const normalizedQuery = normalizeSearchQuery(queryInput) if (normalizedQuery === q) { return } @@ -90,7 +91,7 @@ export function SearchPage() { }, [navigate, page, q, queryInput, sort, starredOnly]) const handleSearch = (query: string) => { - const normalizedQuery = query.trim() + const normalizedQuery = normalizeSearchQuery(query) setQueryInput(query) startTransition(() => { navigate({ to: '/search', search: { q: normalizedQuery, sort, page: 0, starredOnly }, replace: true }) diff --git a/web/src/pages/settings/accounts.tsx b/web/src/pages/settings/accounts.tsx index 222149f9..b9df721e 100644 --- a/web/src/pages/settings/accounts.tsx +++ b/web/src/pages/settings/accounts.tsx @@ -1,6 +1,7 @@ import { useState } from 'react' import { useTranslation } from 'react-i18next' import { useConfirmAccountMerge, useInitiateAccountMerge, useVerifyAccountMerge } from '@/features/auth/use-account-merge' +import { truncateErrorMessage } from '@/shared/lib/error-display' import { Button } from '@/shared/ui/button' import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card' import { Input } from '@/shared/ui/input' @@ -25,7 +26,9 @@ export function AccountSettingsPage() { setVerificationToken(result.verificationToken) setStatusMessage(t('accounts.initiateSuccess', { secondaryUserId: result.secondaryUserId })) } catch (error) { - setStatusMessage(error instanceof Error ? error.message : t('accounts.initiateError')) + setStatusMessage( + truncateErrorMessage(error instanceof Error ? error.message : t('accounts.initiateError')) ?? t('accounts.initiateError'), + ) } } @@ -39,7 +42,9 @@ export function AccountSettingsPage() { }) setStatusMessage(t('accounts.verifySuccess')) } catch (error) { - setStatusMessage(error instanceof Error ? error.message : t('accounts.verifyError')) + setStatusMessage( + truncateErrorMessage(error instanceof Error ? error.message : t('accounts.verifyError')) ?? t('accounts.verifyError'), + ) } } @@ -49,7 +54,9 @@ export function AccountSettingsPage() { await confirmMutation.mutateAsync({ mergeRequestId: Number(mergeRequestId) }) setStatusMessage(t('accounts.confirmSuccess')) } catch (error) { - setStatusMessage(error instanceof Error ? error.message : t('accounts.confirmError')) + setStatusMessage( + truncateErrorMessage(error instanceof Error ? error.message : t('accounts.confirmError')) ?? t('accounts.confirmError'), + ) } } diff --git a/web/src/pages/settings/security.tsx b/web/src/pages/settings/security.tsx index 0426c9e6..85b84ab0 100644 --- a/web/src/pages/settings/security.tsx +++ b/web/src/pages/settings/security.tsx @@ -3,6 +3,7 @@ import { useNavigate } from '@tanstack/react-router' import { useQueryClient } from '@tanstack/react-query' import { useTranslation } from 'react-i18next' import { ApiError, authApi } from '@/api/client' +import { truncateErrorMessage } from '@/shared/lib/error-display' import { toast } from '@/shared/lib/toast' import { Button } from '@/shared/ui/button' import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/shared/ui/card' @@ -49,7 +50,9 @@ export function SecuritySettingsPage() { if (error instanceof ApiError && error.status === 401) { setErrorMessage(t('security.invalidCurrentPassword')) } else { - setErrorMessage(error instanceof Error ? error.message : t('security.defaultError')) + setErrorMessage( + truncateErrorMessage(error instanceof Error ? error.message : t('security.defaultError')) ?? t('security.defaultError'), + ) } } finally { setIsSubmitting(false) diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index 3e5e19c8..dd7da5b3 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -9,9 +9,10 @@ import { InstallCommand } from '@/features/skill/install-command' import { RatingInput } from '@/features/social/rating-input' import { StarButton } from '@/features/social/star-button' import { useAuth } from '@/features/auth/use-auth' -import { adminApi, ApiError, WEB_API_PREFIX } from '@/api/client' +import { adminApi, ApiError, skillDownloadApi } from '@/api/client' import { useSubmitSkillReport } from '@/features/report/use-skill-reports' import { formatLocalDateTime } from '@/shared/lib/date-time' +import { incrementSkillDownloadCount } from '@/shared/lib/skill-download-cache' import { formatCompactCount } from '@/shared/lib/number-format' import { resolveDocumentationFilePath } from '@/shared/lib/skill-documentation' import { NamespaceBadge } from '@/shared/components/namespace-badge' @@ -135,7 +136,18 @@ export function SkillDetailPage() { const submitPromotionMutation = useSubmitPromotion() const reportMutation = useSubmitSkillReport(namespace, slug) - const handleDownload = () => { + const triggerBrowserDownload = (blob: Blob, fileName: string) => { + const objectUrl = window.URL.createObjectURL(blob) + const link = document.createElement('a') + link.href = objectUrl + link.download = fileName + document.body.appendChild(link) + link.click() + link.remove() + window.setTimeout(() => window.URL.revokeObjectURL(objectUrl), 0) + } + + const handleDownload = async () => { if (!user) { requireLogin() return @@ -143,9 +155,21 @@ export function SkillDetailPage() { if (!selectedVersionEntry || isPendingPreview) { return } - const cleanNamespace = namespace.startsWith('@') ? namespace.slice(1) : namespace - const downloadUrl = `${WEB_API_PREFIX}/skills/${cleanNamespace}/${slug}/versions/${selectedVersionEntry.version}/download` - window.open(downloadUrl, '_blank') + + try { + const downloadedFile = await skillDownloadApi.downloadVersion(namespace, slug, selectedVersionEntry.version) + triggerBrowserDownload( + downloadedFile.blob, + downloadedFile.fileName ?? `${slug}-${selectedVersionEntry.version}.zip`, + ) + incrementSkillDownloadCount(queryClient, { namespace, slug }) + queryClient.invalidateQueries({ queryKey: ['skills', namespace, slug] }) + queryClient.invalidateQueries({ queryKey: ['skills', 'my'] }) + queryClient.invalidateQueries({ queryKey: ['skills', 'stars'] }) + queryClient.invalidateQueries({ queryKey: ['skills', 'search'] }) + } catch (error) { + toast.error(t('skillDetail.reportErrorTitle'), error instanceof Error ? error.message : '') + } } const requireLogin = () => { diff --git a/web/src/shared/hooks/use-skill-queries.ts b/web/src/shared/hooks/use-skill-queries.ts index 2e6304dd..1a4d3a44 100644 --- a/web/src/shared/hooks/use-skill-queries.ts +++ b/web/src/shared/hooks/use-skill-queries.ts @@ -1,12 +1,14 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' import type { SkillSummary, SkillDetail, SkillVersion, SkillVersionDetail, SkillFile, SearchParams, PagedResponse, PublishResult, Namespace, NamespaceMember, ManagedNamespace, CreateNamespaceRequest, NamespaceCandidateUser, NamespaceRole } from '@/api/types' import { fetchJson, fetchText, getCsrfHeaders, meApi, namespaceApi, promotionApi, skillLifecycleApi, WEB_API_PREFIX } from '@/api/client' +import { normalizeSearchQuery } from '@/shared/lib/search-query' const PUBLISH_REQUEST_TIMEOUT_MS = 60_000 async function searchSkills(params: SearchParams): Promise> { const queryParams = new URLSearchParams() - if (params.q) queryParams.append('q', params.q) + const normalizedQuery = normalizeSearchQuery(params.q ?? '') + if (normalizedQuery) queryParams.append('q', normalizedQuery) if (params.namespace) { const cleanNamespace = params.namespace.startsWith('@') ? params.namespace.slice(1) : params.namespace queryParams.append('namespace', cleanNamespace) diff --git a/web/src/shared/lib/error-display.test.ts b/web/src/shared/lib/error-display.test.ts new file mode 100644 index 00000000..f1b486f7 --- /dev/null +++ b/web/src/shared/lib/error-display.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from 'vitest' +import { MAX_ERROR_MESSAGE_LENGTH, truncateErrorMessage } from './error-display' + +describe('truncateErrorMessage', () => { + it('keeps short messages unchanged', () => { + expect(truncateErrorMessage('publish failed')).toBe('publish failed') + }) + + it('truncates long messages and appends an ellipsis', () => { + const message = 'x'.repeat(MAX_ERROR_MESSAGE_LENGTH + 20) + + expect(truncateErrorMessage(message)).toBe(`${'x'.repeat(MAX_ERROR_MESSAGE_LENGTH)}...`) + }) +}) diff --git a/web/src/shared/lib/error-display.ts b/web/src/shared/lib/error-display.ts new file mode 100644 index 00000000..c9bd7899 --- /dev/null +++ b/web/src/shared/lib/error-display.ts @@ -0,0 +1,18 @@ +export const MAX_ERROR_MESSAGE_LENGTH = 180 + +export function truncateErrorMessage(message?: string): string | undefined { + if (!message) { + return undefined + } + + const normalizedMessage = message.trim() + if (!normalizedMessage) { + return undefined + } + + if (normalizedMessage.length <= MAX_ERROR_MESSAGE_LENGTH) { + return normalizedMessage + } + + return `${normalizedMessage.slice(0, MAX_ERROR_MESSAGE_LENGTH)}...` +} diff --git a/web/src/shared/lib/search-query.test.ts b/web/src/shared/lib/search-query.test.ts new file mode 100644 index 00000000..b0322df0 --- /dev/null +++ b/web/src/shared/lib/search-query.test.ts @@ -0,0 +1,15 @@ +import { describe, expect, it } from 'vitest' +import { MAX_SEARCH_QUERY_LENGTH, normalizeSearchQuery } from './search-query' + +describe('normalizeSearchQuery', () => { + it('trims whitespace around the query', () => { + expect(normalizeSearchQuery(' hello world ')).toBe('hello world') + }) + + it('limits the query to fifty characters', () => { + const query = 'a'.repeat(MAX_SEARCH_QUERY_LENGTH + 12) + + expect(normalizeSearchQuery(query)).toHaveLength(MAX_SEARCH_QUERY_LENGTH) + expect(normalizeSearchQuery(query)).toBe('a'.repeat(MAX_SEARCH_QUERY_LENGTH)) + }) +}) diff --git a/web/src/shared/lib/search-query.ts b/web/src/shared/lib/search-query.ts new file mode 100644 index 00000000..1b28b3ea --- /dev/null +++ b/web/src/shared/lib/search-query.ts @@ -0,0 +1,5 @@ +export const MAX_SEARCH_QUERY_LENGTH = 50 + +export function normalizeSearchQuery(query: string): string { + return query.trim().slice(0, MAX_SEARCH_QUERY_LENGTH) +} diff --git a/web/src/shared/lib/skill-download-cache.test.ts b/web/src/shared/lib/skill-download-cache.test.ts new file mode 100644 index 00000000..38ca71ce --- /dev/null +++ b/web/src/shared/lib/skill-download-cache.test.ts @@ -0,0 +1,85 @@ +import { QueryClient } from '@tanstack/react-query' +import { describe, expect, it } from 'vitest' +import type { PagedResponse, SkillDetail, SkillSummary } from '@/api/types' +import { incrementSkillDownloadCount } from './skill-download-cache' + +function createSkillSummary(overrides: Partial = {}): SkillSummary { + return { + id: 1, + slug: 'demo-skill', + displayName: 'Demo Skill', + summary: 'summary', + status: 'PUBLISHED', + downloadCount: 10, + starCount: 2, + ratingAvg: 5, + ratingCount: 1, + latestVersion: '1.0.0', + latestVersionId: 100, + latestVersionStatus: 'PUBLISHED', + namespace: 'team', + updatedAt: '2026-03-16T00:00:00Z', + canSubmitPromotion: false, + ...overrides, + } +} + +function createSkillDetail(overrides: Partial = {}): SkillDetail { + return { + id: 1, + slug: 'demo-skill', + displayName: 'Demo Skill', + summary: 'summary', + visibility: 'PUBLIC', + status: 'ACTIVE', + downloadCount: 10, + starCount: 2, + ratingAvg: 5, + ratingCount: 1, + hidden: false, + latestVersion: '1.0.0', + latestVersionId: 100, + namespace: 'team', + canManageLifecycle: false, + canSubmitPromotion: false, + viewingVersionStatus: 'PUBLISHED', + canInteract: true, + ...overrides, + } +} + +describe('incrementSkillDownloadCount', () => { + it('increments the skill detail and cached list entries for the downloaded skill', () => { + const queryClient = new QueryClient() + const searchPage: PagedResponse = { + items: [ + createSkillSummary(), + createSkillSummary({ id: 2, slug: 'other-skill', displayName: 'Other Skill', downloadCount: 4 }), + ], + total: 2, + page: 0, + size: 12, + } + + queryClient.setQueryData(['skills', '@team', 'demo-skill'], createSkillDetail({ namespace: 'team' })) + queryClient.setQueryData(['skills', 'my'], searchPage.items) + queryClient.setQueryData(['skills', 'stars'], searchPage.items) + queryClient.setQueryData(['skills', 'search', { q: '', sort: 'downloads', page: 0, size: 12, starredOnly: false }], searchPage) + + incrementSkillDownloadCount(queryClient, { namespace: '@team', slug: 'demo-skill' }) + + expect(queryClient.getQueryData(['skills', '@team', 'demo-skill'])?.downloadCount).toBe(11) + expect(queryClient.getQueryData(['skills', 'my'])?.[0]?.downloadCount).toBe(11) + expect(queryClient.getQueryData(['skills', 'stars'])?.[0]?.downloadCount).toBe(11) + expect( + queryClient.getQueryData>( + ['skills', 'search', { q: '', sort: 'downloads', page: 0, size: 12, starredOnly: false }], + )?.items[0]?.downloadCount, + ).toBe(11) + expect( + queryClient.getQueryData>( + ['skills', 'search', { q: '', sort: 'downloads', page: 0, size: 12, starredOnly: false }], + )?.items[1]?.downloadCount, + ).toBe(4) + }) +}) diff --git a/web/src/shared/lib/skill-download-cache.ts b/web/src/shared/lib/skill-download-cache.ts new file mode 100644 index 00000000..d5af24c5 --- /dev/null +++ b/web/src/shared/lib/skill-download-cache.ts @@ -0,0 +1,77 @@ +import type { QueryClient } from '@tanstack/react-query' +import type { PagedResponse, SkillDetail, SkillSummary } from '@/api/types' + +type SkillIdentity = { + namespace: string + slug: string +} + +function normalizeNamespace(namespace: string): string { + return namespace.startsWith('@') ? namespace.slice(1) : namespace +} + +function matchesSkill(skill: SkillIdentity, target: SkillIdentity): boolean { + return normalizeNamespace(skill.namespace) === normalizeNamespace(target.namespace) && skill.slug === target.slug +} + +function incrementSummaryDownloadCount(skill: SkillSummary, target: SkillIdentity): SkillSummary { + if (!matchesSkill(skill, target)) { + return skill + } + return { + ...skill, + downloadCount: skill.downloadCount + 1, + } +} + +function incrementDetailDownloadCount(skill: SkillDetail | undefined, target: SkillIdentity): SkillDetail | undefined { + if (!skill || !matchesSkill(skill, target)) { + return skill + } + return { + ...skill, + downloadCount: skill.downloadCount + 1, + } +} + +function incrementSummaryList( + skills: SkillSummary[] | undefined, + target: SkillIdentity, +): SkillSummary[] | undefined { + return skills?.map((skill) => incrementSummaryDownloadCount(skill, target)) +} + +function incrementPagedSummaryList( + page: PagedResponse | undefined, + target: SkillIdentity, +): PagedResponse | undefined { + if (!page) { + return page + } + return { + ...page, + items: page.items.map((skill) => incrementSummaryDownloadCount(skill, target)), + } +} + +export function incrementSkillDownloadCount( + queryClient: QueryClient, + target: SkillIdentity, +): void { + queryClient.setQueryData( + ['skills', target.namespace, target.slug], + (current) => incrementDetailDownloadCount(current, target), + ) + queryClient.setQueryData( + ['skills', 'my'], + (current) => incrementSummaryList(current, target), + ) + queryClient.setQueryData( + ['skills', 'stars'], + (current) => incrementSummaryList(current, target), + ) + queryClient.setQueriesData>( + { queryKey: ['skills', 'search'] }, + (current) => incrementPagedSummaryList(current, target), + ) +} diff --git a/web/src/shared/lib/toast.ts b/web/src/shared/lib/toast.ts index 8a9254f3..b06bfb5c 100644 --- a/web/src/shared/lib/toast.ts +++ b/web/src/shared/lib/toast.ts @@ -1,4 +1,5 @@ import { toast as sonnerToast, type ExternalToast } from 'sonner' +import { truncateErrorMessage } from './error-display' export const CENTER_TOASTER_ID = 'top-center' @@ -26,7 +27,10 @@ export const toast = { sonnerToast.success(message, { description, ...withDefaultToaster(options) }) }, error: (message: string, description?: string, options?: ExternalToast) => { - sonnerToast.error(message, { description, ...withDefaultToaster(options) }) + sonnerToast.error(truncateErrorMessage(message) ?? message, { + description: truncateErrorMessage(description), + ...withDefaultToaster(options), + }) }, warning: (message: string, description?: string, options?: ExternalToast) => { sonnerToast.warning(message, { description, ...withDefaultToaster(options) })