mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-15 04:18:20 +00:00
fix(review): keep author cleanup available
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
parent
c3093a3b81
commit
92b868eb3a
9 changed files with 162 additions and 20 deletions
|
|
@ -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**:
|
||||
|
|
|
|||
|
|
@ -64,7 +64,7 @@ SkillHub 提供了丰富的社交功能,让团队成员可以互动、分享
|
|||
|
||||
1. 在已发布技能的详情页点击「写评价」
|
||||
2. 选择 1-5 星并填写最多 2000 字的评价
|
||||
3. 可以修改或清空评价;清空评价不会删除星级评分
|
||||
3. 可以修改或清空评价;清空评价不会删除星级评分,即使技能之后取消发布也仍可清空自己的文字
|
||||
4. 被管理员隐藏的评价在重新编辑后仍保持隐藏,只有技能管理员或超级管理员可以恢复公开
|
||||
|
||||
**查看通知**:
|
||||
|
|
|
|||
|
|
@ -96,7 +96,6 @@ public class SkillReviewAppService {
|
|||
String userId,
|
||||
Map<Long, NamespaceRole> namespaceRoles,
|
||||
Set<String> platformRoles) {
|
||||
requireInteractableSkill(skillId, userId, namespaceRoles, platformRoles);
|
||||
return toMine(ratingService.clearReview(skillId, userId));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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"))
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<? extends Throwable> expectedType) {
|
||||
Throwable current = error;
|
||||
while (current != null) {
|
||||
if (expectedType.isInstance(current)) {
|
||||
return true;
|
||||
}
|
||||
current = current.getCause();
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -30,10 +30,15 @@ function ReviewStars({ value, onChange, disabled = false }: {
|
|||
disabled?: boolean
|
||||
}) {
|
||||
const { t } = useTranslation()
|
||||
const ratingName = useId()
|
||||
|
||||
if (!onChange) {
|
||||
return (
|
||||
<div className="flex items-center gap-1" aria-label={t('skillReviews.ratingDisplay', { score: value })}>
|
||||
<div
|
||||
className="flex items-center gap-1"
|
||||
role="img"
|
||||
aria-label={t('skillReviews.ratingDisplay', { score: value })}
|
||||
>
|
||||
{[1, 2, 3, 4, 5].map((score) => (
|
||||
<Star
|
||||
key={score}
|
||||
|
|
@ -51,21 +56,28 @@ function ReviewStars({ value, onChange, disabled = false }: {
|
|||
return (
|
||||
<div className="flex items-center gap-1" role="radiogroup" aria-label={t('skillReviews.scoreLabel')}>
|
||||
{[1, 2, 3, 4, 5].map((score) => (
|
||||
<button
|
||||
key={score}
|
||||
type="button"
|
||||
className="rounded p-0.5 transition-transform hover:scale-110"
|
||||
role="radio"
|
||||
aria-checked={score === value}
|
||||
onClick={() => onChange(score)}
|
||||
disabled={disabled}
|
||||
aria-label={t('skillReviews.ratingOption', { score })}
|
||||
>
|
||||
<Star className={cn(
|
||||
'h-4 w-4',
|
||||
score <= value ? 'fill-yellow-400 text-yellow-400' : 'text-muted-foreground/40',
|
||||
)} />
|
||||
</button>
|
||||
<span key={score}>
|
||||
<input
|
||||
id={`${ratingName}-${score}`}
|
||||
className="peer sr-only"
|
||||
type="radio"
|
||||
name={ratingName}
|
||||
value={score}
|
||||
checked={score === value}
|
||||
onChange={() => onChange(score)}
|
||||
disabled={disabled}
|
||||
aria-label={t('skillReviews.ratingOption', { score })}
|
||||
/>
|
||||
<label
|
||||
htmlFor={`${ratingName}-${score}`}
|
||||
className="block cursor-pointer rounded p-0.5 transition-transform hover:scale-110 peer-disabled:cursor-not-allowed peer-disabled:opacity-50 peer-focus-visible:outline peer-focus-visible:outline-2 peer-focus-visible:outline-offset-2 peer-focus-visible:outline-primary"
|
||||
>
|
||||
<Star aria-hidden="true" className={cn(
|
||||
'h-4 w-4',
|
||||
score <= value ? 'fill-yellow-400 text-yellow-400' : 'text-muted-foreground/40',
|
||||
)} />
|
||||
</label>
|
||||
</span>
|
||||
))}
|
||||
</div>
|
||||
)
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ export interface SkillReview extends Omit<GeneratedSkillReview,
|
|||
'id' | 'userId' | 'displayName' | 'avatarUrl' | 'score' | 'reviewText' | 'status' |
|
||||
'authoredByViewer' | 'moderationReason' | 'createdAt' | 'updatedAt'> {
|
||||
id: number
|
||||
userId: string
|
||||
userId?: string | null
|
||||
displayName: string
|
||||
avatarUrl?: string | null
|
||||
score: number
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue