From e050c162f2e455442eb514b422843ff00c700a70 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 15 Oct 2026 19:17:06 +0800 Subject: [PATCH 1/4] docs(lifecycle): explain deletion of promoted skills Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/14-skill-lifecycle.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/14-skill-lifecycle.md b/docs/14-skill-lifecycle.md index 1b78c8240..918359663 100644 --- a/docs/14-skill-lifecycle.md +++ b/docs/14-skill-lifecycle.md @@ -147,6 +147,18 @@ Status: current code-aligned reference - 全局 Skill 也可独立上传并审核。提升更新与独立上传使用相同版本号时不能互相覆盖；不同号版本分别发布，latest 指向最后通过审核的版本，不按版本字符串大小排序。 - 撤销提升不同于归档或隐藏：平台审核通过后移除全局派生 Skill 的公众入口及记录，保留来源团队 Skill、提升和撤销审计，并保护共用对象存储文件。重新提升形成新的全局 Skill ID。 +### 5.4 已提升技能如何删除 + +团队技能与提升生成的全局技能是两条关联记录。只撤回待审核版本、取消更新或撤回尚未批准的申请，**不会删除已经发布的全局技能，也不会解除首次提升的关联**。 + +若删除时提示“已提升的技能须通过审核撤销流程删除”，按以下顺序操作： + +1. 打开**来源团队技能详情页**，在提升区域选择“申请撤销提升”，填写原因并提交。 +2. 平台管理员在提升管理的撤销申请中审核；申请处于待审或被拒绝时，全局技能仍然存在，两条技能都不能绕过该流程直接删除。具有相应平台治理权限的管理员也可在来源详情页直接撤销。 +3. 撤销批准后，全局技能及其地址自动删除，来源团队技能和版本仍保留。此时如需连来源一起删除，由技能所有者或超级管理员执行来源技能的永久删除。 + +撤销时已有的全局更新申请会失效，不能在全局记录删除后重新发布到该记录。提升撤销保留提升、撤销与删除审计；来源技能后续永久删除仍按既有物理删除流程清理其业务记录，操作审计继续保留。不要通过手工清除来源关联字段来绕过撤销审核。 + ## 6. 权限边界 - `withdraw-review`：仅提交人本人 From 1319e791244c1a414c49c13e019dc23ac25732ea Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 15 Oct 2026 19:24:14 +0800 Subject: [PATCH 2/4] fix(lifecycle): guide promoted skill deletion and verify revocation Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/14-skill-lifecycle.md | 4 +- .../src/main/resources/messages.properties | 2 +- .../src/main/resources/messages_zh.properties | 2 +- .../PromotionDeletionPostgresTest.java | 186 ++++++++++++++++++ 4 files changed, 190 insertions(+), 4 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/PromotionDeletionPostgresTest.java diff --git a/docs/14-skill-lifecycle.md b/docs/14-skill-lifecycle.md index 918359663..e536f24d8 100644 --- a/docs/14-skill-lifecycle.md +++ b/docs/14-skill-lifecycle.md @@ -151,9 +151,9 @@ Status: current code-aligned reference 团队技能与提升生成的全局技能是两条关联记录。只撤回待审核版本、取消更新或撤回尚未批准的申请，**不会删除已经发布的全局技能，也不会解除首次提升的关联**。 -若删除时提示“已提升的技能须通过审核撤销流程删除”，按以下顺序操作： +若删除时提示须先撤销全局发布（旧版本提示“已提升的技能须通过审核撤销流程删除”），按以下顺序操作： -1. 打开**来源团队技能详情页**，在提升区域选择“申请撤销提升”，填写原因并提交。 +1. 打开**来源团队技能详情页**，在提升区域选择“申请撤销全局发布”，填写原因并提交。 2. 平台管理员在提升管理的撤销申请中审核；申请处于待审或被拒绝时，全局技能仍然存在，两条技能都不能绕过该流程直接删除。具有相应平台治理权限的管理员也可在来源详情页直接撤销。 3. 撤销批准后，全局技能及其地址自动删除，来源团队技能和版本仍保留。此时如需连来源一起删除，由技能所有者或超级管理员执行来源技能的永久删除。 diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 197d47424..9e5f42f6a 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -290,7 +290,7 @@ promotion.revocation.target_changed=The global target changed; submit a new revo promotion.revocation.duplicate_pending=A revocation request is already pending for this global skill promotion.revocation.not_found=Revocation request {0} was not found promotion.revocation.not_pending=Revocation request {0} is no longer pending -promotion.revocation.required=Use the reviewed revocation workflow to remove a promoted skill +promotion.revocation.required=Request global revocation from the source team skill details; delete the source after approval promotion.revocation.page_invalid=Page must be nonnegative and size must be between 1 and 100 error.auth.settings.unavailable=Authentication settings are unavailable error.system.roleGrant.unverifiedProvider=This identity provider does not attest verified email and cannot use initial role rules diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 854abbf5d..89a35088b 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -290,7 +290,7 @@ promotion.revocation.target_changed=全局目标已经变化，请重新提交 promotion.revocation.duplicate_pending=该全局技能已有待审核的撤销申请 promotion.revocation.not_found=撤销申请 {0} 不存在 promotion.revocation.not_pending=撤销申请 {0} 已不在待审核状态 -promotion.revocation.required=已提升的技能须通过审核撤销流程删除 +promotion.revocation.required=请先在来源团队技能详情中申请撤销全局发布，审核通过后再删除来源技能 promotion.revocation.page_invalid=页码不能为负数，每页数量须在 1 到 100 之间 error.auth.settings.unavailable=认证配置暂时不可用 error.system.roleGrant.unverifiedProvider=该身份来源不提供已验证邮箱，不能用于首次角色授权规则 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/PromotionDeletionPostgresTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/PromotionDeletionPostgresTest.java new file mode 100644 index 000000000..c7858ad89 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/PromotionDeletionPostgresTest.java @@ -0,0 +1,186 @@ +package com.iflytek.skillhub.repository; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceType; +import com.iflytek.skillhub.domain.review.PromotionRequest; +import com.iflytek.skillhub.domain.review.PromotionRequestKind; +import com.iflytek.skillhub.domain.review.PromotionRevocationService; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.security.SecurityScanService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.service.SkillHardDeleteService; +import com.iflytek.skillhub.domain.skill.service.SkillStorageDeletionCompensationService; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.infra.jpa.NamespaceJpaRepository; +import com.iflytek.skillhub.infra.jpa.PromotionRequestJpaRepository; +import com.iflytek.skillhub.infra.jpa.PromotionRevocationHistoryJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillVersionJpaRepository; +import com.iflytek.skillhub.infra.jpa.UserAccountJpaRepository; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.time.Clock; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Import; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionTemplate; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** Exercises the deletion boundary with real foreign keys, commits and retained audit history. */ +@DataJpaTest(showSql = false) +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@ActiveProfiles("test") +@Testcontainers +@Import({PromotionRevocationService.class, PromotionRevocationHistoryJpaRepository.class, + SkillHardDeleteService.class, AuditLogService.class, PromotionDeletionPostgresTest.Config.class}) +@Transactional(propagation = Propagation.NOT_SUPPORTED) +class PromotionDeletionPostgresTest { + @Container + private static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer<>("postgres:16-alpine"); + + @DynamicPropertySource + static void postgres(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); + registry.add("spring.datasource.username", POSTGRES::getUsername); + registry.add("spring.datasource.password", POSTGRES::getPassword); + registry.add("spring.datasource.driver-class-name", () -> "org.postgresql.Driver"); + registry.add("spring.jpa.database-platform", () -> "org.hibernate.dialect.PostgreSQLDialect"); + registry.add("spring.flyway.enabled", () -> true); + registry.add("spring.jpa.hibernate.ddl-auto", () -> "validate"); + } + + @TestConfiguration + static class Config { + @Bean Clock clock() { return Clock.systemUTC(); } + @Bean ObjectMapper objectMapper() { return new ObjectMapper(); } + } + + @Autowired private NamespaceJpaRepository namespaces; + @Autowired private UserAccountJpaRepository users; + @Autowired private SkillJpaRepository skills; + @Autowired private SkillVersionJpaRepository versions; + @Autowired private PromotionRequestJpaRepository promotions; + @Autowired private PromotionRevocationService revocations; + @Autowired private SkillHardDeleteService deletion; + @Autowired private PlatformTransactionManager transactions; + @Autowired private JdbcTemplate jdbc; + @MockBean private ObjectStorageService storage; + @MockBean private SkillStorageDeletionCompensationService compensation; + @MockBean private SecurityScanService scanning; + + @Test + void rejectedUpdateDoesNotRemoveThePublishedPromotion() { + Graph graph = fixture(ReviewTaskStatus.REJECTED); + assertDeletionBlocked(graph.source(), graph.teamSlug(), graph.owner()); + assertDeletionBlocked(graph.target(), "global", graph.owner()); + assertThat(promotions.findActiveInitialBySourceSkillId(graph.source().getId())).isPresent(); + assertThat(skills.existsById(graph.target().getId())).isTrue(); + assertThat(jdbc.queryForObject("SELECT count(*) FROM audit_log WHERE action = 'DELETE_SKILL_HARD' AND target_id IN (?, ?)", + Integer.class, graph.source().getId(), graph.target().getId())).isZero(); + } + + @Test + void onlyApprovedRevocationRemovesGlobalAndAllowsSourceDeletionWithAuditRetained() { + Graph graph = fixture(ReviewTaskStatus.PENDING); + var request = revocations.submit(graph.source().getId(), graph.owner(), Map.of(), Set.of(), "Remove global listing"); + assertDeletionBlocked(graph.source(), graph.teamSlug(), graph.owner()); + assertDeletionBlocked(graph.target(), "global", graph.owner()); + revocations.approve(request.getId(), graph.reviewer(), Set.of("SUPER_ADMIN"), "Approved", null, null); + assertThat(skills.existsById(graph.target().getId())).isFalse(); + assertThat(skills.existsById(graph.source().getId())).isTrue(); + assertThat(promotions.findActiveInitialBySourceSkillId(graph.source().getId())).isEmpty(); + assertThat(jdbc.queryForList("SELECT target_skill_id FROM promotion_request WHERE source_skill_id = ?", + Long.class, graph.source().getId())).containsOnlyNulls(); + assertThat(jdbc.queryForObject("SELECT status FROM promotion_request WHERE id = ?", + String.class, graph.updateId())).isEqualTo("REJECTED"); + assertThat(jdbc.queryForObject("SELECT target_skill_id_snapshot FROM promotion_request WHERE id = ?", + Long.class, graph.initialId())).isEqualTo(graph.target().getId()); + assertThat(jdbc.queryForObject("SELECT count(*) FROM skill_version WHERE skill_id = ?", + Integer.class, graph.source().getId())).isEqualTo(2); + deletion.hardDeleteSkill(graph.source(), graph.teamSlug(), graph.owner(), null, null); + assertThat(skills.existsById(graph.source().getId())).isFalse(); + assertThat(jdbc.queryForObject("SELECT status FROM promotion_revocation_request WHERE id = ?", + String.class, request.getId())).isEqualTo("APPROVED"); + assertThat(jdbc.queryForObject("SELECT count(*) FROM audit_log WHERE action = 'REVOKE_PROMOTION_TARGET' AND target_id = ?", + Integer.class, graph.target().getId())).isEqualTo(1); + assertThat(jdbc.queryForObject("SELECT count(*) FROM audit_log WHERE action = 'DELETE_SKILL_HARD' AND target_id = ?", + Integer.class, graph.source().getId())).isEqualTo(1); + } + + private void assertDeletionBlocked(Skill skill, String namespace, String actor) { + assertThatThrownBy(() -> deletion.hardDeleteSkill(skill, namespace, actor, null, null)) + .isInstanceOf(DomainBadRequestException.class).hasMessageContaining("promotion.revocation.required"); + } + + private Graph fixture(ReviewTaskStatus updateStatus) { + return new TransactionTemplate(transactions).execute(status -> { + String suffix = UUID.randomUUID().toString().substring(0, 8); + String owner = "delete-owner-" + suffix; + String reviewer = "delete-admin-" + suffix; + users.saveAndFlush(new UserAccount(owner, owner, null, null)); + users.saveAndFlush(new UserAccount(reviewer, reviewer, null, null)); + Namespace team = namespaces.saveAndFlush(new Namespace("delete-" + suffix, "Deletion team", owner)); + Namespace global = namespaces.findBySlug("global").orElseGet(() -> { + Namespace created = new Namespace("global", "Global", reviewer); + created.setType(NamespaceType.GLOBAL); + return namespaces.saveAndFlush(created); + }); + Skill source = skills.saveAndFlush(new Skill(team.getId(), "delete-" + suffix, owner, SkillVisibility.PUBLIC)); + SkillVersion first = publishedVersion(source, "1.0.0", owner); + SkillVersion second = publishedVersion(source, "1.1.0", owner); + source.setLatestVersionId(second.getId()); + skills.saveAndFlush(source); + Skill target = new Skill(global.getId(), source.getSlug(), owner, SkillVisibility.PUBLIC); + target.setSourceSkillId(source.getId()); + target = skills.saveAndFlush(target); + SkillVersion targetVersion = publishedVersion(target, "1.0.0", owner); + target.setLatestVersionId(targetVersion.getId()); + skills.saveAndFlush(target); + PromotionRequest initial = new PromotionRequest(source.getId(), first.getId(), global.getId(), owner); + initial.setStatus(ReviewTaskStatus.APPROVED); + initial.setTargetSkillId(target.getId()); + initial.setTargetVersionId(targetVersion.getId()); + initial = promotions.saveAndFlush(initial); + PromotionRequest update = new PromotionRequest(source.getId(), second.getId(), global.getId(), owner); + update.setRequestKind(PromotionRequestKind.UPDATE); + update.setStatus(updateStatus); + update.setTargetSkillId(target.getId()); + update = promotions.saveAndFlush(update); + return new Graph(source, target, team.getSlug(), owner, reviewer, initial.getId(), update.getId()); + }); + } + + private SkillVersion publishedVersion(Skill skill, String version, String owner) { + SkillVersion created = new SkillVersion(skill.getId(), version, owner); + created.setStatus(SkillVersionStatus.PUBLISHED); + return versions.saveAndFlush(created); + } + + private record Graph(Skill source, Skill target, String teamSlug, String owner, String reviewer, + Long initialId, Long updateId) { } +} From 7dbee64360acc7c19908e472f594ebafa7e1114a Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 16 Oct 2026 11:18:03 +0800 Subject: [PATCH 3/4] fix(web): report skill deletion errors once Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- web/src/pages/skill-detail.tsx | 29 +++---- .../shared/hooks/use-delete-skill.test.tsx | 86 +++++++++++++++++++ 2 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 web/src/shared/hooks/use-delete-skill.test.tsx diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index aa6e15352..18481f8e3 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -611,23 +611,18 @@ export function SkillDetailPage() { if (!skill || !isDeleteSlugConfirmationValid(skill.slug, deleteSkillInput)) { return } - try { - await deleteSkillMutation.mutateAsync({ namespace, slug, ownerId: skill.ownerId }) - setSkillDeleted(true) - toast.success( - t('skillDetail.deleteSkillSuccessTitle'), - t('skillDetail.deleteSkillSuccessDescription', { skill: skill.displayName }), - ) - setDeleteSkillInputOpen(false) - navigateAfterOverlays(() => { - navigate({ to: resolveDeletedSkillReturnTo(search.returnTo) }) - }) - queryClient.removeQueries({ queryKey: ['skills', namespace, slug] }) - queryClient.invalidateQueries({ queryKey: ['skills', 'my'] }) - } catch (error) { - toast.error(t('skillDetail.deleteSkillErrorTitle'), error instanceof Error ? error.message : '') - throw error - } + await deleteSkillMutation.mutateAsync({ namespace, slug, ownerId: skill.ownerId }) + setSkillDeleted(true) + toast.success( + t('skillDetail.deleteSkillSuccessTitle'), + t('skillDetail.deleteSkillSuccessDescription', { skill: skill.displayName }), + ) + setDeleteSkillInputOpen(false) + navigateAfterOverlays(() => { + navigate({ to: resolveDeletedSkillReturnTo(search.returnTo) }) + }) + queryClient.removeQueries({ queryKey: ['skills', namespace, slug] }) + queryClient.invalidateQueries({ queryKey: ['skills', 'my'] }) } const handleDeleteVersion = async () => { diff --git a/web/src/shared/hooks/use-delete-skill.test.tsx b/web/src/shared/hooks/use-delete-skill.test.tsx new file mode 100644 index 000000000..43fc24e40 --- /dev/null +++ b/web/src/shared/hooks/use-delete-skill.test.tsx @@ -0,0 +1,86 @@ +// @vitest-environment jsdom +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { useMutation } from '@tanstack/react-query' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { App } from '@/app/providers' +import { skillLifecycleApi } from '@/api/client' +import { ApiError } from '@/shared/lib/api-error' +import { toast } from '@/shared/lib/toast' +import { useDeleteSkill } from './use-skill-queries' + +vi.mock('@/app/router', () => ({ router: {} })) +vi.mock('@tanstack/react-router', () => ({ RouterProvider: () => })) +vi.mock('@/shared/components/toaster', () => ({ Toaster: () => null })) +vi.mock('@/shared/lib/toast', () => ({ toast: { error: vi.fn(), success: vi.fn() } })) + +const blockedError = new ApiError('revoke global publication first', 409, 'revoke global publication first') + +// Exercise the real application MutationCache. Like skill-detail, the caller +// reports success only; the global handler owns errors and authentication redirects. +function DeleteHarness() { + const deletion = useDeleteSkill() + const unrelated = useMutation({ mutationFn: async () => { throw blockedError } }) + const deleteSkill = async () => { + try { + await deletion.mutateAsync({ namespace: 'team', slug: 'promoted', ownerId: 'owner' }) + toast.success('deleted') + } catch { + // The click boundary consumes rejection; MutationCache already handled it. + } + } + return <> + + + +} + +afterEach(() => { + cleanup() + vi.restoreAllMocks() + vi.clearAllMocks() + vi.unstubAllGlobals() +}) + +describe('skill deletion feedback ownership', () => { + it('shows one actionable error per failed deletion, including a new retry', async () => { + const request = vi.spyOn(skillLifecycleApi, 'deleteSkill').mockRejectedValue(blockedError) + render() + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(1)) + expect(toast.error).toHaveBeenLastCalledWith(blockedError.serverMessage) + await waitFor(() => expect(screen.getByRole('button', { name: 'Delete' }).hasAttribute('disabled')).toBe(false)) + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + await waitFor(() => expect(request).toHaveBeenCalledTimes(2)) + await waitFor(() => expect(toast.error).toHaveBeenCalledTimes(2)) + expect(toast.success).not.toHaveBeenCalled() + }) + + it('reports a successful deletion once without an error notification', async () => { + vi.spyOn(skillLifecycleApi, 'deleteSkill').mockResolvedValue({ skillId: 123 }) + render() + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + await waitFor(() => expect(toast.success).toHaveBeenCalledTimes(1)) + expect(toast.error).not.toHaveBeenCalled() + }) + + + it('keeps the existing login redirect and one notification when the session expires', async () => { + vi.spyOn(skillLifecycleApi, 'deleteSkill').mockRejectedValue(new ApiError('session expired', 401)) + render() + const location = { href: '' } + // Override only location on a delegated window; React still sees the real DOM. + const testWindow = Object.create(window) + Object.defineProperty(testWindow, 'location', { value: location }) + vi.stubGlobal('window', testWindow) + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + await waitFor(() => expect(location.href).toBe('/login')) + expect(toast.error).toHaveBeenCalledTimes(1) + expect(toast.success).not.toHaveBeenCalled() + }) + + it('retains global error feedback for mutations without local ownership', async () => { + render() + fireEvent.click(screen.getByRole('button', { name: 'Other request' })) + await waitFor(() => expect(toast.error).toHaveBeenCalledExactlyOnceWith(blockedError.serverMessage)) + }) +}) From d2382086179cfa591ae0a3112dd87efdd9c12d2a Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 16 Oct 2026 11:44:01 +0800 Subject: [PATCH 4/4] fix(web): consume handled skill deletion failures Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- web/src/pages/skill-detail.test.tsx | 24 +++++++++++++++++++++++- web/src/pages/skill-detail.tsx | 8 +++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/web/src/pages/skill-detail.test.tsx b/web/src/pages/skill-detail.test.tsx index b86525d82..fcdfa6882 100644 --- a/web/src/pages/skill-detail.test.tsx +++ b/web/src/pages/skill-detail.test.tsx @@ -23,6 +23,7 @@ const usePromotionSourceStateMock = vi.fn() const submitPromotionMock = vi.fn() const submitRevocationMock = vi.fn() const directRevocationMock = vi.fn() +const deleteSkillMock = vi.fn() const useRevocationHistoryMock = vi.fn() const routeParams = { namespace: 'global', slug: 'demo-skill' } const searchMock = vi.hoisted(() => ({ value: { returnTo: '/dashboard/skills', version: undefined as string | undefined } })) @@ -212,7 +213,7 @@ vi.mock('@/shared/hooks/use-skill-queries', () => ({ useSkillReadme: (...args: unknown[]) => useSkillReadmeMock(...args), useSkillFile: (...args: unknown[]) => useSkillFileMock(...args), useArchiveSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), - useDeleteSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), + useDeleteSkill: () => ({ mutateAsync: deleteSkillMock, isPending: false }), useDeleteSkillVersion: () => ({ mutateAsync: vi.fn(), isPending: false }), useRereleaseSkillVersion: () => ({ mutateAsync: vi.fn(), isPending: false }), useUnarchiveSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), @@ -296,6 +297,7 @@ describe('SkillDetailPage', () => { usePromotionSourceStateMock.mockReturnValue({ data: undefined, isLoading: false, error: null }) searchMock.value = { returnTo: '/dashboard/skills', version: undefined } navigateMock.mockReset() + deleteSkillMock.mockReset() useSkillFilesMock.mockReset() useSkillReadmeMock.mockReset() useSkillFileMock.mockReset() @@ -334,6 +336,26 @@ describe('SkillDetailPage', () => { useSkillFileMock.mockReturnValue({ data: null, isLoading: false, error: null }) }) + it('consumes protected deletion failures at the actual button boundary and allows retry', async () => { + deleteSkillMock.mockRejectedValue(new Error('revoke global publication first')) + render() + fireEvent.click(screen.getByRole('button', { name: 'skillDetail.deleteSkill' })) + fireEvent.click(within(screen.getByRole('dialog', { name: 'skillDetail.deleteSkillConfirmTitle' })) + .getByRole('button', { name: 'skillDetail.deleteSkillContinue' })) + const dialog = await screen.findByRole('dialog', { name: 'skillDetail.deleteSkillInputTitle' }) + fireEvent.change(within(dialog).getByPlaceholderText('skillDetail.deleteSkillInputPlaceholder'), { target: { value: 'demo-skill' } }) + for (let attempt = 1; attempt <= 2; attempt++) { + fireEvent.click(within(dialog).getByRole('button', { name: 'skillDetail.deleteSkillFinal' })) + await waitFor(() => expect(deleteSkillMock).toHaveBeenCalledTimes(attempt)) + expect(within(dialog).getByDisplayValue('demo-skill')).toBeTruthy() + } + expect(deleteSkillMock).toHaveBeenLastCalledWith({ namespace: 'global', slug: 'demo-skill', ownerId: 'owner-1' }) + expect(toastMocks.success).not.toHaveBeenCalled() + expect(toastMocks.error).not.toHaveBeenCalled() // The real MutationCache owns feedback, tested separately. + expect(navigateMock).not.toHaveBeenCalled() + // An uncaught async onClick rejection also makes Vitest fail this real-page run. + }) + it('shows the avatar once beside the title, without an owner management section', () => { useSkillDetailMock.mockReturnValue({ data: createSkill({ avatarUrl: '/approved-avatar.png' }), isLoading: false, isFetching: false, error: null }) const view = render() diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index 18481f8e3..caf8f2a41 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -611,7 +611,13 @@ export function SkillDetailPage() { if (!skill || !isDeleteSlugConfirmationValid(skill.slug, deleteSkillInput)) { return } - await deleteSkillMutation.mutateAsync({ namespace, slug, ownerId: skill.ownerId }) + try { + await deleteSkillMutation.mutateAsync({ namespace, slug, ownerId: skill.ownerId }) + } catch { + // MutationCache already reports the error and handles expired sessions. + // Consume the button handler rejection without running success side effects. + return + } setSkillDeleted(true) toast.success( t('skillDetail.deleteSkillSuccessTitle'),