From 45d341f14477a43bd506fe5c8e784e6d3702153f Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:15:52 +0800 Subject: [PATCH] feat(review): add skill comments and user feedback (#793) * feat(review): add skill review domain model Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * feat(review): expose skill reviews in API and UI Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): preserve moderation under concurrent edits Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): scope concurrent write conflicts Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): restore web build compatibility Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): keep author cleanup available Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): preserve author cleanup access Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): require review score contract Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(review): strengthen failure and concurrency coverage Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(review): tighten persistence assertions Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(review): disambiguate repository ports Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): enable request validation Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(review): align validation and postgres coverage Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(namespace): verify invalid batch has no side effects Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(web): align accessibility and plural assertions Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(i18n): require complete plural references Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): wrap editor actions on mobile Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): wrap long mobile labels Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): disable edits for archived skills Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * fix(review): enforce archived mutation guard Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --------- Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- docs/02-domain-model.md | 10 +- docs/05-business-flows.md | 4 + docs/06-api-design.md | 6 + docs/skillhub/en/guide/social.md | 27 ++ docs/skillhub/guide/social.md | 27 ++ server/skillhub-app/pom.xml | 4 + .../admin/AdminSkillReviewController.java | 59 +++ .../portal/SkillReviewController.java | 95 ++++ .../skillhub/dto/SkillReviewMeResponse.java | 19 + .../dto/SkillReviewModerationRequest.java | 7 + .../skillhub/dto/SkillReviewRequest.java | 12 + .../skillhub/dto/SkillReviewResponse.java | 19 + .../exception/GlobalExceptionHandler.java | 10 + .../JpaSkillReviewQueryRepository.java | 68 +++ .../SkillReviewQueryRepository.java | 9 + .../service/SkillReviewAppService.java | 206 +++++++++ .../migration/V47__skill_rating_reviews.sql | 15 + .../src/main/resources/messages.properties | 7 + .../src/main/resources/messages_ru.properties | 7 + .../src/main/resources/messages_zh.properties | 7 + .../controller/LocalAuthControllerTest.java | 21 +- .../NamespaceBatchMemberControllerTest.java | 16 +- .../controller/SkillReviewControllerTest.java | 188 ++++++++ ...llReviewModerationFlowIntegrationTest.java | 165 +++++++ .../exception/GlobalExceptionHandlerTest.java | 20 + .../JpaSkillReviewQueryRepositoryTest.java | 69 +++ .../SkillRatingOptimisticLockingTest.java | 196 ++++++++ .../service/SkillReviewAppServiceTest.java | 198 ++++++++ .../policy/RouteSecurityPolicyRegistry.java | 12 + .../RouteSecurityPolicyRegistryTest.java | 20 + .../exception/DomainConflictException.java | 16 + .../skill/service/SkillQueryService.java | 3 +- .../skillhub/domain/social/SkillRating.java | 96 +++- .../domain/social/SkillRatingRepository.java | 6 + .../domain/social/SkillRatingService.java | 66 +++ .../domain/social/SkillReviewStatus.java | 7 + .../skill/service/SkillQueryServiceTest.java | 51 +++ .../domain/social/SkillRatingServiceTest.java | 139 ++++++ .../infra/jpa/JpaSkillRatingRepository.java | 21 + web/src/api/generated/schema.d.ts | 426 +++++++++++++++++- .../features/social/skill-reviews.test.tsx | 187 ++++++++ web/src/features/social/skill-reviews.tsx | 329 ++++++++++++++ web/src/features/social/use-skill-reviews.ts | 131 ++++++ web/src/i18n/locales/en.json | 34 ++ web/src/i18n/locales/ru.json | 36 ++ web/src/i18n/locales/zh.json | 33 ++ web/src/i18n/ru-locale.test.ts | 14 +- web/src/pages/skill-detail.test.tsx | 14 + web/src/pages/skill-detail.tsx | 5 +- 49 files changed, 3102 insertions(+), 35 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillReviewController.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillReviewController.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewMeResponse.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewModerationRequest.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewRequest.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewResponse.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepository.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/SkillReviewQueryRepository.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillReviewAppService.java create mode 100644 server/skillhub-app/src/main/resources/db/migration/V47__skill_rating_reviews.sql create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/SkillReviewModerationFlowIntegrationTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepositoryTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/shared/exception/DomainConflictException.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillReviewStatus.java create mode 100644 web/src/features/social/skill-reviews.test.tsx create mode 100644 web/src/features/social/skill-reviews.tsx create mode 100644 web/src/features/social/use-skill-reviews.ts diff --git a/docs/02-domain-model.md b/docs/02-domain-model.md index 5aaaa4a6..f076addd 100644 --- a/docs/02-domain-model.md +++ b/docs/02-domain-model.md @@ -230,10 +230,18 @@ | skill_id | bigint | | | user_id | varchar(128) | | | score | tinyint | 1-5 | +| review_text | varchar(2000) | 可选文字评价;空值表示仅评分 | +| review_status | enum | `VISIBLE` / `HIDDEN`,隐藏不影响评分聚合 | +| moderated_by | varchar(128) | 最近一次管理操作人,nullable | +| moderated_at | datetime | 最近一次管理时间,nullable | +| moderation_reason | varchar(500) | 隐藏原因,nullable | +| lock_version | bigint | 乐观锁版本;并发编辑或治理冲突返回 409 | | created_at | datetime | | | updated_at | datetime | | -唯一约束:`(skill_id, user_id)`,每人每技能一条,可修改 +唯一约束:`(skill_id, user_id)`,每人每技能一条,可修改。删除文字评价只清空 +`review_text`,保留评分和既有治理状态;管理员隐藏评价时也保留评分,避免作者通过 +清空后重新提交绕过治理,或治理动作改变聚合分数。 ### user_account diff --git a/docs/05-business-flows.md b/docs/05-business-flows.md index 3cd6a7cb..74b206a6 100644 --- a/docs/05-business-flows.md +++ b/docs/05-business-flows.md @@ -252,6 +252,10 @@ Web 端与 CLI 保持同一发布语义,只是在交互上可提供更明确 → 异步重算 skill.rating_avg 和 rating_count(SELECT AVG + Redis 分布式锁防重复重算) ``` +文字评价复用同一条 `skill_rating` 记录:用户提交 `score + review_text` 时同步更新评分并触发 +`SkillRatedEvent`;删除评价只清空文字,保留星级评分。公开列表仅返回 `VISIBLE` 评价; +`SKILL_ADMIN` / `SUPER_ADMIN` 可隐藏或恢复评价,管理动作写入审计日志且不改变评分聚合。 + ## 7 异步事件汇总 | 事件 | 触发时机 | 消费方 | diff --git a/docs/06-api-design.md b/docs/06-api-design.md index 56cb5541..a0f580ae 100644 --- a/docs/06-api-design.md +++ b/docs/06-api-design.md @@ -82,6 +82,7 @@ | GET | `/api/v1/skills/{namespace}/{slug}/tags/{tagName}/download` | 按标签下载(解析标签指向的版本后下载) | | GET | `/api/v1/skills/{namespace}/{slug}/tags/{tagName}/files` | 按标签查看文件清单 | | GET | `/api/v1/skills/{namespace}/{slug}/tags/{tagName}/file?path=...` | 按标签读取单个文件 | +| GET | `/api/v1/skills/{skillId}/reviews` | 公开评价分页列表;仅返回可见评价,管理员可见隐藏项 | | GET | `/api/v1/namespaces` | 公开命名空间列表 | | GET | `/api/v1/namespaces/{slug}` | 命名空间详情 | @@ -201,6 +202,9 @@ Public API 的可见性规则: | POST | `/api/v1/skills/{namespace}/{slug}/star` | 收藏 | | DELETE | `/api/v1/skills/{namespace}/{slug}/star` | 取消收藏 | | POST | `/api/v1/skills/{namespace}/{slug}/rating` | 评分 | +| GET | `/api/v1/skills/{skillId}/reviews/me` | 当前用户的评分与文字评价 | +| PUT | `/api/v1/skills/{skillId}/reviews/me` | 新增或更新当前用户评价(`score` 1-5,`reviewText` 最长 2000) | +| DELETE | `/api/v1/skills/{skillId}/reviews/me` | 删除文字评价并保留星级评分 | | GET | `/api/v1/me/stars` | 我的收藏列表 | | GET | `/api/v1/me/skills` | 我发布的技能列表 | @@ -312,6 +316,8 @@ Admin API 按最小权限拆分,不再统一要求 SUPER_ADMIN: | POST | `/api/v1/admin/skills/{id}/hide` | 隐藏技能(仅 `SUPER_ADMIN`) | | POST | `/api/v1/admin/skills/{id}/unhide` | 恢复技能(仅 `SUPER_ADMIN`) | | POST | `/api/v1/admin/skills/versions/{versionId}/yank` | 撤回已发布版本(`SKILL_ADMIN` / `SUPER_ADMIN`) | +| POST | `/api/v1/admin/skill-reviews/{reviewId}/hide` | 隐藏用户评价(`SKILL_ADMIN` / `SUPER_ADMIN`) | +| POST | `/api/v1/admin/skill-reviews/{reviewId}/restore` | 恢复用户评价(`SKILL_ADMIN` / `SUPER_ADMIN`) | ### 用户治理(需 USER_ADMIN / SUPER_ADMIN) diff --git a/docs/skillhub/en/guide/social.md b/docs/skillhub/en/guide/social.md index a8c4128f..a18df1e3 100644 --- a/docs/skillhub/en/guide/social.md +++ b/docs/skillhub/en/guide/social.md @@ -60,6 +60,13 @@ View skill packages with the most stars and highest ratings to discover best pra 3. The rating takes effect immediately and impacts the skill package's average rating 4. You can update your rating at any time +**Writing a Review**: + +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 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**: 1. Click the notification icon in the top navigation bar @@ -125,6 +132,24 @@ GET /api/v1/me/stars?page=0&size=20 GET /api/v1/skills/{skillId}/rating ``` +**Review APIs**: + +```bash +# Public review list +GET /api/v1/skills/{skillId}/reviews?page=0&size=20 + +# Read, create, or update the current user's review +GET /api/v1/skills/{skillId}/reviews/me +PUT /api/v1/skills/{skillId}/reviews/me + +# Clear review text while retaining the star rating +DELETE /api/v1/skills/{skillId}/reviews/me + +# SKILL_ADMIN or SUPER_ADMIN moderation +POST /api/v1/admin/skill-reviews/{reviewId}/hide +POST /api/v1/admin/skill-reviews/{reviewId}/restore +``` + **Response Example**: ```json { @@ -137,6 +162,8 @@ GET /api/v1/skills/{skillId}/rating > **Rating Rules**: Each user can rate each skill package only once. Ratings can be updated but not deleted. +> **Review Rules**: Only published skills can be reviewed, and the public list contains visible reviews only. Refresh and retry after a concurrent-update conflict. + - **Star Count**: A skill package's star count is displayed in search results and on the detail page - **Average Rating**: A skill package's average rating affects search ranking - **Notification Settings**: Users can disable certain notification types in their settings diff --git a/docs/skillhub/guide/social.md b/docs/skillhub/guide/social.md index 6fc733ba..17e9fc94 100644 --- a/docs/skillhub/guide/social.md +++ b/docs/skillhub/guide/social.md @@ -60,6 +60,13 @@ SkillHub 提供了丰富的社交功能,让团队成员可以互动、分享 3. 评分会立即生效,影响技能包的平均评分 4. 可以随时修改评分 +**撰写评价**: + +1. 在已发布技能的详情页点击「写评价」 +2. 选择 1-5 星并填写最多 2000 字的评价 +3. 可以修改或清空评价;清空评价不会删除星级评分,即使技能之后取消发布也仍可清空自己的文字 +4. 被管理员隐藏的评价在重新编辑后仍保持隐藏,只有技能管理员或超级管理员可以恢复公开 + **查看通知**: 1. 点击顶部导航栏的通知图标 @@ -125,6 +132,24 @@ GET /api/v1/me/stars?page=0&size=20 GET /api/v1/skills/{skillId}/rating ``` +**评价接口**: + +```bash +# 公开评价列表 +GET /api/v1/skills/{skillId}/reviews?page=0&size=20 + +# 查看、创建或修改自己的评价 +GET /api/v1/skills/{skillId}/reviews/me +PUT /api/v1/skills/{skillId}/reviews/me + +# 清空评价文字并保留星级评分 +DELETE /api/v1/skills/{skillId}/reviews/me + +# 技能管理员或超级管理员隐藏、恢复评价 +POST /api/v1/admin/skill-reviews/{reviewId}/hide +POST /api/v1/admin/skill-reviews/{reviewId}/restore +``` + **响应示例**: ```json { @@ -137,6 +162,8 @@ GET /api/v1/skills/{skillId}/rating > **评分规则**:每个用户对每个技能包只能评分一次,可以修改评分但不能删除。 +> **评价规则**:只有已发布技能可以评价;公开列表只显示可见评价。并发修改发生冲突时,刷新详情后重试。 + - **星标数量**:技能包的星标数会显示在搜索结果和详情页 - **平均评分**:技能包的平均评分会影响搜索排序 - **通知设置**:用户可以在设置中关闭某些类型的通知 diff --git a/server/skillhub-app/pom.xml b/server/skillhub-app/pom.xml index a946bf3f..876d9ddc 100644 --- a/server/skillhub-app/pom.xml +++ b/server/skillhub-app/pom.xml @@ -22,6 +22,10 @@ org.springframework.boot spring-boot-starter-web + + org.springframework.boot + spring-boot-starter-validation + org.springframework.boot spring-boot-starter-actuator diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillReviewController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillReviewController.java new file mode 100644 index 00000000..791da2f2 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSkillReviewController.java @@ -0,0 +1,59 @@ +package com.iflytek.skillhub.controller.admin; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.controller.BaseApiController; +import com.iflytek.skillhub.dto.ApiResponse; +import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.dto.SkillReviewModerationRequest; +import com.iflytek.skillhub.dto.SkillReviewResponse; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.SkillReviewAppService; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.validation.Valid; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("/api/v1/admin/skill-reviews") +@PreAuthorize("hasAnyRole('SKILL_ADMIN', 'SUPER_ADMIN')") +public class AdminSkillReviewController extends BaseApiController { + + private final SkillReviewAppService reviewAppService; + + public AdminSkillReviewController(ApiResponseFactory responseFactory, + SkillReviewAppService reviewAppService) { + super(responseFactory); + this.reviewAppService = reviewAppService; + } + + @PostMapping("/{reviewId}/hide") + public ApiResponse hide( + @PathVariable Long reviewId, + @Valid @RequestBody(required = false) SkillReviewModerationRequest request, + @AuthenticationPrincipal PlatformPrincipal principal, + HttpServletRequest httpRequest) { + return ok("response.success.updated", reviewAppService.hide( + reviewId, + principal.userId(), + request != null ? request.reason() : null, + AuditRequestContext.from(httpRequest) + )); + } + + @PostMapping("/{reviewId}/restore") + public ApiResponse restore( + @PathVariable Long reviewId, + @AuthenticationPrincipal PlatformPrincipal principal, + HttpServletRequest httpRequest) { + return ok("response.success.updated", reviewAppService.restore( + reviewId, + principal.userId(), + AuditRequestContext.from(httpRequest) + )); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillReviewController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillReviewController.java new file mode 100644 index 00000000..b29a1d9f --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillReviewController.java @@ -0,0 +1,95 @@ +package com.iflytek.skillhub.controller.portal; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.controller.BaseApiController; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.dto.ApiResponse; +import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.dto.SkillReviewMeResponse; +import com.iflytek.skillhub.dto.SkillReviewRequest; +import com.iflytek.skillhub.dto.SkillReviewResponse; +import com.iflytek.skillhub.service.SkillReviewAppService; +import jakarta.validation.Valid; +import java.util.Map; +import java.util.Set; +import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.RequestAttribute; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping({"/api/v1/skills", "/api/web/skills"}) +public class SkillReviewController extends BaseApiController { + + private final SkillReviewAppService reviewAppService; + + public SkillReviewController(ApiResponseFactory responseFactory, + SkillReviewAppService reviewAppService) { + super(responseFactory); + this.reviewAppService = reviewAppService; + } + + @GetMapping("/{skillId}/reviews") + public ApiResponse> list( + @PathVariable Long skillId, + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size, + @AuthenticationPrincipal PlatformPrincipal principal, + @RequestAttribute(value = "userNsRoles", required = false) Map namespaceRoles) { + return ok("response.success.read", reviewAppService.list( + skillId, + principal != null ? principal.userId() : null, + namespaceRoles, + roles(principal), + page, + size + )); + } + + @GetMapping("/{skillId}/reviews/me") + public ApiResponse getMine( + @PathVariable Long skillId, + @AuthenticationPrincipal PlatformPrincipal principal, + @RequestAttribute(value = "userNsRoles", required = false) Map namespaceRoles) { + return ok("response.success.read", reviewAppService.getMine( + skillId, principal.userId(), namespaceRoles, roles(principal))); + } + + @PutMapping("/{skillId}/reviews/me") + public ApiResponse upsert( + @PathVariable Long skillId, + @Valid @RequestBody SkillReviewRequest request, + @AuthenticationPrincipal PlatformPrincipal principal, + @RequestAttribute(value = "userNsRoles", required = false) Map namespaceRoles) { + return ok("response.success.updated", reviewAppService.upsert( + skillId, + principal.userId(), + request.score(), + request.reviewText(), + namespaceRoles, + roles(principal) + )); + } + + @DeleteMapping("/{skillId}/reviews/me") + public ApiResponse clear( + @PathVariable Long skillId, + @AuthenticationPrincipal PlatformPrincipal principal, + @RequestAttribute(value = "userNsRoles", required = false) Map namespaceRoles) { + return ok("response.success.updated", reviewAppService.clear( + skillId, principal.userId(), namespaceRoles, roles(principal))); + } + + private Set roles(PlatformPrincipal principal) { + return principal != null && principal.platformRoles() != null + ? principal.platformRoles() + : Set.of(); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewMeResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewMeResponse.java new file mode 100644 index 00000000..ecafd42a --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewMeResponse.java @@ -0,0 +1,19 @@ +package com.iflytek.skillhub.dto; + +import java.time.Instant; + +public record SkillReviewMeResponse( + boolean rated, + short score, + boolean reviewed, + Long reviewId, + String reviewText, + String status, + String moderationReason, + Instant createdAt, + Instant updatedAt +) { + public static SkillReviewMeResponse empty() { + return new SkillReviewMeResponse(false, (short) 0, false, null, null, null, null, null, null); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewModerationRequest.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewModerationRequest.java new file mode 100644 index 00000000..f1ebc960 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewModerationRequest.java @@ -0,0 +1,7 @@ +package com.iflytek.skillhub.dto; + +import jakarta.validation.constraints.Size; + +public record SkillReviewModerationRequest( + @Size(max = 500) String reason +) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewRequest.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewRequest.java new file mode 100644 index 00000000..1ca50751 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewRequest.java @@ -0,0 +1,12 @@ +package com.iflytek.skillhub.dto; + +import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Size; + +public record SkillReviewRequest( + @NotNull @Min(1) @Max(5) Short score, + @NotBlank @Size(max = 2000) String reviewText +) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewResponse.java new file mode 100644 index 00000000..9c4156b1 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillReviewResponse.java @@ -0,0 +1,19 @@ +package com.iflytek.skillhub.dto; + +import com.fasterxml.jackson.annotation.JsonInclude; +import java.time.Instant; + +@JsonInclude(JsonInclude.Include.NON_NULL) +public record SkillReviewResponse( + Long id, + String userId, + String displayName, + String avatarUrl, + short score, + String reviewText, + String status, + boolean authoredByViewer, + String moderationReason, + Instant createdAt, + Instant updatedAt +) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java index e2b90929..ba93bb1d 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java @@ -17,6 +17,7 @@ import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.orm.ObjectOptimisticLockingFailureException; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.Authentication; import org.springframework.validation.FieldError; @@ -68,6 +69,15 @@ public class GlobalExceptionHandler { return renderLocalizedError(ex, HttpStatus.valueOf(ex.statusCode()), request); } + @ExceptionHandler(ObjectOptimisticLockingFailureException.class) + public ResponseEntity> handlePersistenceConflict( + RuntimeException ex, + HttpServletRequest request) { + logHandledException(HttpStatus.CONFLICT, "error.request.conflict", request); + return ResponseEntity.status(HttpStatus.CONFLICT).body( + apiResponseFactory.error(409, "error.request.conflict")); + } + @ExceptionHandler(MethodArgumentNotValidException.class) public ResponseEntity> handleValidation(MethodArgumentNotValidException ex, HttpServletRequest request) { String msg = ex.getBindingResult().getFieldErrors().stream() diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepository.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepository.java new file mode 100644 index 00000000..886483a5 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepository.java @@ -0,0 +1,68 @@ +package com.iflytek.skillhub.repository; + +import com.iflytek.skillhub.domain.social.SkillRating; +import com.iflytek.skillhub.domain.social.SkillRatingRepository; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.dto.SkillReviewResponse; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; +import org.springframework.stereotype.Repository; + +@Repository +public class JpaSkillReviewQueryRepository implements SkillReviewQueryRepository { + + private final SkillRatingRepository ratingRepository; + private final UserAccountRepository userAccountRepository; + + public JpaSkillReviewQueryRepository(SkillRatingRepository ratingRepository, + UserAccountRepository userAccountRepository) { + this.ratingRepository = ratingRepository; + this.userAccountRepository = userAccountRepository; + } + + @Override + public Page list(Long skillId, + String viewerId, + boolean includeHidden, + Pageable pageable) { + Page reviews = includeHidden + ? ratingRepository.findReviewsBySkillId(skillId, pageable) + : ratingRepository.findVisibleReviewsBySkillId(skillId, pageable); + List authorIds = reviews.getContent().stream() + .map(SkillRating::getUserId) + .distinct() + .toList(); + Map authors = userAccountRepository.findByIdIn(authorIds).stream() + .collect(Collectors.toMap(UserAccount::getId, Function.identity())); + return reviews.map(review -> toResponse( + review, + authors.get(review.getUserId()), + viewerId, + includeHidden + )); + } + + private SkillReviewResponse toResponse(SkillRating review, + UserAccount author, + String viewerId, + boolean includeModerationDetails) { + return new SkillReviewResponse( + review.getId(), + includeModerationDetails ? review.getUserId() : null, + author != null ? author.getDisplayName() : review.getUserId(), + author != null ? author.getAvatarUrl() : null, + review.getScore(), + review.getReviewText(), + review.getReviewStatus().name(), + review.getUserId().equals(viewerId), + includeModerationDetails ? review.getModerationReason() : null, + review.getCreatedAt(), + review.getUpdatedAt() + ); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/SkillReviewQueryRepository.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/SkillReviewQueryRepository.java new file mode 100644 index 00000000..8a22206c --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/SkillReviewQueryRepository.java @@ -0,0 +1,9 @@ +package com.iflytek.skillhub.repository; + +import com.iflytek.skillhub.dto.SkillReviewResponse; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; + +public interface SkillReviewQueryRepository { + Page list(Long skillId, String viewerId, boolean includeHidden, Pageable pageable); +} 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 new file mode 100644 index 00000000..49fba830 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillReviewAppService.java @@ -0,0 +1,206 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.audit.AuditDetail; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillStatus; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.VisibilityChecker; +import com.iflytek.skillhub.domain.social.SkillRating; +import com.iflytek.skillhub.domain.social.SkillRatingService; +import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.dto.SkillReviewMeResponse; +import com.iflytek.skillhub.dto.SkillReviewResponse; +import com.iflytek.skillhub.observability.RequestIdAccessor; +import com.iflytek.skillhub.repository.SkillReviewQueryRepository; +import java.util.Map; +import java.util.Set; +import org.springframework.data.domain.PageRequest; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +@Service +public class SkillReviewAppService { + + private static final int MAX_PAGE_SIZE = 100; + + private final SkillRepository skillRepository; + private final SkillVersionRepository skillVersionRepository; + private final VisibilityChecker visibilityChecker; + private final SkillRatingService ratingService; + private final SkillReviewQueryRepository queryRepository; + private final AuditLogService auditLogService; + private final RequestIdAccessor requestIdAccessor; + + public SkillReviewAppService(SkillRepository skillRepository, + SkillVersionRepository skillVersionRepository, + VisibilityChecker visibilityChecker, + SkillRatingService ratingService, + SkillReviewQueryRepository queryRepository, + AuditLogService auditLogService, + RequestIdAccessor requestIdAccessor) { + this.skillRepository = skillRepository; + this.skillVersionRepository = skillVersionRepository; + this.visibilityChecker = visibilityChecker; + this.ratingService = ratingService; + this.queryRepository = queryRepository; + this.auditLogService = auditLogService; + this.requestIdAccessor = requestIdAccessor; + } + + public PageResponse list(Long skillId, + String viewerId, + Map namespaceRoles, + Set platformRoles, + int page, + int size) { + requireVisibleSkill(skillId, viewerId, namespaceRoles, platformRoles); + if (page < 0 || size < 1 || size > MAX_PAGE_SIZE) { + throw new DomainBadRequestException("error.pagination.invalid", MAX_PAGE_SIZE); + } + boolean includeHidden = isReviewModerator(platformRoles); + return PageResponse.from(queryRepository.list( + skillId, + viewerId, + includeHidden, + PageRequest.of(page, size) + )); + } + + public SkillReviewMeResponse getMine(Long skillId, + String userId, + Map namespaceRoles, + Set platformRoles) { + return ratingService.getUserFeedback(skillId, userId) + .map(this::toMine) + .orElseGet(SkillReviewMeResponse::empty); + } + + public SkillReviewMeResponse upsert(Long skillId, + String userId, + short score, + String reviewText, + Map namespaceRoles, + Set platformRoles) { + requireInteractableSkill(skillId, userId, namespaceRoles, platformRoles); + return toMine(ratingService.upsertReview(skillId, userId, score, reviewText)); + } + + public SkillReviewMeResponse clear(Long skillId, + String userId, + Map namespaceRoles, + Set platformRoles) { + return toMine(ratingService.clearReview(skillId, userId)); + } + + @Transactional + public SkillReviewResponse hide(Long reviewId, + String moderatorId, + String reason, + AuditRequestContext auditContext) { + SkillRating review = ratingService.hideReview(reviewId, moderatorId, reason); + recordModerationAudit("SKILL_REVIEW_HIDE", review, moderatorId, reason, auditContext); + return toModerationResponse(review); + } + + @Transactional + public SkillReviewResponse restore(Long reviewId, + String moderatorId, + AuditRequestContext auditContext) { + SkillRating review = ratingService.restoreReview(reviewId, moderatorId); + recordModerationAudit("SKILL_REVIEW_RESTORE", review, moderatorId, null, auditContext); + return toModerationResponse(review); + } + + private Skill requireVisibleSkill(Long skillId, + String userId, + Map namespaceRoles, + Set platformRoles) { + Skill skill = skillRepository.findById(skillId) + .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillId)); + if (!visibilityChecker.canAccess( + skill, + userId, + namespaceRoles != null ? namespaceRoles : Map.of(), + platformRoles != null ? platformRoles : Set.of())) { + throw new DomainForbiddenException("error.skill.access.denied", skill.getSlug()); + } + return skill; + } + + private Skill requireInteractableSkill(Long skillId, + String userId, + Map namespaceRoles, + Set platformRoles) { + Skill skill = requireVisibleSkill(skillId, userId, namespaceRoles, platformRoles); + boolean published = skill.getLatestVersionId() != null + && skillVersionRepository.findById(skill.getLatestVersionId()) + .map(version -> version.getStatus() == SkillVersionStatus.PUBLISHED) + .orElse(false); + if (skill.getStatus() != SkillStatus.ACTIVE || !published) { + throw new DomainBadRequestException("error.skillReview.notInteractable"); + } + return skill; + } + + private SkillReviewMeResponse toMine(SkillRating review) { + return new SkillReviewMeResponse( + true, + review.getScore(), + review.hasReview(), + review.hasReview() ? review.getId() : null, + review.getReviewText(), + review.hasReview() ? review.getReviewStatus().name() : null, + review.getModerationReason(), + review.getCreatedAt(), + review.getUpdatedAt() + ); + } + + private SkillReviewResponse toModerationResponse(SkillRating review) { + return new SkillReviewResponse( + review.getId(), + review.getUserId(), + review.getUserId(), + null, + review.getScore(), + review.getReviewText(), + review.getReviewStatus().name(), + false, + review.getModerationReason(), + review.getCreatedAt(), + review.getUpdatedAt() + ); + } + + private boolean isReviewModerator(Set platformRoles) { + return platformRoles != null + && (platformRoles.contains("SKILL_ADMIN") || platformRoles.contains("SUPER_ADMIN")); + } + + private void recordModerationAudit(String action, + SkillRating review, + String moderatorId, + String reason, + AuditRequestContext context) { + auditLogService.record( + moderatorId, + action, + "SKILL_REVIEW", + review.getId(), + requestIdAccessor.current(), + context != null ? context.clientIp() : null, + context != null ? context.userAgent() : null, + AuditDetail.builder() + .put("skillId", review.getSkillId()) + .put("reason", reason) + .build() + ); + } +} diff --git a/server/skillhub-app/src/main/resources/db/migration/V47__skill_rating_reviews.sql b/server/skillhub-app/src/main/resources/db/migration/V47__skill_rating_reviews.sql new file mode 100644 index 00000000..2fdaec7f --- /dev/null +++ b/server/skillhub-app/src/main/resources/db/migration/V47__skill_rating_reviews.sql @@ -0,0 +1,15 @@ +ALTER TABLE skill_rating + ADD COLUMN review_text VARCHAR(2000), + ADD COLUMN review_status VARCHAR(16) NOT NULL DEFAULT 'VISIBLE', + ADD COLUMN moderated_by VARCHAR(128) REFERENCES user_account(id), + ADD COLUMN moderated_at TIMESTAMPTZ, + ADD COLUMN moderation_reason VARCHAR(500), + ADD COLUMN lock_version BIGINT NOT NULL DEFAULT 0, + ADD CONSTRAINT chk_skill_rating_review_status + CHECK (review_status IN ('VISIBLE', 'HIDDEN')); + +CREATE INDEX idx_skill_rating_visible_reviews + ON skill_rating(skill_id, updated_at DESC, id DESC) + WHERE review_status = 'VISIBLE' + AND review_text IS NOT NULL + AND BTRIM(review_text) <> ''; diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index c350aff7..c6d80a2a 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -187,3 +187,10 @@ promotion.sort.field.invalid=Unsupported promotion sort field: {0} promotion.sort.direction.invalid=Unsupported promotion sort direction: {0} promotion.sort.pending_unsupported=Pending promotion requests do not support reviewed-time sorting error.skill.subscription.noPermission=You do not have permission to subscribe to this skill. +error.skillReview.notFound=Skill review not found +error.skillReview.text.required=Review text is required +error.skillReview.text.tooLong=Review text must not exceed {0} characters +error.skillReview.reason.tooLong=Moderation reason must not exceed {0} characters +error.pagination.invalid=Page must be non-negative and size must be between 1 and {0} +error.request.conflict=The data changed while this request was being processed. Refresh and try again. +error.skillReview.notInteractable=Reviews are available only for published skills diff --git a/server/skillhub-app/src/main/resources/messages_ru.properties b/server/skillhub-app/src/main/resources/messages_ru.properties index 8ea9d823..0302c2f5 100644 --- a/server/skillhub-app/src/main/resources/messages_ru.properties +++ b/server/skillhub-app/src/main/resources/messages_ru.properties @@ -177,3 +177,10 @@ promotion.status.invalid=Неподдерживаемый статус прод promotion.sort.field.invalid=Неподдерживаемое поле сортировки продвижения: {0} promotion.sort.direction.invalid=Неподдерживаемое направление сортировки продвижения: {0} promotion.sort.pending_unsupported=Ожидающие заявки на продвижение не поддерживают сортировку по времени ревью +error.skillReview.notFound=Отзыв о скилле не найден +error.skillReview.text.required=Текст отзыва обязателен +error.skillReview.text.tooLong=Текст отзыва не должен превышать {0} символов +error.skillReview.reason.tooLong=Причина модерации не должна превышать {0} символов +error.pagination.invalid=Номер страницы не может быть отрицательным, а размер должен быть от 1 до {0} +error.request.conflict=Данные изменились во время обработки запроса. Обновите страницу и повторите попытку. +error.skillReview.notInteractable=Отзывы доступны только для опубликованных навыков diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 25b879ef..8b3e7a66 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -187,3 +187,10 @@ promotion.sort.field.invalid=不支持的提升审核排序字段:{0} promotion.sort.direction.invalid=不支持的提升审核排序方向:{0} promotion.sort.pending_unsupported=待审核提升请求不支持按处理时间排序 error.skill.subscription.noPermission=您没有订阅此技能的权限。 +error.skillReview.notFound=未找到技能评价 +error.skillReview.text.required=评价内容不能为空 +error.skillReview.text.tooLong=评价内容不能超过 {0} 个字符 +error.skillReview.reason.tooLong=管理原因不能超过 {0} 个字符 +error.pagination.invalid=页码不能为负数,每页数量必须在 1 到 {0} 之间 +error.request.conflict=数据在请求处理期间已发生变化,请刷新后重试 +error.skillReview.notInteractable=仅已发布的技能可以评价 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java index 2425acde..50082983 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/LocalAuthControllerTest.java @@ -1,7 +1,6 @@ package com.iflytek.skillhub.controller; import static org.mockito.BDDMockito.given; -import static org.mockito.BDDMockito.willThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; @@ -116,9 +115,6 @@ class LocalAuthControllerTest { @Test void register_rejectsInvalidEmailFormat() throws Exception { - given(localAuthService.register("bob", "Abcd123!", "not-an-email")) - .willThrow(new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.local.email.invalid")); - mockMvc.perform(post("/api/v1/auth/local/register") .with(csrf()) .header("Accept-Language", "zh-CN") @@ -129,14 +125,11 @@ class LocalAuthControllerTest { .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value(400)); - verify(localAuthService).register("bob", "Abcd123!", "not-an-email"); + verify(localAuthService, never()).register("bob", "Abcd123!", "not-an-email"); } @Test void register_rejectsBlankEmail() throws Exception { - given(localAuthService.register("bob", "Abcd123!", " ")) - .willThrow(new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.local.email.notBlank")); - mockMvc.perform(post("/api/v1/auth/local/register") .with(csrf()) .contentType(MediaType.APPLICATION_JSON) @@ -146,7 +139,7 @@ class LocalAuthControllerTest { .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value(400)); - verify(localAuthService).register("bob", "Abcd123!", " "); + verify(localAuthService, never()).register("bob", "Abcd123!", " "); } @Test @@ -277,9 +270,6 @@ class LocalAuthControllerTest { @Test void requestPasswordReset_rejectsInvalidEmailFormat() throws Exception { - willThrow(new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.password.reset.email.invalid")) - .given(passwordResetService).requestPasswordReset("alice"); - mockMvc.perform(post("/api/v1/auth/local/password-reset/request") .with(csrf()) .contentType(MediaType.APPLICATION_JSON) @@ -289,7 +279,7 @@ class LocalAuthControllerTest { .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value(400)); - verify(passwordResetService).requestPasswordReset("alice"); + verify(passwordResetService, never()).requestPasswordReset("alice"); } @Test @@ -308,9 +298,6 @@ class LocalAuthControllerTest { @Test void confirmPasswordReset_rejectsInvalidEmailFormat() throws Exception { - willThrow(new AuthFlowException(HttpStatus.BAD_REQUEST, "validation.auth.password.reset.email.invalid")) - .given(passwordResetService).confirmPasswordReset("alice", "123456", "Abcd123!"); - mockMvc.perform(post("/api/v1/auth/local/password-reset/confirm") .with(csrf()) .contentType(MediaType.APPLICATION_JSON) @@ -320,7 +307,7 @@ class LocalAuthControllerTest { .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.code").value(400)); - verify(passwordResetService).confirmPasswordReset("alice", "123456", "Abcd123!"); + verify(passwordResetService, never()).confirmPasswordReset("alice", "123456", "Abcd123!"); } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespaceBatchMemberControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespaceBatchMemberControllerTest.java index aa4a542b..8ac33168 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespaceBatchMemberControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NamespaceBatchMemberControllerTest.java @@ -16,11 +16,13 @@ import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.service.NamespaceMemberCandidateService; +import com.iflytek.skillhub.service.NamespacePortalCommandAppService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.http.MediaType; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.authority.SimpleGrantedAuthority; @@ -35,6 +37,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -67,6 +71,9 @@ class NamespaceBatchMemberControllerTest { @MockBean private NamespaceMemberCandidateService namespaceMemberCandidateService; + @SpyBean + private NamespacePortalCommandAppService namespacePortalCommandAppService; + @MockBean private DeviceAuthService deviceAuthService; @@ -187,9 +194,7 @@ class NamespaceBatchMemberControllerTest { @Test void batchAddMembers_emptyArray_returnsError() throws Exception { - // @NotEmpty on BatchMemberRequest.members triggers validation error - // Spring Boot 3.2+ raises HandlerMethodValidationException (500) rather than - // MethodArgumentNotValidException (400) for record-based @RequestBody validation + // @NotEmpty on BatchMemberRequest.members is enforced before the controller runs. mockMvc.perform(post("/api/v1/namespaces/team-a/members/batch") .with(csrf()) .with(auth("owner-1")) @@ -198,7 +203,10 @@ class NamespaceBatchMemberControllerTest { .content(""" {"members":[]} """)) - .andExpect(status().isInternalServerError()); + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + + verify(namespacePortalCommandAppService, never()).batchAddMembers(any(), any(), any()); } @Test 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 new file mode 100644 index 00000000..7bc413c3 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillReviewControllerTest.java @@ -0,0 +1,188 @@ +package com.iflytek.skillhub.controller; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +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; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.MediaType; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +class SkillReviewControllerTest { + + @Autowired private MockMvc mockMvc; + @MockBean private SkillReviewAppService reviewAppService; + @MockBean private NamespaceMemberRepository namespaceMemberRepository; + + @Test + void publicReviewListIsAnonymous() throws Exception { + when(reviewAppService.list(eq(10L), eq(null), any(), eq(Set.of()), eq(0), eq(20))) + .thenReturn(new PageResponse<>(List.of(), 0, 0, 20)); + + mockMvc.perform(get("/api/v1/skills/10/reviews")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .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")) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + } + + @Test + void anonymousUserCannotUpsertOrClearReview() throws Exception { + mockMvc.perform(put("/api/v1/skills/10/reviews/me") + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"score\":4,\"reviewText\":\"Useful\"}")) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + + mockMvc.perform(delete("/api/v1/skills/10/reviews/me") + .with(csrf())) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.code").value(401)); + } + + @Test + void authenticatedUserCanUpsertReview() throws Exception { + var principal = principal("user-42", Set.of()); + when(reviewAppService.upsert(eq(10L), eq("user-42"), eq((short) 4), eq("Useful"), any(), eq(Set.of()))) + .thenReturn(new SkillReviewMeResponse( + true, (short) 4, true, 8L, "Useful", "VISIBLE", null, null, null)); + + mockMvc.perform(put("/api/v1/skills/10/reviews/me") + .with(authentication(authToken(principal, "ROLE_USER"))) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"score\":4,\"reviewText\":\"Useful\"}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.reviewed").value(true)) + .andExpect(jsonPath("$.data.reviewText").value("Useful")); + + verify(reviewAppService).upsert(eq(10L), eq("user-42"), eq((short) 4), eq("Useful"), any(), eq(Set.of())); + } + + @Test + void reviewScoreIsRequiredByTheApiContract() throws Exception { + var principal = principal("user-42", Set.of()); + + mockMvc.perform(put("/api/v1/skills/10/reviews/me") + .with(authentication(authToken(principal, "ROLE_USER"))) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"reviewText\":\"Useful\"}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + } + + @Test + void reviewTextIsRequiredByTheApiContract() throws Exception { + var principal = principal("user-42", Set.of()); + + mockMvc.perform(put("/api/v1/skills/10/reviews/me") + .with(authentication(authToken(principal, "ROLE_USER"))) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"score\":4}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + } + + @Test + void skillAdminCanHideReview() throws Exception { + var principal = principal("admin", Set.of("SKILL_ADMIN")); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/8/hide") + .with(authentication(authToken(principal, "ROLE_SKILL_ADMIN"))) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"reason\":\"spam\"}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)); + + verify(reviewAppService).hide(eq(8L), eq("admin"), eq("spam"), any()); + } + + @Test + void ordinaryUserCannotHideReview() throws Exception { + var principal = principal("user-42", Set.of()); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/8/hide") + .with(authentication(authToken(principal, "ROLE_USER"))) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{}")) + .andExpect(status().isForbidden()); + } + + @Test + void moderationReasonRejectsMoreThanFiveHundredCharacters() throws Exception { + var principal = principal("admin", Set.of("SKILL_ADMIN")); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/8/hide") + .with(authentication(authToken(principal, "ROLE_SKILL_ADMIN"))) + .with(csrf()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"reason\":\"" + "x".repeat(501) + "\"}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + } + + private PlatformPrincipal principal(String userId, Set roles) { + return new PlatformPrincipal(userId, userId, userId + "@example.test", null, "local", roles); + } + + private UsernamePasswordAuthenticationToken authToken(PlatformPrincipal principal, String role) { + return new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority(role)) + ); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/SkillReviewModerationFlowIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/SkillReviewModerationFlowIntegrationTest.java new file mode 100644 index 00000000..c93e07a2 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/admin/SkillReviewModerationFlowIntegrationTest.java @@ -0,0 +1,165 @@ +package com.iflytek.skillhub.controller.admin; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.iflytek.skillhub.SkillhubApplication; +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.audit.AuditLog; +import com.iflytek.skillhub.domain.audit.AuditLogRepository; +import com.iflytek.skillhub.domain.governance.GovernanceNotificationService; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.social.SkillRating; +import com.iflytek.skillhub.domain.social.SkillRatingRepository; +import com.iflytek.skillhub.domain.social.SkillReviewStatus; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.infra.jpa.AuditLogJpaRepository; +import com.iflytek.skillhub.infra.jpa.JpaSkillRatingRepository; +import com.iflytek.skillhub.infra.jpa.NamespaceJpaRepository; +import com.iflytek.skillhub.infra.jpa.SkillJpaRepository; +import com.iflytek.skillhub.infra.jpa.UserAccountJpaRepository; +import com.iflytek.skillhub.notification.service.NotificationDispatcher; +import java.util.List; +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.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.context.annotation.Import; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +@SpringBootTest(classes = SkillhubApplication.class) +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class SkillReviewModerationFlowIntegrationTest { + + private static final String ADMIN_ID = "review-admin"; + private static final String AUTHOR_ID = "review-author"; + + @Autowired private MockMvc mockMvc; + @Autowired private UserAccountJpaRepository userAccountRepository; + @Autowired private NamespaceJpaRepository namespaceRepository; + @Autowired private SkillJpaRepository skillRepository; + @Autowired private SkillRatingRepository ratingRepository; + @Autowired private JpaSkillRatingRepository ratingJpaRepository; + + @Autowired private AuditLogRepository auditLogRepository; + @SpyBean private AuditLogJpaRepository auditLogJpaRepository; + @MockBean private DeviceAuthService deviceAuthService; + @MockBean private RbacService rbacService; + @MockBean private GovernanceNotificationService governanceNotificationService; + @MockBean private NotificationDispatcher notificationDispatcher; + + @Test + void hideAndRestorePersistModerationStateAndAuditRows() throws Exception { + SkillRating review = createReview(); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/" + review.getId() + "/hide") + .contentType("application/json") + .content("{\"reason\":\"off topic\"}") + .with(authentication(adminAuth())) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.status").value("HIDDEN")); + + SkillRating hidden = ratingRepository.findById(review.getId()).orElseThrow(); + assertThat(hidden.getReviewStatus()).isEqualTo(SkillReviewStatus.HIDDEN); + assertThat(hidden.getModeratedBy()).isEqualTo(ADMIN_ID); + assertThat(hidden.getModerationReason()).isEqualTo("off topic"); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/" + review.getId() + "/restore") + .with(authentication(adminAuth())) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.status").value("VISIBLE")); + + SkillRating saved = ratingRepository.findById(review.getId()).orElseThrow(); + assertThat(saved.getReviewStatus()).isEqualTo(SkillReviewStatus.VISIBLE); + assertThat(saved.getModeratedBy()).isEqualTo(ADMIN_ID); + + List actions = auditLogJpaRepository.findAll().stream() + .filter(log -> ADMIN_ID.equals(log.getActorUserId())) + .filter(log -> review.getId().equals(log.getTargetId())) + .filter(log -> "SKILL_REVIEW".equals(log.getTargetType())) + .map(AuditLog::getAction) + .toList(); + assertThat(actions).contains("SKILL_REVIEW_HIDE", "SKILL_REVIEW_RESTORE"); + } + + @Test + void hideRollsBackModerationWhenAuditPersistenceFails() throws Exception { + SkillRating review = createReview(); + doThrow(new DataIntegrityViolationException("forced audit failure")) + .when(auditLogRepository).save(any(AuditLog.class)); + + mockMvc.perform(post("/api/v1/admin/skill-reviews/" + review.getId() + "/hide") + .contentType("application/json") + .content("{\"reason\":\"off topic\"}") + .with(authentication(adminAuth())) + .with(csrf())) + .andExpect(status().isInternalServerError()); + + SkillRating saved = ratingRepository.findById(review.getId()).orElseThrow(); + assertThat(saved.getReviewStatus()).isEqualTo(SkillReviewStatus.VISIBLE); + assertThat(saved.getModeratedBy()).isNull(); + assertThat(saved.getModerationReason()).isNull(); + } + + private SkillRating createReview() { + String suffix = UUID.randomUUID().toString().substring(0, 8); + saveUserIfAbsent(ADMIN_ID, "Review Admin", "review-admin@example.test"); + saveUserIfAbsent(AUTHOR_ID, "Review Author", "review-author@example.test"); + + Namespace namespace = namespaceRepository.saveAndFlush( + new Namespace("review-team-" + suffix, "Review Team " + suffix, AUTHOR_ID)); + Skill skill = new Skill(namespace.getId(), "review-skill-" + suffix, AUTHOR_ID, SkillVisibility.PUBLIC); + skill.setCreatedBy(AUTHOR_ID); + skill.setUpdatedBy(AUTHOR_ID); + skill = skillRepository.saveAndFlush(skill); + + SkillRating review = new SkillRating(skill.getId(), AUTHOR_ID, (short) 4); + review.updateReview((short) 4, "Useful review"); + return ratingJpaRepository.saveAndFlush(review); + } + + private void saveUserIfAbsent(String userId, String displayName, String email) { + if (!userAccountRepository.existsById(userId)) { + userAccountRepository.saveAndFlush(new UserAccount(userId, displayName, email, null)); + } + } + + private UsernamePasswordAuthenticationToken adminAuth() { + PlatformPrincipal principal = new PlatformPrincipal( + ADMIN_ID, + "Review Admin", + "review-admin@example.test", + "", + "session", + Set.of("SKILL_ADMIN") + ); + return new UsernamePasswordAuthenticationToken( + principal, + null, + List.of(new SimpleGrantedAuthority("ROLE_SKILL_ADMIN")) + ); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java index 1cfcee32..82b2a3b6 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java @@ -14,6 +14,7 @@ import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.metrics.SkillHubMetrics; import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.security.SensitiveLogSanitizer; +import com.iflytek.skillhub.domain.social.SkillRating; import com.iflytek.skillhub.storage.StorageAccessException; import jakarta.servlet.http.HttpServletRequest; import java.time.Clock; @@ -34,6 +35,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.orm.ObjectOptimisticLockingFailureException; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; @@ -72,6 +74,8 @@ class GlobalExceptionHandlerTest { messageSource.addMessage("error.unsupportedMediaType", java.util.Locale.getDefault(), "Unsupported media type"); messageSource.addMessage("error.notAcceptable", java.util.Locale.getDefault(), "Requested response media type is not acceptable"); + messageSource.addMessage("error.request.conflict", java.util.Locale.getDefault(), + "Refresh and try again"); requestIdAccessor = new RequestIdAccessor(); ApiResponseFactory responseFactory = new ApiResponseFactory( messageSource, @@ -144,6 +148,22 @@ class GlobalExceptionHandlerTest { .doesNotContain("userId=")); } + @Test + void persistenceConflictsReturn409WithoutLeakingDatabaseDetails() { + attachAppender(); + prepareClientErrorRequest("PUT", "/api/v1/skills/10/reviews/me"); + + ResponseEntity> response = handler.handlePersistenceConflict( + new ObjectOptimisticLockingFailureException(SkillRating.class, 7L), request); + + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT); + assertThat(response.getBody()).isNotNull(); + assertThat(response.getBody().code()).isEqualTo(409); + assertThat(response.getBody().msg()).isEqualTo("Refresh and try again"); + assertThat(loggedMessages()).allSatisfy(message -> assertThat(message) + .doesNotContain("user-secret")); + } + @Test void handleStorageAccess_shouldLogAuthenticationWithoutStableUserId() { authenticateRequest(); 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 new file mode 100644 index 00000000..6a8959b9 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaSkillReviewQueryRepositoryTest.java @@ -0,0 +1,69 @@ +package com.iflytek.skillhub.repository; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.domain.social.SkillRating; +import com.iflytek.skillhub.domain.social.SkillRatingRepository; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.domain.PageImpl; +import org.springframework.data.domain.PageRequest; + +@ExtendWith(MockitoExtension.class) +class JpaSkillReviewQueryRepositoryTest { + + @Mock private SkillRatingRepository ratingRepository; + @Mock private UserAccountRepository userAccountRepository; + + @Test + void assemblesAuthorProfileWithoutPerRowUserQueries() { + SkillRating rating = new SkillRating(10L, "author-1", (short) 5); + rating.updateReview((short) 5, "excellent"); + UserAccount author = new UserAccount("author-1", "Alice", null, "avatar.png"); + PageRequest page = PageRequest.of(0, 20); + when(ratingRepository.findVisibleReviewsBySkillId(10L, page)) + .thenReturn(new PageImpl<>(List.of(rating), page, 1)); + when(userAccountRepository.findByIdIn(List.of("author-1"))).thenReturn(List.of(author)); + + var result = new JpaSkillReviewQueryRepository(ratingRepository, userAccountRepository) + .list(10L, "author-1", false, page); + + assertThat(result.getContent()).singleElement().satisfies(review -> { + assertThat(review.displayName()).isEqualTo("Alice"); + assertThat(review.avatarUrl()).isEqualTo("avatar.png"); + 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..98d72631 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillRatingOptimisticLockingTest.java @@ -0,0 +1,196 @@ +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 com.iflytek.skillhub.domain.social.SkillReviewStatus; +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; +import jakarta.persistence.OptimisticLockException; +import org.hibernate.exception.ConstraintViolationException; +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.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +@DataJpaTest +@AutoConfigureTestDatabase(replace = AutoConfigureTestDatabase.Replace.NONE) +@ActiveProfiles("test") +@Testcontainers +class SkillRatingOptimisticLockingTest { + + @Container + private static final PostgreSQLContainer POSTGRES = + new PostgreSQLContainer<>("postgres:16-alpine"); + + @DynamicPropertySource + static void configurePostgres(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"); + } + + @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.hideReview("moderator", "Policy violation"); + firstManager.getTransaction().commit(); + + stale.updateReview((short) 2, "Stale update"); + assertThatThrownBy(staleManager.getTransaction()::commit) + .satisfies(error -> assertThat(hasCause(error, OptimisticLockException.class)).isTrue()); + + EntityManager verifier = entityManagerFactory.createEntityManager(); + try { + SkillRating saved = verifier.find(SkillRating.class, ratingId); + assertThat(saved.getReviewStatus()).isEqualTo(SkillReviewStatus.HIDDEN); + assertThat(saved.getModerationReason()).isEqualTo("Policy violation"); + } finally { + verifier.close(); + } + } finally { + rollbackIfActive(firstManager); + rollbackIfActive(staleManager); + firstManager.close(); + staleManager.close(); + deleteRating(ratingId); + } + } + + @Test + @Transactional(propagation = Propagation.NOT_SUPPORTED) + void duplicateFirstInsertIsRejectedByTheDatabaseUniqueConstraint() { + EntityManager firstManager = entityManagerFactory.createEntityManager(); + EntityManager secondManager = entityManagerFactory.createEntityManager(); + try { + firstManager.getTransaction().begin(); + secondManager.getTransaction().begin(); + assertThat(countRatings(firstManager, 20L, "duplicate-author")).isZero(); + assertThat(countRatings(secondManager, 20L, "duplicate-author")).isZero(); + + SkillRating first = new SkillRating(20L, "duplicate-author", (short) 4); + first.updateReview((short) 4, "First insert"); + firstManager.persist(first); + firstManager.getTransaction().commit(); + + SkillRating duplicate = new SkillRating(20L, "duplicate-author", (short) 5); + duplicate.updateReview((short) 5, "Duplicate insert"); + assertThatThrownBy(() -> { + secondManager.persist(duplicate); + secondManager.flush(); + secondManager.getTransaction().commit(); + }).satisfies(error -> assertThat(hasCause(error, ConstraintViolationException.class)).isTrue()); + + EntityManager verifier = entityManagerFactory.createEntityManager(); + try { + assertThat(countRatings(verifier, 20L, "duplicate-author")).isEqualTo(1L); + } finally { + verifier.close(); + } + } finally { + rollbackIfActive(firstManager); + rollbackIfActive(secondManager); + firstManager.close(); + secondManager.close(); + deleteRatings(20L, "duplicate-author"); + } + } + + 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 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 long countRatings(EntityManager entityManager, Long skillId, String userId) { + return entityManager.createQuery(""" + SELECT COUNT(r) FROM SkillRating r + WHERE r.skillId = :skillId AND r.userId = :userId + """, Long.class) + .setParameter("skillId", skillId) + .setParameter("userId", userId) + .getSingleResult(); + } + + private void deleteRatings(Long skillId, String userId) { + EntityManager entityManager = entityManagerFactory.createEntityManager(); + try { + entityManager.getTransaction().begin(); + entityManager.createQuery(""" + DELETE FROM SkillRating r + WHERE r.skillId = :skillId AND r.userId = :userId + """) + .setParameter("skillId", skillId) + .setParameter("userId", userId) + .executeUpdate(); + entityManager.getTransaction().commit(); + } finally { + rollbackIfActive(entityManager); + entityManager.close(); + } + } + + 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 new file mode 100644 index 00000000..be792eed --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillReviewAppServiceTest.java @@ -0,0 +1,198 @@ +package com.iflytek.skillhub.service; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillStatus; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.VisibilityChecker; +import com.iflytek.skillhub.domain.social.SkillRating; +import com.iflytek.skillhub.domain.social.SkillRatingService; +import com.iflytek.skillhub.observability.RequestIdAccessor; +import com.iflytek.skillhub.repository.SkillReviewQueryRepository; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; + +@ExtendWith(MockitoExtension.class) +class SkillReviewAppServiceTest { + + @Mock private SkillRepository skillRepository; + @Mock private SkillVersionRepository skillVersionRepository; + @Mock private SkillRatingService ratingService; + @Mock private SkillReviewQueryRepository queryRepository; + @Mock private AuditLogService auditLogService; + @Mock private RequestIdAccessor requestIdAccessor; + + private SkillReviewAppService service; + + @BeforeEach + void setUp() { + service = new SkillReviewAppService( + skillRepository, + skillVersionRepository, + new VisibilityChecker(), + ratingService, + queryRepository, + auditLogService, + requestIdAccessor + ); + } + + @Test + void publicListingExcludesHiddenReviewsForRegularViewer() { + Skill skill = publishedSkill(SkillVisibility.PUBLIC); + when(skillRepository.findById(10L)).thenReturn(Optional.of(skill)); + when(queryRepository.list(eq(10L), eq(null), eq(false), any(Pageable.class))) + .thenReturn(Page.empty()); + + service.list(10L, null, Map.of(), Set.of(), 0, 20); + + verify(queryRepository).list(eq(10L), eq(null), eq(false), any(Pageable.class)); + } + + @Test + void skillAdminListingIncludesHiddenReviews() { + Skill skill = publishedSkill(SkillVisibility.PUBLIC); + when(skillRepository.findById(10L)).thenReturn(Optional.of(skill)); + when(queryRepository.list(eq(10L), eq("admin"), eq(true), any(Pageable.class))) + .thenReturn(Page.empty()); + + service.list(10L, "admin", Map.of(), Set.of("SKILL_ADMIN"), 0, 20); + + verify(queryRepository).list(eq(10L), eq("admin"), eq(true), any(Pageable.class)); + } + + @Test + void reviewPaginationRejectsNegativePageAndSizesOutsideOneToOneHundred() { + Skill skill = publishedSkill(SkillVisibility.PUBLIC); + when(skillRepository.findById(10L)).thenReturn(Optional.of(skill)); + + assertThatThrownBy(() -> service.list(10L, null, Map.of(), Set.of(), -1, 20)) + .isInstanceOf(DomainBadRequestException.class); + assertThatThrownBy(() -> service.list(10L, null, Map.of(), Set.of(), 0, 0)) + .isInstanceOf(DomainBadRequestException.class); + assertThatThrownBy(() -> service.list(10L, null, Map.of(), Set.of(), 0, 101)) + .isInstanceOf(DomainBadRequestException.class); + + verify(queryRepository, never()).list(any(), any(), anyBoolean(), any(Pageable.class)); + } + + @Test + void privateSkillReviewMutationRequiresSkillAccess() { + Skill skill = publishedSkill(SkillVisibility.PRIVATE); + when(skillRepository.findById(10L)).thenReturn(Optional.of(skill)); + + assertThatThrownBy(() -> service.upsert( + 10L, "other-user", (short) 5, "great", Map.of(), Set.of())) + .isInstanceOf(DomainForbiddenException.class); + + verify(ratingService, never()).upsertReview(any(), any(), any(Short.class), any()); + } + + @Test + void unpublishedSkillReviewMutationIsRejectedServerSide() { + Skill skill = publishedSkill(SkillVisibility.PUBLIC); + SkillVersion pending = new SkillVersion(10L, "1.0.0", "owner"); + pending.setStatus(SkillVersionStatus.PENDING_REVIEW); + when(skillRepository.findById(10L)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(100L)).thenReturn(Optional.of(pending)); + + assertThatThrownBy(() -> service.upsert( + 10L, "owner", (short) 5, "not published", Map.of(), Set.of())) + .isInstanceOf(com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException.class) + .hasMessage("error.skillReview.notInteractable"); + + verify(ratingService, never()).upsertReview(any(), any(), any(Short.class), any()); + } + + @Test + void archivedSkillReviewMutationIsRejectedServerSide() { + Skill skill = publishedSkill(SkillVisibility.PUBLIC); + skill.setStatus(SkillStatus.ARCHIVED); + SkillVersion published = new SkillVersion(10L, "1.0.0", "owner"); + published.setStatus(SkillVersionStatus.PUBLISHED); + when(skillRepository.findById(10L)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(100L)).thenReturn(Optional.of(published)); + + assertThatThrownBy(() -> service.upsert( + 10L, "owner", (short) 5, "archived", Map.of(), Set.of())) + .isInstanceOf(DomainBadRequestException.class) + .hasMessage("error.skillReview.notInteractable"); + + 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 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); + review.updateReview((short) 4, "helpful review"); + when(ratingService.hideReview(null, "admin", "spam")).thenReturn(review); + when(requestIdAccessor.current()).thenReturn("request-1"); + + service.hide(null, "admin", "spam", new AuditRequestContext("127.0.0.1", "test")); + + verify(auditLogService).record( + eq("admin"), + eq("SKILL_REVIEW_HIDE"), + eq("SKILL_REVIEW"), + eq(null), + eq("request-1"), + eq("127.0.0.1"), + eq("test"), + eq("{\"skillId\":10,\"reason\":\"spam\"}") + ); + } + + private Skill publishedSkill(SkillVisibility visibility) { + Skill skill = new Skill(1L, "demo", "owner", visibility); + skill.setLatestVersionId(100L); + return skill; + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java index 09ed21c7..58b47ae0 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java @@ -39,11 +39,19 @@ public class RouteSecurityPolicyRegistry { RouteAuthorizationPolicy.authenticated(HttpMethod.DELETE, "/api/v1/skills/*/star"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/v1/skills/*/rating"), RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/v1/skills/*/rating"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/reviews"), + RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/v1/skills/*/reviews/me"), + RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/v1/skills/*/reviews/me"), + RouteAuthorizationPolicy.authenticated(HttpMethod.DELETE, "/api/v1/skills/*/reviews/me"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/*/star"), RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/web/skills/*/star"), RouteAuthorizationPolicy.authenticated(HttpMethod.DELETE, "/api/web/skills/*/star"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/*/rating"), RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/web/skills/*/rating"), + RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/reviews"), + RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/*/reviews/me"), + RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/web/skills/*/reviews/me"), + RouteAuthorizationPolicy.authenticated(HttpMethod.DELETE, "/api/web/skills/*/reviews/me"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions"), @@ -112,9 +120,13 @@ public class RouteSecurityPolicyRegistry { ApiTokenPolicy.allow(HttpMethod.PUT, "/api/v1/skills/*/star"), ApiTokenPolicy.allow(HttpMethod.DELETE, "/api/v1/skills/*/star"), ApiTokenPolicy.allow(HttpMethod.PUT, "/api/v1/skills/*/rating"), + ApiTokenPolicy.allow(HttpMethod.PUT, "/api/v1/skills/*/reviews/me"), + ApiTokenPolicy.allow(HttpMethod.DELETE, "/api/v1/skills/*/reviews/me"), ApiTokenPolicy.allow(HttpMethod.PUT, "/api/web/skills/*/star"), ApiTokenPolicy.allow(HttpMethod.DELETE, "/api/web/skills/*/star"), ApiTokenPolicy.allow(HttpMethod.PUT, "/api/web/skills/*/rating"), + ApiTokenPolicy.allow(HttpMethod.PUT, "/api/web/skills/*/reviews/me"), + ApiTokenPolicy.allow(HttpMethod.DELETE, "/api/web/skills/*/reviews/me"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/namespaces"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/namespaces/*"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/namespaces"), diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java index 0967b746..4cde9090 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java @@ -34,6 +34,26 @@ class RouteSecurityPolicyRegistryTest { registry.accessLevel("GET", "/api/v1/resolve/team/demo")); } + @Test + void reviewRoutesExposePublicListingButProtectCurrentUserMutations() { + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.PERMIT_ALL, + registry.accessLevel("GET", "/api/v1/skills/10/reviews")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, + registry.accessLevel("GET", "/api/v1/skills/10/reviews/me")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, + registry.accessLevel("PUT", "/api/v1/skills/10/reviews/me")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, + registry.accessLevel("DELETE", "/api/v1/skills/10/reviews/me")); + } + + @Test + void apiTokenPolicyAllowsCurrentUserReviewMutations() { + assertTrue(registry.authorizeApiToken( + "PUT", "/api/v1/skills/10/reviews/me", Set.of()).allowed()); + assertTrue(registry.authorizeApiToken( + "DELETE", "/api/v1/skills/10/reviews/me", Set.of()).allowed()); + } + @Test void authorizeApiToken_requiresPublishScopeForPublishEndpoints() { var denied = registry.authorizeApiToken("POST", "/api/web/skills/global/publish", Set.of("skill:read")); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/shared/exception/DomainConflictException.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/shared/exception/DomainConflictException.java new file mode 100644 index 00000000..085c593a --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/shared/exception/DomainConflictException.java @@ -0,0 +1,16 @@ +package com.iflytek.skillhub.domain.shared.exception; + +/** + * Domain exception used when a concurrent request prevents a safe state change. + */ +public class DomainConflictException extends LocalizedDomainException { + + public DomainConflictException(String messageCode, Object... messageArgs) { + super(messageCode, messageArgs); + } + + @Override + public int statusCode() { + return 409; + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java index c7a86cd7..d115e485 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java @@ -242,7 +242,8 @@ public class SkillQueryService { skill.getUpdatedAt(), canManageRestrictedSkill(skill, currentUserId, userNsRoles), canSubmitPromotion(namespace, skill, publishedVersion, currentUserId, userNsRoles), - headlineVersion == null || "PUBLISHED".equals(headlineVersion.status()), + skill.getStatus() == SkillStatus.ACTIVE + && (headlineVersion == null || "PUBLISHED".equals(headlineVersion.status())), currentUserId == null || !Objects.equals(skill.getOwnerId(), currentUserId), headlineVersion, publishedVersion, diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRating.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRating.java index b38850ca..0448ea47 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRating.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRating.java @@ -9,9 +9,15 @@ import java.time.Instant; @Table(name = "skill_rating", uniqueConstraints = @UniqueConstraint(columnNames = {"skill_id", "user_id"})) public class SkillRating { + private static final int MAX_REVIEW_LENGTH = 2000; + @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; + @Version + @Column(name = "lock_version", nullable = false) + private Long lockVersion; + @Column(name = "skill_id", nullable = false) private Long skillId; @@ -21,6 +27,22 @@ public class SkillRating { @Column(nullable = false) private Short score; + @Column(name = "review_text", length = MAX_REVIEW_LENGTH) + private String reviewText; + + @Enumerated(EnumType.STRING) + @Column(name = "review_status", nullable = false, length = 16) + private SkillReviewStatus reviewStatus = SkillReviewStatus.VISIBLE; + + @Column(name = "moderated_by", length = 128) + private String moderatedBy; + + @Column(name = "moderated_at") + private Instant moderatedAt; + + @Column(name = "moderation_reason", length = 500) + private String moderationReason; + @Column(name = "created_at", nullable = false) private Instant createdAt; @@ -37,11 +59,77 @@ public class SkillRating { } public void updateScore(short newScore) { - if (newScore < 1 || newScore > 5) throw new DomainBadRequestException("error.rating.score.invalid"); + validateScore(newScore); this.score = newScore; this.updatedAt = Instant.now(Clock.systemUTC()); } + public void updateReview(short newScore, String newReviewText) { + validateScore(newScore); + this.score = newScore; + this.reviewText = normalizeReviewText(newReviewText); + this.updatedAt = Instant.now(Clock.systemUTC()); + } + + public void clearReview() { + this.reviewText = null; + this.updatedAt = Instant.now(Clock.systemUTC()); + } + + public void hideReview(String moderatorId, String reason) { + ensureReviewExists(); + this.reviewStatus = SkillReviewStatus.HIDDEN; + this.moderatedBy = moderatorId; + this.moderatedAt = Instant.now(Clock.systemUTC()); + this.moderationReason = normalizeReason(reason); + } + + public void restoreReview(String moderatorId) { + ensureReviewExists(); + this.reviewStatus = SkillReviewStatus.VISIBLE; + this.moderatedBy = moderatorId; + this.moderatedAt = Instant.now(Clock.systemUTC()); + this.moderationReason = null; + } + + public boolean hasReview() { + return reviewText != null && !reviewText.isBlank(); + } + + private static void validateScore(short value) { + if (value < 1 || value > 5) { + throw new DomainBadRequestException("error.rating.score.invalid"); + } + } + + private static String normalizeReviewText(String value) { + if (value == null || value.isBlank()) { + throw new DomainBadRequestException("error.skillReview.text.required"); + } + String normalized = value.trim(); + if (normalized.length() > MAX_REVIEW_LENGTH) { + throw new DomainBadRequestException("error.skillReview.text.tooLong", MAX_REVIEW_LENGTH); + } + return normalized; + } + + private static String normalizeReason(String value) { + if (value == null || value.isBlank()) { + return null; + } + String normalized = value.trim(); + if (normalized.length() > 500) { + throw new DomainBadRequestException("error.skillReview.reason.tooLong", 500); + } + return normalized; + } + + private void ensureReviewExists() { + if (!hasReview()) { + throw new DomainBadRequestException("error.skillReview.notFound"); + } + } + @PrePersist void prePersist() { this.createdAt = Instant.now(Clock.systemUTC()); @@ -55,9 +143,15 @@ public class SkillRating { // getters public Long getId() { return id; } + public Long getLockVersion() { return lockVersion; } public Long getSkillId() { return skillId; } public String getUserId() { return userId; } public Short getScore() { return score; } + public String getReviewText() { return reviewText; } + public SkillReviewStatus getReviewStatus() { return reviewStatus; } + public String getModeratedBy() { return moderatedBy; } + public Instant getModeratedAt() { return moderatedAt; } + public String getModerationReason() { return moderationReason; } public Instant getCreatedAt() { return createdAt; } public Instant getUpdatedAt() { return updatedAt; } } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingRepository.java index f0a8a245..a608cfe6 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingRepository.java @@ -1,13 +1,19 @@ package com.iflytek.skillhub.domain.social; import java.util.Optional; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; /** * Domain repository contract for per-user ratings and rating aggregates on one skill. */ public interface SkillRatingRepository { SkillRating save(SkillRating rating); + void flush(); + Optional findById(Long id); Optional findBySkillIdAndUserId(Long skillId, String userId); + Page findVisibleReviewsBySkillId(Long skillId, Pageable pageable); + Page findReviewsBySkillId(Long skillId, Pageable pageable); double averageScoreBySkillId(Long skillId); int countBySkillId(Long skillId); void deleteBySkillId(Long skillId); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingService.java index 95e67b0c..879b7eac 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingService.java @@ -1,10 +1,12 @@ package com.iflytek.skillhub.domain.social; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainConflictException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.social.event.SkillRatedEvent; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -44,12 +46,76 @@ public class SkillRatingService { eventPublisher.publishEvent(new SkillRatedEvent(skillId, userId, score)); } + @Transactional + public SkillRating upsertReview(Long skillId, String userId, short score, String reviewText) { + ensureSkillExists(skillId); + SkillRating rating = ratingRepository.findBySkillIdAndUserId(skillId, userId) + .orElseGet(() -> new SkillRating(skillId, userId, score)); + rating.updateReview(score, reviewText); + SkillRating saved; + try { + saved = ratingRepository.save(rating); + ratingRepository.flush(); + } catch (DataIntegrityViolationException exception) { + throw new DomainConflictException("error.request.conflict"); + } + eventPublisher.publishEvent(new SkillRatedEvent(skillId, userId, score)); + return saved; + } + + @Transactional + public SkillRating clearReview(Long skillId, String userId) { + ensureSkillExists(skillId); + SkillRating rating = ratingRepository.findBySkillIdAndUserId(skillId, userId) + .filter(SkillRating::hasReview) + .orElseThrow(() -> new DomainNotFoundException("error.skillReview.notFound")); + rating.clearReview(); + SkillRating saved = ratingRepository.save(rating); + ratingRepository.flush(); + return saved; + } + + @Transactional + public SkillRating hideReview(Long reviewId, String moderatorId, String reason) { + SkillRating rating = findReview(reviewId); + rating.hideReview(moderatorId, reason); + SkillRating saved = ratingRepository.save(rating); + ratingRepository.flush(); + return saved; + } + + @Transactional + public SkillRating restoreReview(Long reviewId, String moderatorId) { + SkillRating rating = findReview(reviewId); + rating.restoreReview(moderatorId); + SkillRating saved = ratingRepository.save(rating); + ratingRepository.flush(); + return saved; + } + public Optional getUserRating(Long skillId, String userId) { ensureSkillExists(skillId); return ratingRepository.findBySkillIdAndUserId(skillId, userId) .map(SkillRating::getScore); } + public Optional getUserReview(Long skillId, String userId) { + ensureSkillExists(skillId); + return ratingRepository.findBySkillIdAndUserId(skillId, userId) + .filter(SkillRating::hasReview); + } + + public Optional getUserFeedback(Long skillId, String userId) { + ensureSkillExists(skillId); + return ratingRepository.findBySkillIdAndUserId(skillId, userId); + } + + private SkillRating findReview(Long reviewId) { + return ratingRepository.findById(reviewId) + .filter(SkillRating::hasReview) + .orElseThrow(() -> new DomainNotFoundException("error.skillReview.notFound")); + } + private void ensureSkillExists(Long skillId) { if (skillRepository.findById(skillId).isEmpty()) { throw new DomainNotFoundException("skill.not_found", skillId); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillReviewStatus.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillReviewStatus.java new file mode 100644 index 00000000..f0b9e631 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillReviewStatus.java @@ -0,0 +1,7 @@ +package com.iflytek.skillhub.domain.social; + +/** Visibility state for the optional review text attached to a skill rating. */ +public enum SkillReviewStatus { + VISIBLE, + HIDDEN +} diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java index 7fb08e3c..67d34fc4 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java @@ -1212,6 +1212,57 @@ class SkillQueryServiceTest { assertTrue(result.canInteract()); } + @Test + void testGetSkillDetail_ShouldDisableInteractionForArchivedSkill() throws Exception { + String namespaceSlug = "test-ns"; + String skillSlug = "test-skill"; + String ownerId = "owner-1"; + Map userNsRoles = Map.of(); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", ownerId); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, ownerId, SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setStatus(SkillStatus.ARCHIVED); + skill.setLatestVersionId(11L); + + SkillVersion published = new SkillVersion(1L, "1.0.0", ownerId); + setId(published, 11L); + published.setStatus(SkillVersionStatus.PUBLISHED); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + when(skillVersionRepository.findById(11L)).thenReturn(Optional.of(published)); + + SkillQueryService.SkillDetailDTO result = service.getSkillDetail(namespaceSlug, skillSlug, ownerId, userNsRoles); + + assertNotNull(result.headlineVersion()); + assertEquals("PUBLISHED", result.headlineVersion().status()); + assertFalse(result.canInteract()); + } + + @Test + void testGetSkillDetail_ShouldKeepInteractionForActiveSkillWithoutHeadlineVersion() throws Exception { + String namespaceSlug = "test-ns"; + String skillSlug = "test-skill"; + String ownerId = "owner-1"; + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", ownerId); + setId(namespace, 1L); + Skill skill = new Skill(1L, skillSlug, ownerId, SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setStatus(SkillStatus.ACTIVE); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, skillSlug)).thenReturn(List.of(skill)); + + SkillQueryService.SkillDetailDTO result = service.getSkillDetail( + namespaceSlug, skillSlug, ownerId, Map.of()); + + assertNull(result.headlineVersion()); + assertTrue(result.canInteract()); + } + @Test void testGetSkillDetail_ShouldIncludeRejectedOwnerPreviewComment() throws Exception { String namespaceSlug = "test-ns"; diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java index 00bf2133..53f3a262 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/social/SkillRatingServiceTest.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.domain.social; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainConflictException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; @@ -11,6 +12,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.*; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import java.util.Optional; @@ -77,4 +79,141 @@ class SkillRatingServiceTest { assertThatThrownBy(() -> service.getUserRating(99L, "10")) .isInstanceOf(DomainNotFoundException.class); } + + @Test + void upsertReview_creates_review_and_rating() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill())); + when(ratingRepository.findBySkillIdAndUserId(1L, "user-1")).thenReturn(Optional.empty()); + when(ratingRepository.save(any())).thenAnswer(invocation -> invocation.getArgument(0)); + + SkillRating review = service.upsertReview(1L, "user-1", (short) 5, " Useful skill. "); + + assertThat(review.getScore()).isEqualTo((short) 5); + assertThat(review.getReviewText()).isEqualTo("Useful skill."); + assertThat(review.getReviewStatus()).isEqualTo(SkillReviewStatus.VISIBLE); + verify(eventPublisher).publishEvent(any(SkillRatedEvent.class)); + } + + @Test + void upsertReview_preserves_hidden_status_when_author_edits() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill())); + SkillRating existing = new SkillRating(1L, "user-1", (short) 2); + existing.updateReview((short) 2, "Original review"); + existing.hideReview("moderator-1", "Policy violation"); + when(ratingRepository.findBySkillIdAndUserId(1L, "user-1")).thenReturn(Optional.of(existing)); + when(ratingRepository.save(any())).thenAnswer(invocation -> invocation.getArgument(0)); + + SkillRating review = service.upsertReview(1L, "user-1", (short) 4, "Edited review"); + + assertThat(review.getReviewStatus()).isEqualTo(SkillReviewStatus.HIDDEN); + assertThat(review.getReviewText()).isEqualTo("Edited review"); + } + + @Test + void upsertReview_rejects_blank_or_too_long_text() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill())); + when(ratingRepository.findBySkillIdAndUserId(1L, "user-1")).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.upsertReview(1L, "user-1", (short) 4, " ")) + .isInstanceOf(DomainBadRequestException.class); + assertThatThrownBy(() -> service.upsertReview(1L, "user-1", (short) 4, "x".repeat(2001))) + .isInstanceOf(DomainBadRequestException.class); + } + + @Test + void upsertReview_mapsConcurrentFirstInsertToConflict() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill())); + when(ratingRepository.findBySkillIdAndUserId(1L, "user-1")).thenReturn(Optional.empty()); + when(ratingRepository.save(any())).thenAnswer(invocation -> invocation.getArgument(0)); + doThrow(new DataIntegrityViolationException("duplicate rating")) + .when(ratingRepository).flush(); + + assertThatThrownBy(() -> service.upsertReview(1L, "user-1", (short) 4, "Useful")) + .isInstanceOf(DomainConflictException.class) + .hasMessage("error.request.conflict"); + + verify(eventPublisher, never()).publishEvent(any(SkillRatedEvent.class)); + } + + @Test + void clearReview_keeps_rating_row_and_score() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill())); + SkillRating existing = new SkillRating(1L, "user-1", (short) 4); + existing.updateReview((short) 4, "Useful skill"); + when(ratingRepository.findBySkillIdAndUserId(1L, "user-1")).thenReturn(Optional.of(existing)); + when(ratingRepository.save(existing)).thenReturn(existing); + + SkillRating result = service.clearReview(1L, "user-1"); + + assertThat(result.hasReview()).isFalse(); + assertThat(result.getScore()).isEqualTo((short) 4); + assertThat(result.getReviewStatus()).isEqualTo(SkillReviewStatus.VISIBLE); + } + + @Test + void clearAndResubmitReview_preservesHiddenModerationState() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill())); + SkillRating existing = new SkillRating(1L, "user-1", (short) 4); + existing.updateReview((short) 4, "Hidden review"); + existing.hideReview("moderator-1", "Policy violation"); + when(ratingRepository.findBySkillIdAndUserId(1L, "user-1")) + .thenReturn(Optional.of(existing)); + when(ratingRepository.save(existing)).thenReturn(existing); + + service.clearReview(1L, "user-1"); + SkillRating resubmitted = service.upsertReview(1L, "user-1", (short) 5, "Rewritten review"); + + assertThat(resubmitted.getReviewStatus()).isEqualTo(SkillReviewStatus.HIDDEN); + assertThat(resubmitted.getModeratedBy()).isEqualTo("moderator-1"); + assertThat(resubmitted.getModerationReason()).isEqualTo("Policy violation"); + assertThat(resubmitted.getReviewText()).isEqualTo("Rewritten review"); + verify(ratingRepository, times(2)).flush(); + } + + @Test + void moderator_can_hide_and_restore_review() { + SkillRating existing = new SkillRating(1L, "user-1", (short) 4); + existing.updateReview((short) 4, "Useful skill"); + when(ratingRepository.findById(7L)).thenReturn(Optional.of(existing)); + when(ratingRepository.save(existing)).thenReturn(existing); + + SkillRating hidden = service.hideReview(7L, "moderator-1", "Off topic"); + assertThat(hidden.getReviewStatus()).isEqualTo(SkillReviewStatus.HIDDEN); + assertThat(hidden.getModeratedBy()).isEqualTo("moderator-1"); + assertThat(hidden.getModerationReason()).isEqualTo("Off topic"); + + SkillRating restored = service.restoreReview(7L, "moderator-2"); + assertThat(restored.getReviewStatus()).isEqualTo(SkillReviewStatus.VISIBLE); + assertThat(restored.getModeratedBy()).isEqualTo("moderator-2"); + assertThat(restored.getModerationReason()).isNull(); + } + + @Test + void moderationReason_acceptsFiveHundredCharactersAndRejectsFiveHundredOne() { + SkillRating valid = new SkillRating(1L, "user-1", (short) 4); + valid.updateReview((short) 4, "Useful skill"); + SkillRating invalid = new SkillRating(1L, "user-2", (short) 4); + invalid.updateReview((short) 4, "Another useful skill"); + when(ratingRepository.findById(7L)).thenReturn(Optional.of(valid)); + when(ratingRepository.findById(8L)).thenReturn(Optional.of(invalid)); + when(ratingRepository.save(valid)).thenReturn(valid); + + SkillRating hidden = service.hideReview(7L, "moderator", "x".repeat(500)); + assertThat(hidden.getModerationReason()).hasSize(500); + + assertThatThrownBy(() -> service.hideReview(8L, "moderator", "x".repeat(501))) + .isInstanceOf(DomainBadRequestException.class) + .hasMessage("error.skillReview.reason.tooLong"); + verify(ratingRepository, never()).save(invalid); + } + + @Test + void clearReview_throws_when_user_has_only_rating() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill())); + when(ratingRepository.findBySkillIdAndUserId(1L, "user-1")) + .thenReturn(Optional.of(new SkillRating(1L, "user-1", (short) 4))); + + assertThatThrownBy(() -> service.clearReview(1L, "user-1")) + .isInstanceOf(DomainNotFoundException.class); + } } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillRatingRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillRatingRepository.java index 6b613ce3..541cdf1f 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillRatingRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillRatingRepository.java @@ -5,6 +5,8 @@ import com.iflytek.skillhub.domain.social.SkillRatingRepository; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; import org.springframework.stereotype.Repository; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; import java.util.Optional; /** @@ -14,6 +16,25 @@ import java.util.Optional; public interface JpaSkillRatingRepository extends JpaRepository, SkillRatingRepository { Optional findBySkillIdAndUserId(Long skillId, String userId); + @Query(""" + SELECT r FROM SkillRating r + WHERE r.skillId = :skillId + AND r.reviewStatus = com.iflytek.skillhub.domain.social.SkillReviewStatus.VISIBLE + AND r.reviewText IS NOT NULL + AND TRIM(r.reviewText) <> '' + ORDER BY r.updatedAt DESC, r.id DESC + """) + Page findVisibleReviewsBySkillId(Long skillId, Pageable pageable); + + @Query(""" + SELECT r FROM SkillRating r + WHERE r.skillId = :skillId + AND r.reviewText IS NOT NULL + AND TRIM(r.reviewText) <> '' + ORDER BY r.updatedAt DESC, r.id DESC + """) + Page findReviewsBySkillId(Long skillId, Pageable pageable); + @Query("SELECT COALESCE(AVG(r.score), 0) FROM SkillRating r WHERE r.skillId = :skillId") double averageScoreBySkillId(Long skillId); diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index 03786be7..8fbd4236 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -68,6 +68,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/skills/{skillId}/reviews/me": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["getMine"]; + put: operations["upsert"]; + post?: never; + delete: operations["clear"]; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/{skillId}/reviews/me": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["getMine_1"]; + put: operations["upsert_1"]; + post?: never; + delete: operations["clear_1"]; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/web/skills/{skillId}/rating": { parameters: { query?: never; @@ -1195,7 +1227,7 @@ export interface paths { path?: never; cookie?: never; }; - get: operations["list_2"]; + get: operations["list_4"]; put?: never; post: operations["create"]; delete?: never; @@ -1540,6 +1572,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/admin/skill-reviews/{reviewId}/restore": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["restore"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/admin/skill-reviews/{reviewId}/hide": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations["hide"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/admin/skill-reports/{reportId}/resolve": { parameters: { query?: never; @@ -1860,6 +1924,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/skills/{skillId}/reviews": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["list"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/skills/{skillId}/reviews": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["list_1"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/web/skills/{namespace}/{slug}/versions/{version}/files": { parameters: { query?: never; @@ -2731,7 +2827,7 @@ export interface paths { path?: never; cookie?: never; }; - get: operations["list"]; + get: operations["list_2"]; put?: never; post?: never; delete?: never; @@ -2747,7 +2843,7 @@ export interface paths { path?: never; cookie?: never; }; - get: operations["list_1"]; + get: operations["list_3"]; put?: never; post?: never; delete?: never; @@ -3307,7 +3403,7 @@ export interface paths { path?: never; cookie?: never; }; - get: operations["list_3"]; + get: operations["list_5"]; put?: never; post?: never; delete?: never; @@ -3649,6 +3745,35 @@ export interface components { timestamp?: string; requestId?: string; }; + SkillReviewRequest: { + /** Format: int32 */ + score: number; + reviewText: string; + }; + ApiResponseSkillReviewMeResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["SkillReviewMeResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + SkillReviewMeResponse: { + rated?: boolean; + /** Format: int32 */ + score?: number; + reviewed?: boolean; + /** Format: int64 */ + reviewId?: number; + reviewText?: string; + status?: string; + moderationReason?: string; + /** Format: date-time */ + createdAt?: string; + /** Format: date-time */ + updatedAt?: string; + }; SkillRatingRequest: { /** Format: int32 */ score: number; @@ -4241,6 +4366,35 @@ export interface components { timestamp?: string; requestId?: string; }; + ApiResponseSkillReviewResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["SkillReviewResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + SkillReviewResponse: { + /** Format: int64 */ + id?: number; + userId?: string; + displayName?: string; + avatarUrl?: string; + /** Format: int32 */ + score?: number; + reviewText?: string; + status?: string; + authoredByViewer?: boolean; + moderationReason?: string; + /** Format: date-time */ + createdAt?: string; + /** Format: date-time */ + updatedAt?: string; + }; + SkillReviewModerationRequest: { + reason?: string; + }; AdminSkillReportActionRequest: { comment?: string; disposition?: string; @@ -4478,6 +4632,24 @@ export interface components { timestamp?: string; requestId?: string; }; + ApiResponsePageResponseSkillReviewResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["PageResponseSkillReviewResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + PageResponseSkillReviewResponse: { + items?: components["schemas"]["SkillReviewResponse"][]; + /** Format: int64 */ + total?: number; + /** Format: int32 */ + page?: number; + /** Format: int32 */ + size?: number; + }; ApiResponseSkillRatingStatusResponse: { /** Format: int32 */ code?: number; @@ -5846,6 +6018,146 @@ export interface operations { }; }; }; + getMine: { + parameters: { + query?: never; + header?: never; + path: { + skillId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillReviewMeResponse"]; + }; + }; + }; + }; + upsert: { + parameters: { + query?: never; + header?: never; + path: { + skillId: number; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["SkillReviewRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillReviewMeResponse"]; + }; + }; + }; + }; + clear: { + parameters: { + query?: never; + header?: never; + path: { + skillId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillReviewMeResponse"]; + }; + }; + }; + }; + getMine_1: { + parameters: { + query?: never; + header?: never; + path: { + skillId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillReviewMeResponse"]; + }; + }; + }; + }; + upsert_1: { + parameters: { + query?: never; + header?: never; + path: { + skillId: number; + }; + cookie?: never; + }; + requestBody: { + content: { + "application/json": components["schemas"]["SkillReviewRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillReviewMeResponse"]; + }; + }; + }; + }; + clear_1: { + parameters: { + query?: never; + header?: never; + path: { + skillId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillReviewMeResponse"]; + }; + }; + }; + }; getUserRating: { parameters: { query?: never; @@ -8099,7 +8411,7 @@ export interface operations { }; }; }; - list_2: { + list_4: { parameters: { query?: { page?: number; @@ -8690,6 +9002,54 @@ export interface operations { }; }; }; + restore: { + parameters: { + query?: never; + header?: never; + path: { + reviewId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillReviewResponse"]; + }; + }; + }; + }; + hide: { + parameters: { + query?: never; + header?: never; + path: { + reviewId: number; + }; + cookie?: never; + }; + requestBody?: { + content: { + "application/json": components["schemas"]["SkillReviewModerationRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSkillReviewResponse"]; + }; + }; + }; + }; resolveReport: { parameters: { query?: never; @@ -9258,6 +9618,56 @@ export interface operations { }; }; }; + list: { + parameters: { + query?: { + page?: number; + size?: number; + }; + header?: never; + path: { + skillId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponsePageResponseSkillReviewResponse"]; + }; + }; + }; + }; + list_1: { + parameters: { + query?: { + page?: number; + size?: number; + }; + header?: never; + path: { + skillId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponsePageResponseSkillReviewResponse"]; + }; + }; + }; + }; listFiles: { parameters: { query?: never; @@ -10624,7 +11034,7 @@ export interface operations { }; }; }; - list: { + list_2: { parameters: { query?: { category?: string; @@ -10648,7 +11058,7 @@ export interface operations { }; }; }; - list_1: { + list_3: { parameters: { query?: { category?: string; @@ -11463,7 +11873,7 @@ export interface operations { }; }; }; - list_3: { + list_5: { parameters: { query?: { status?: string; diff --git a/web/src/features/social/skill-reviews.test.tsx b/web/src/features/social/skill-reviews.test.tsx new file mode 100644 index 00000000..c150213a --- /dev/null +++ b/web/src/features/social/skill-reviews.test.tsx @@ -0,0 +1,187 @@ +/** @vitest-environment jsdom */ + +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import en from '@/i18n/locales/en.json' +import ru from '@/i18n/locales/ru.json' +import zh from '@/i18n/locales/zh.json' + +const mocks = vi.hoisted(() => ({ + pages: new Map>; total: number; page: number; size: number }>(), + requestedPages: [] as number[], + saveMutate: vi.fn(), + clearMutate: vi.fn(), + moderateMutate: vi.fn(), + mineEnabled: [] as boolean[], +})) + +vi.mock('react-i18next', async () => { + const actual = await vi.importActual('react-i18next') + const translations: Record = { + 'skillReviews.title': 'User reviews', + 'skillReviews.count': '{{count}} reviews', + 'skillReviews.write': 'Write a review', + 'skillReviews.edit': 'Edit my review', + 'skillReviews.scoreLabel': 'Your score', + 'skillReviews.ratingDisplay': 'Rating: {{score}} out of 5 stars', + 'skillReviews.ratingOption': 'Rate {{score}} out of 5 stars', + 'skillReviews.reviewTextLabel': 'Review text', + 'skillReviews.placeholder': 'Share your experience', + 'skillReviews.save': 'Save review', + 'skillReviews.cancel': 'Cancel', + 'skillReviews.delete': 'Delete review', + 'skillReviews.empty': 'No reviews', + 'skillReviews.previous': 'Previous', + 'skillReviews.next': 'Next', + } + return { + ...actual, + useTranslation: () => ({ + i18n: { language: 'en' }, + t: (key: string, values?: Record) => Object.entries(values ?? {}).reduce( + (text, [name, value]) => text.replace(`{{${name}}}`, String(value)), + translations[key] ?? key, + ), + }), + } +}) + +vi.mock('@/features/auth/use-auth', () => ({ + useAuth: () => ({ isAuthenticated: true, hasRole: () => false }), +})) + +vi.mock('@/shared/lib/date-time', () => ({ + formatLocalDateTime: (value: string) => value, +})) + +vi.mock('@/shared/lib/toast', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})) + +vi.mock('./use-skill-reviews', () => ({ + useSkillReviews: (_skillId: number, page: number) => { + mocks.requestedPages.push(page) + return { data: mocks.pages.get(page), isLoading: false, isError: false } + }, + useMySkillReview: (_skillId: number, enabled: boolean) => { + mocks.mineEnabled.push(enabled) + return { data: { + rated: true, + score: 4, + reviewed: true, + reviewId: 7, + 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 }), +})) + +import { SkillReviews } from './skill-reviews' + +describe('skill reviews', () => { + beforeEach(() => { + mocks.pages.clear() + mocks.pages.set(0, { items: [], total: 0, page: 0, size: 20 }) + mocks.requestedPages.length = 0 + mocks.mineEnabled.length = 0 + }) + + afterEach(() => { + cleanup() + vi.clearAllMocks() + }) + + it('exposes an accessible rating group and labelled review editor', () => { + render() + + fireEvent.click(screen.getByRole('button', { name: 'Edit my review' })) + + const rating = screen.getByRole('radiogroup', { name: 'Your score' }) + expect(rating).toBeTruthy() + expect(screen.getByRole('radio', { name: 'Rate 4 out of 5 stars' })) + .toHaveProperty('checked', true) + expect(screen.getByRole('textbox', { name: 'Review text' })).toHaveProperty('value', 'Useful review') + }) + + it('keeps a way back when a refetch empties the last page', () => { + mocks.pages.set(0, { items: [], total: 21, page: 0, size: 20 }) + mocks.pages.set(1, { + items: [{ + id: 8, + displayName: 'Alice', + score: 5, + reviewText: 'Great', + status: 'VISIBLE', + authoredByViewer: false, + createdAt: '2026-09-01T00:00:00Z', + updatedAt: '2026-09-01T00:00:00Z', + }], + total: 21, + page: 1, + size: 20, + }) + const view = render() + + fireEvent.click(screen.getByRole('button', { name: 'Next' })) + expect(mocks.requestedPages).toContain(1) + + mocks.pages.set(1, { items: [], total: 20, page: 1, size: 20 }) + view.rerender() + fireEvent.click(screen.getByRole('button', { name: 'Previous' })) + expect(mocks.requestedPages[mocks.requestedPages.length - 1]).toBe(0) + }) + + it('wraps long reviewer names and review text', () => { + const reviewer = 'review_author_1788284593_353294' + const reviewText = 'x'.repeat(200) + mocks.pages.set(0, { + items: [{ + id: 8, + displayName: reviewer, + score: 5, + reviewText, + status: 'VISIBLE', + authoredByViewer: false, + createdAt: '2026-09-01T00:00:00Z', + updatedAt: '2026-09-01T00:00:00Z', + }], + total: 1, + page: 0, + size: 20, + }) + + render() + + expect(screen.getByText(reviewer).className).toContain('[overflow-wrap:anywhere]') + expect(screen.getByText(reviewText).className).toContain('[overflow-wrap:anywhere]') + }) + + it('keeps review interaction copy in every supported locale', () => { + for (const locale of [en, zh, ru]) { + expect(locale.skillReviews.ratingDisplay).toBeTruthy() + expect(locale.skillReviews.ratingOption).toBeTruthy() + expect(locale.skillReviews.reviewTextLabel).toBeTruthy() + expect(locale.skillReviews.hide).toBeTruthy() + expect(locale.skillReviews.restore).toBeTruthy() + } + expect(en.skillReviews.count_one).toBe('{{count}} review') + expect(en.skillReviews.count_other).toBe('{{count}} reviews') + expect(ru.skillReviews.count_one).toBeTruthy() + expect(ru.skillReviews.count_few).toBeTruthy() + expect(ru.skillReviews.count_many).toBeTruthy() + expect(ru.skillReviews.count_other).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 new file mode 100644 index 00000000..0e627e49 --- /dev/null +++ b/web/src/features/social/skill-reviews.tsx @@ -0,0 +1,329 @@ +import { useId, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { Loader2, MessageSquare, ShieldAlert, Star } from 'lucide-react' +import { useAuth } from '@/features/auth/use-auth' +import { Button } from '@/shared/ui/button' +import { Card } from '@/shared/ui/card' +import { Textarea } from '@/shared/ui/textarea' +import { formatLocalDateTime } from '@/shared/lib/date-time' +import { toast } from '@/shared/lib/toast' +import { cn } from '@/shared/lib/utils' +import { + type MySkillReview, + type SkillReview, + useClearSkillReview, + useModerateSkillReview, + useMySkillReview, + useSkillReviews, + useUpsertSkillReview, +} from './use-skill-reviews' + +interface SkillReviewsProps { + skillId: number + canInteract: boolean + onRequireLogin: () => void +} + +function ReviewStars({ value, onChange, disabled = false }: { + value: number + onChange?: (value: number) => void + disabled?: boolean +}) { + const { t } = useTranslation() + const ratingName = useId() + + if (!onChange) { + return ( +
+ {[1, 2, 3, 4, 5].map((score) => ( +
+ ) + } + + return ( +
+ {[1, 2, 3, 4, 5].map((score) => ( + + onChange(score)} + disabled={disabled} + aria-label={t('skillReviews.ratingOption', { score })} + /> + + + ))} +
+ ) +} + +function ReviewEditor({ skillId, review, onDone }: { + skillId: number + review?: MySkillReview + onDone: () => void +}) { + const { t } = useTranslation() + const reviewTextId = useId() + const [score, setScore] = useState(review?.rated ? review.score : 5) + const [reviewText, setReviewText] = useState(review?.reviewText ?? '') + const save = useUpsertSkillReview(skillId) + const clear = useClearSkillReview(skillId) + const pending = save.isPending || clear.isPending + + const handleSave = () => { + const normalized = reviewText.trim() + if (!normalized) { + toast.error(t('skillReviews.textRequired')) + return + } + save.mutate({ score, reviewText: normalized }, { + onSuccess: () => { + toast.success(t('skillReviews.saved')) + onDone() + }, + onError: (error) => toast.error(t('skillReviews.saveFailed'), error.message), + }) + } + + const handleDelete = () => { + clear.mutate(undefined, { + onSuccess: () => { + toast.success(t('skillReviews.deleted')) + onDone() + }, + onError: (error) => toast.error(t('skillReviews.deleteFailed'), error.message), + }) + } + + return ( +
+
+ {t('skillReviews.scoreLabel')} + +
+ +