fix(review): preserve author cleanup access

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-09-01 19:13:20 +08:00
parent 92b868eb3a
commit 26dad593e1
5 changed files with 60 additions and 6 deletions

View file

@ -76,7 +76,6 @@ public class SkillReviewAppService {
String userId,
Map<Long, NamespaceRole> namespaceRoles,
Set<String> platformRoles) {
requireVisibleSkill(skillId, userId, namespaceRoles, platformRoles);
return ratingService.getUserFeedback(skillId, userId)
.map(this::toMine)
.orElseGet(SkillReviewMeResponse::empty);

View file

@ -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<? extends Throwable> expectedType) {
Throwable current = error;
while (current != null) {

View file

@ -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);

View file

@ -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(<SkillReviews skillId={10} canInteract={false} onRequireLogin={vi.fn()} />)
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()
})
})

View file

@ -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 (
<Card className="p-6 space-y-5">
<div className="flex flex-wrap items-start justify-between gap-4">
@ -258,6 +269,10 @@ export function SkillReviews({ skillId, canInteract, onRequireLogin }: SkillRevi
<Button variant="outline" onClick={startEditing}>
{mine.data?.reviewed ? t('skillReviews.edit') : t('skillReviews.write')}
</Button>
) : !canInteract && isAuthenticated && mine.data?.reviewed ? (
<Button variant="outline" onClick={deleteUnavailableReview} disabled={clearMine.isPending}>
{t('skillReviews.delete')}
</Button>
) : null}
</div>