From 92b868eb3aff97fe9d0346039f2b7779286b00f8 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 1 Sep 2026 19:09:45 +0800 Subject: [PATCH] fix(review): keep author cleanup available Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/skillhub/en/guide/social.md | 2 +- docs/skillhub/guide/social.md | 2 +- .../service/SkillReviewAppService.java | 1 - .../controller/SkillReviewControllerTest.java | 16 ++++ .../JpaSkillReviewQueryRepositoryTest.java | 22 ++++++ .../SkillRatingOptimisticLockingTest.java | 79 +++++++++++++++++++ .../service/SkillReviewAppServiceTest.java | 14 ++++ web/src/features/social/skill-reviews.tsx | 44 +++++++---- web/src/features/social/use-skill-reviews.ts | 2 +- 9 files changed, 162 insertions(+), 20 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java diff --git a/docs/skillhub/en/guide/social.md b/docs/skillhub/en/guide/social.md index 5470a893..a18df1e3 100644 --- a/docs/skillhub/en/guide/social.md +++ b/docs/skillhub/en/guide/social.md @@ -64,7 +64,7 @@ View skill packages with the most stars and highest ratings to discover best pra 1. Select "Write a review" on a published skill's detail page 2. Choose 1-5 stars and enter up to 2,000 characters -3. You can edit or clear the text; clearing it keeps the star rating +3. You can edit or clear the text; clearing it keeps the star rating and remains available if the skill is later unpublished 4. A review hidden by an administrator stays hidden after author edits and becomes public only after an administrator restores it **Viewing Notifications**: diff --git a/docs/skillhub/guide/social.md b/docs/skillhub/guide/social.md index 78ccfc16..17e9fc94 100644 --- a/docs/skillhub/guide/social.md +++ b/docs/skillhub/guide/social.md @@ -64,7 +64,7 @@ SkillHub 提供了丰富的社交功能，让团队成员可以互动、分享 1. 在已发布技能的详情页点击「写评价」 2. 选择 1-5 星并填写最多 2000 字的评价 -3. 可以修改或清空评价；清空评价不会删除星级评分 +3. 可以修改或清空评价；清空评价不会删除星级评分，即使技能之后取消发布也仍可清空自己的文字 4. 被管理员隐藏的评价在重新编辑后仍保持隐藏，只有技能管理员或超级管理员可以恢复公开 **查看通知**： 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 39d2d060..2776b556 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 @@ -96,7 +96,6 @@ public class SkillReviewAppService { String userId, Map namespaceRoles, Set platformRoles) { - requireInteractableSkill(skillId, userId, namespaceRoles, platformRoles); return toMine(ratingService.clearReview(skillId, userId)); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java index 8c593139..15a9761c 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java @@ -16,6 +16,7 @@ import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.dto.PageResponse; import com.iflytek.skillhub.dto.SkillReviewMeResponse; +import com.iflytek.skillhub.dto.SkillReviewResponse; import com.iflytek.skillhub.service.SkillReviewAppService; import java.util.List; import java.util.Set; @@ -50,6 +51,21 @@ class SkillReviewControllerTest { .andExpect(jsonPath("$.data.total").value(0)); } + @Test + void publicReviewListOmitsInternalUserAndModerationFields() throws Exception { + SkillReviewResponse review = new SkillReviewResponse( + 8L, null, "Alice", null, (short) 5, "Useful", "VISIBLE", false, + null, null, null); + when(reviewAppService.list(eq(10L), eq(null), any(), eq(Set.of()), eq(0), eq(20))) + .thenReturn(new PageResponse<>(List.of(review), 1, 0, 20)); + + mockMvc.perform(get("/api/v1/skills/10/reviews")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.items[0].displayName").value("Alice")) + .andExpect(jsonPath("$.data.items[0].userId").doesNotExist()) + .andExpect(jsonPath("$.data.items[0].moderationReason").doesNotExist()); + } + @Test void currentUserReviewRequiresAuthentication() throws Exception { mockMvc.perform(get("/api/v1/skills/10/reviews/me")) diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepositoryTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepositoryTest.java index ca477046..6a8959b9 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepositoryTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepositoryTest.java @@ -41,7 +41,29 @@ class JpaSkillReviewQueryRepositoryTest { assertThat(review.reviewText()).isEqualTo("excellent"); assertThat(review.authoredByViewer()).isTrue(); assertThat(review.status()).isEqualTo("VISIBLE"); + assertThat(review.userId()).isNull(); + assertThat(review.moderationReason()).isNull(); }); verify(userAccountRepository).findByIdIn(List.of("author-1")); } + + @Test + void administratorListingIncludesModerationDetails() { + SkillRating rating = new SkillRating(10L, "author-1", (short) 2); + rating.updateReview((short) 2, "off topic"); + rating.hideReview("admin", "Policy violation"); + PageRequest page = PageRequest.of(0, 20); + when(ratingRepository.findReviewsBySkillId(10L, page)) + .thenReturn(new PageImpl<>(List.of(rating), page, 1)); + when(userAccountRepository.findByIdIn(List.of("author-1"))).thenReturn(List.of()); + + var result = new JpaSkillReviewQueryRepository(ratingRepository, userAccountRepository) + .list(10L, "admin", true, page); + + assertThat(result.getContent()).singleElement().satisfies(review -> { + assertThat(review.userId()).isEqualTo("author-1"); + assertThat(review.moderationReason()).isEqualTo("Policy violation"); + assertThat(review.status()).isEqualTo("HIDDEN"); + }); + } } 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 new file mode 100644 index 00000000..2698beb4 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java @@ -0,0 +1,79 @@ +package com.iflytek.skillhub.repository; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.iflytek.skillhub.domain.social.SkillRating; +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; +import jakarta.persistence.OptimisticLockException; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; + +@DataJpaTest +class SkillRatingOptimisticLockingTest { + + @Autowired + private EntityManagerFactory entityManagerFactory; + + @Test + @Transactional(propagation = Propagation.NOT_SUPPORTED) + void concurrentReviewUpdatesRejectTheStaleWriter() { + Long ratingId = persistRating(); + EntityManager firstManager = entityManagerFactory.createEntityManager(); + EntityManager staleManager = entityManagerFactory.createEntityManager(); + try { + firstManager.getTransaction().begin(); + staleManager.getTransaction().begin(); + SkillRating first = firstManager.find(SkillRating.class, ratingId); + SkillRating stale = staleManager.find(SkillRating.class, ratingId); + + first.updateReview((short) 5, "First update"); + firstManager.getTransaction().commit(); + + stale.updateReview((short) 2, "Stale update"); + assertThatThrownBy(staleManager.getTransaction()::commit) + .satisfies(error -> assertThat(hasCause(error, OptimisticLockException.class)).isTrue()); + } finally { + rollbackIfActive(firstManager); + rollbackIfActive(staleManager); + firstManager.close(); + staleManager.close(); + } + } + + private Long persistRating() { + EntityManager entityManager = entityManagerFactory.createEntityManager(); + try { + entityManager.getTransaction().begin(); + SkillRating rating = new SkillRating(10L, "author", (short) 4); + rating.updateReview((short) 4, "Original review"); + entityManager.persist(rating); + entityManager.getTransaction().commit(); + return rating.getId(); + } finally { + rollbackIfActive(entityManager); + entityManager.close(); + } + } + + private void rollbackIfActive(EntityManager entityManager) { + if (entityManager.getTransaction().isActive()) { + entityManager.getTransaction().rollback(); + } + } + + private boolean hasCause(Throwable error, Class expectedType) { + Throwable current = error; + while (current != null) { + if (expectedType.isInstance(current)) { + return true; + } + current = current.getCause(); + } + return false; + } +} 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 09cef6aa..3e3f59b0 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 @@ -108,6 +108,20 @@ class SkillReviewAppServiceTest { verify(ratingService, never()).upsertReview(any(), any(), any(Short.class), any()); } + @Test + void authorCanClearReviewAfterSkillStopsBeingInteractable() { + SkillRating review = new SkillRating(10L, "author", (short) 4); + review.updateReview((short) 4, "Remove me"); + review.clearReview(); + when(ratingService.clearReview(10L, "author")).thenReturn(review); + + service.clear(10L, "author", Map.of(), Set.of()); + + verify(ratingService).clearReview(10L, "author"); + verify(skillRepository, never()).findById(any()); + verify(skillVersionRepository, never()).findById(any()); + } + @Test void hideWritesModerationAuditInSameWorkflow() { SkillRating review = new SkillRating(10L, "author", (short) 4); diff --git a/web/src/features/social/skill-reviews.tsx b/web/src/features/social/skill-reviews.tsx index a906817b..f7b99a26 100644 --- a/web/src/features/social/skill-reviews.tsx +++ b/web/src/features/social/skill-reviews.tsx @@ -30,10 +30,15 @@ function ReviewStars({ value, onChange, disabled = false }: { disabled?: boolean }) { const { t } = useTranslation() + const ratingName = useId() if (!onChange) { return ( -
+
{[1, 2, 3, 4, 5].map((score) => ( {[1, 2, 3, 4, 5].map((score) => ( - + + onChange(score)} + disabled={disabled} + aria-label={t('skillReviews.ratingOption', { score })} + /> + + ))}
) diff --git a/web/src/features/social/use-skill-reviews.ts b/web/src/features/social/use-skill-reviews.ts index 45dbfb60..5307af56 100644 --- a/web/src/features/social/use-skill-reviews.ts +++ b/web/src/features/social/use-skill-reviews.ts @@ -11,7 +11,7 @@ export interface SkillReview extends Omit { id: number - userId: string + userId?: string | null displayName: string avatarUrl?: string | null score: number