From 26dad593e12655200262e7488d6ada7b95d38ffa Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:13:20 +0800 Subject: [PATCH] fix(review): preserve author cleanup access Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../service/SkillReviewAppService.java | 1 - .../SkillRatingOptimisticLockingTest.java | 16 +++++++++++++++ .../service/SkillReviewAppServiceTest.java | 12 +++++++++++ .../features/social/skill-reviews.test.tsx | 20 +++++++++++++++---- web/src/features/social/skill-reviews.tsx | 17 +++++++++++++++- 5 files changed, 60 insertions(+), 6 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillReviewAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillReviewAppService.java index 2776b556..4c33b921 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillReviewAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillReviewAppService.java @@ -76,7 +76,6 @@ public class SkillReviewAppService { String userId, Map namespaceRoles, Set platformRoles) { - requireVisibleSkill(skillId, userId, namespaceRoles, platformRoles); return ratingService.getUserFeedback(skillId, userId) .map(this::toMine) .orElseGet(SkillReviewMeResponse::empty); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java index 2698beb4..6d5996ef 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java @@ -42,6 +42,7 @@ class SkillRatingOptimisticLockingTest { rollbackIfActive(staleManager); firstManager.close(); staleManager.close(); + deleteRating(ratingId); } } @@ -66,6 +67,21 @@ class SkillRatingOptimisticLockingTest { } } + private void deleteRating(Long ratingId) { + EntityManager entityManager = entityManagerFactory.createEntityManager(); + try { + entityManager.getTransaction().begin(); + SkillRating rating = entityManager.find(SkillRating.class, ratingId); + if (rating != null) { + entityManager.remove(rating); + } + entityManager.getTransaction().commit(); + } finally { + rollbackIfActive(entityManager); + entityManager.close(); + } + } + private boolean hasCause(Throwable error, Class expectedType) { Throwable current = error; while (current != null) { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java index 3e3f59b0..53697e56 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java @@ -122,6 +122,18 @@ class SkillReviewAppServiceTest { verify(skillVersionRepository, never()).findById(any()); } + @Test + void authorCanReadOwnReviewAfterSkillStopsBeingVisible() { + SkillRating review = new SkillRating(10L, "author", (short) 4); + review.updateReview((short) 4, "My review"); + when(ratingService.getUserFeedback(10L, "author")).thenReturn(Optional.of(review)); + + service.getMine(10L, "author", Map.of(), Set.of()); + + verify(ratingService).getUserFeedback(10L, "author"); + verify(skillRepository, never()).findById(any()); + } + @Test void hideWritesModerationAuditInSameWorkflow() { SkillRating review = new SkillRating(10L, "author", (short) 4); diff --git a/web/src/features/social/skill-reviews.test.tsx b/web/src/features/social/skill-reviews.test.tsx index 07e1e019..cd82ff59 100644 --- a/web/src/features/social/skill-reviews.test.tsx +++ b/web/src/features/social/skill-reviews.test.tsx @@ -12,6 +12,7 @@ const mocks = vi.hoisted(() => ({ saveMutate: vi.fn(), clearMutate: vi.fn(), moderateMutate: vi.fn(), + mineEnabled: [] as boolean[], })) vi.mock('react-i18next', async () => { @@ -62,8 +63,9 @@ vi.mock('./use-skill-reviews', () => ({ mocks.requestedPages.push(page) return { data: mocks.pages.get(page), isLoading: false, isError: false } }, - useMySkillReview: () => ({ - data: { + useMySkillReview: (_skillId: number, enabled: boolean) => { + mocks.mineEnabled.push(enabled) + return { data: { rated: true, score: 4, reviewed: true, @@ -71,8 +73,8 @@ vi.mock('./use-skill-reviews', () => ({ reviewText: 'Useful review', status: 'VISIBLE', updatedAt: '2026-09-01T00:00:00Z', - }, - }), + } } + }, useUpsertSkillReview: () => ({ mutate: mocks.saveMutate, isPending: false }), useClearSkillReview: () => ({ mutate: mocks.clearMutate, isPending: false }), useModerateSkillReview: () => ({ mutate: mocks.moderateMutate, isPending: false }), @@ -85,6 +87,7 @@ describe('skill reviews', () => { mocks.pages.clear() mocks.pages.set(0, { items: [], total: 0, page: 0, size: 20 }) mocks.requestedPages.length = 0 + mocks.mineEnabled.length = 0 }) afterEach(() => { @@ -141,4 +144,13 @@ describe('skill reviews', () => { expect(locale.skillReviews.restore).toBeTruthy() } }) + + it('lets an authenticated author clear existing text when the skill is not interactable', () => { + render() + + expect(mocks.mineEnabled).toContain(true) + fireEvent.click(screen.getByRole('button', { name: 'Delete review' })) + expect(mocks.clearMutate).toHaveBeenCalledOnce() + expect(screen.queryByRole('button', { name: 'Edit my review' })).toBeNull() + }) }) diff --git a/web/src/features/social/skill-reviews.tsx b/web/src/features/social/skill-reviews.tsx index f7b99a26..8f130d9b 100644 --- a/web/src/features/social/skill-reviews.tsx +++ b/web/src/features/social/skill-reviews.tsx @@ -225,7 +225,8 @@ export function SkillReviews({ skillId, canInteract, onRequireLogin }: SkillRevi const [page, setPage] = useState(0) const [editing, setEditing] = useState(false) const reviews = useSkillReviews(skillId, page) - const mine = useMySkillReview(skillId, isAuthenticated && canInteract) + const mine = useMySkillReview(skillId, isAuthenticated) + const clearMine = useClearSkillReview(skillId) const canModerate = hasRole('SKILL_ADMIN') || hasRole('SUPER_ADMIN') const totalPages = reviews.data ? Math.ceil(reviews.data.total / reviews.data.size) : 0 @@ -242,6 +243,16 @@ export function SkillReviews({ skillId, canInteract, onRequireLogin }: SkillRevi setEditing(true) } + const deleteUnavailableReview = () => { + clearMine.mutate(undefined, { + onSuccess: () => { + toast.success(t('skillReviews.deleted')) + setPage(0) + }, + onError: (error) => toast.error(t('skillReviews.deleteFailed'), error.message), + }) + } + return (
@@ -258,6 +269,10 @@ export function SkillReviews({ skillId, canInteract, onRequireLogin }: SkillRevi + ) : !canInteract && isAuthenticated && mine.data?.reviewed ? ( + ) : null}