diff --git a/server/skillhub-app/pom.xml b/server/skillhub-app/pom.xml index 413e7eaf..a946bf3f 100644 --- a/server/skillhub-app/pom.xml +++ b/server/skillhub-app/pom.xml @@ -13,6 +13,10 @@ skillhub-app + + 1.21.4 + + org.springframework.boot @@ -121,6 +125,16 @@ h2 test + + org.testcontainers + junit-jupiter + test + + + org.testcontainers + postgresql + test + diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java index cbca1ada..8923664b 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/ReviewController.java @@ -8,11 +8,13 @@ import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.dto.PageResponse; import com.iflytek.skillhub.dto.ReviewActionRequest; import com.iflytek.skillhub.dto.ReviewSkillDetailResponse; +import com.iflytek.skillhub.dto.ReviewProgressPageResponse; import com.iflytek.skillhub.dto.ReviewTaskRequest; import com.iflytek.skillhub.dto.ReviewTaskResponse; import com.iflytek.skillhub.service.AuditRequestContext; import com.iflytek.skillhub.service.GovernanceWorkflowAppService; import jakarta.servlet.http.HttpServletRequest; +import java.util.List; import java.util.Map; import org.springframework.core.io.InputStreamResource; import org.springframework.http.HttpHeaders; @@ -136,6 +138,38 @@ public class ReviewController extends BaseApiController { return ok("response.success.read", governanceWorkflowAppService.listMyReviewSubmissions(page, size, userId)); } + @GetMapping("/my-progress") + public ApiResponse listMyProgress( + @RequestParam(required = false) String status, + @RequestParam(defaultValue = "") String q, + @RequestParam(defaultValue = "0") int page, + @RequestParam(defaultValue = "20") int size, + @RequestAttribute("userId") String userId) { + return ok( + "response.success.read", + governanceWorkflowAppService.listMyReviewProgress(status, q, page, size, userId) + ); + } + + @GetMapping("/my-progress/{id}/attempts") + public ApiResponse> listMyAttempts( + @PathVariable Long id, + @RequestAttribute("userId") String userId) { + return ok("response.success.read", governanceWorkflowAppService.listMyReviewAttempts(id, userId)); + } + + @GetMapping("/{id}/attempts") + public ApiResponse> listReviewAttempts( + @PathVariable Long id, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "userNsRoles", required = false) + Map userNsRoles) { + return ok( + "response.success.read", + governanceWorkflowAppService.listReviewAttempts(id, userId, userNsRoles) + ); + } + @GetMapping("/{id}") public ApiResponse getReviewDetail(@PathVariable Long id, @RequestAttribute("userId") String userId, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressPageResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressPageResponse.java new file mode 100644 index 00000000..dfae6715 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressPageResponse.java @@ -0,0 +1,14 @@ +package com.iflytek.skillhub.dto; + +import java.util.List; + +/** + * Author-facing review progress page with search-scoped current-status totals. + */ +public record ReviewProgressPageResponse( + List items, + long total, + int page, + int size, + ReviewProgressStatusCounts statusCounts +) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressResponse.java new file mode 100644 index 00000000..f69885b9 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressResponse.java @@ -0,0 +1,19 @@ +package com.iflytek.skillhub.dto; + +import java.time.Instant; + +/** + * Author-facing summary for one skill version's review attempts. + */ +public record ReviewProgressResponse( + Long latestReviewTaskId, + Long skillId, + String namespace, + String skillSlug, + String skillVersion, + String latestStatus, + String latestReviewComment, + Instant latestSubmittedAt, + Instant latestReviewedAt, + long attemptCount +) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressStatusCounts.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressStatusCounts.java new file mode 100644 index 00000000..a94361b8 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/ReviewProgressStatusCounts.java @@ -0,0 +1,10 @@ +package com.iflytek.skillhub.dto; + +/** + * Current review-status totals for the author's grouped skill-version progress. + */ +public record ReviewProgressStatusCounts( + long pending, + long approved, + long rejected +) {} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaGovernanceQueryRepository.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaGovernanceQueryRepository.java index 646f032e..8b7edab7 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaGovernanceQueryRepository.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaGovernanceQueryRepository.java @@ -104,15 +104,19 @@ public class JpaGovernanceQueryRepository implements GovernanceQueryRepository { ? Map.of() : skillVersionRepository.findByIdIn(versionIds).stream() .collect(Collectors.toMap(SkillVersion::getId, Function.identity())); - List skillIds = distinct(versionsById.values().stream().map(SkillVersion::getSkillId).toList()); + Set skillIds = new LinkedHashSet<>(distinct( + versionsById.values().stream().map(SkillVersion::getSkillId).toList())); + skillIds.addAll(distinct(tasks.stream().map(ReviewTask::getSkillId).toList())); Map skillsById = skillIds.isEmpty() ? Map.of() - : skillRepository.findByIdIn(skillIds).stream() + : skillRepository.findByIdIn(List.copyOf(skillIds)).stream() .collect(Collectors.toMap(Skill::getId, Function.identity())); - List namespaceIds = distinct(skillsById.values().stream().map(Skill::getNamespaceId).toList()); + Set namespaceIds = new LinkedHashSet<>(distinct( + skillsById.values().stream().map(Skill::getNamespaceId).toList())); + namespaceIds.addAll(distinct(tasks.stream().map(ReviewTask::getNamespaceId).toList())); Map namespacesById = namespaceIds.isEmpty() ? Map.of() - : namespaceRepository.findByIdIn(namespaceIds).stream() + : namespaceRepository.findByIdIn(List.copyOf(namespaceIds)).stream() .collect(Collectors.toMap(Namespace::getId, Function.identity())); List userIds = distinctStrings(tasks.stream() .flatMap(task -> java.util.stream.Stream.of(task.getSubmittedBy(), task.getReviewedBy())) @@ -168,9 +172,14 @@ public class JpaGovernanceQueryRepository implements GovernanceQueryRepository { } private ReviewTaskResponse toReviewTaskResponse(ReviewTask task, ReviewReadBundle bundle) { - SkillVersion version = require(bundle.versionsById(), task.getSkillVersionId(), "skill_version.not_found"); - Skill skill = require(bundle.skillsById(), version.getSkillId(), "skill.not_found"); - Namespace namespace = require(bundle.namespacesById(), skill.getNamespaceId(), "namespace.not_found"); + Long skillId = task.getSkillId() != null + ? task.getSkillId() + : require(bundle.versionsById(), task.getSkillVersionId(), "skill_version.not_found").getSkillId(); + Skill skill = require(bundle.skillsById(), skillId, "skill.not_found"); + Namespace namespace = require(bundle.namespacesById(), task.getNamespaceId(), "namespace.not_found"); + String skillVersion = task.getSkillVersion() != null + ? task.getSkillVersion() + : require(bundle.versionsById(), task.getSkillVersionId(), "skill_version.not_found").getVersion(); UserAccount submittedBy = bundle.usersById().get(task.getSubmittedBy()); UserAccount reviewedBy = task.getReviewedBy() != null ? bundle.usersById().get(task.getReviewedBy()) : null; return new ReviewTaskResponse( @@ -178,7 +187,7 @@ public class JpaGovernanceQueryRepository implements GovernanceQueryRepository { task.getSkillVersionId(), namespace.getSlug(), skill.getSlug(), - version.getVersion(), + skillVersion, task.getStatus().name(), task.getSubmittedBy(), submittedBy != null ? submittedBy.getDisplayName() : null, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaReviewProgressQueryRepository.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaReviewProgressQueryRepository.java new file mode 100644 index 00000000..04c09be0 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaReviewProgressQueryRepository.java @@ -0,0 +1,176 @@ +package com.iflytek.skillhub.repository; + +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.dto.ReviewProgressPageResponse; +import com.iflytek.skillhub.dto.ReviewProgressResponse; +import com.iflytek.skillhub.dto.ReviewProgressStatusCounts; +import jakarta.persistence.EntityManager; +import jakarta.persistence.Query; +import java.sql.Timestamp; +import java.time.Instant; +import java.time.OffsetDateTime; +import java.util.List; +import org.springframework.stereotype.Repository; +import org.springframework.transaction.annotation.Transactional; + +/** + * PostgreSQL read-model query for review progress. + * + *

Direct SQL is intentional here: the page boundary applies to grouped skill-version attempts, + * not individual review tasks. Window functions keep grouping, latest-attempt selection, counts, + * filtering, and pagination in the database instead of loading an author's full history.

+ */ +@Repository +public class JpaReviewProgressQueryRepository implements ReviewProgressQueryRepository { + + private static final String RANKED_CTE = """ + WITH ranked AS ( + SELECT task.id, + task.skill_id, + task.namespace_id, + task.skill_version, + task.status, + task.review_comment, + task.submitted_at, + task.reviewed_at, + ROW_NUMBER() OVER ( + PARTITION BY task.skill_id, task.skill_version + ORDER BY task.submitted_at DESC, task.id DESC + ) AS attempt_rank, + COUNT(*) OVER ( + PARTITION BY task.skill_id, task.skill_version + ) AS attempt_count + FROM review_task task + WHERE task.submitted_by = :userId + ), latest AS ( + SELECT * + FROM ranked + WHERE attempt_rank = 1 + ) + """; + + private static final String MY_PROGRESS_SQL = RANKED_CTE + """ + SELECT latest.id, + latest.skill_id, + namespace.slug, + skill.slug, + latest.skill_version, + latest.status, + latest.review_comment, + latest.submitted_at, + latest.reviewed_at, + latest.attempt_count + FROM latest + JOIN skill ON skill.id = latest.skill_id + JOIN namespace ON namespace.id = latest.namespace_id + WHERE ( + :query = '' + OR LOWER(skill.slug) LIKE :queryPattern + OR LOWER(namespace.slug) LIKE :queryPattern + ) + AND (:status = '' OR latest.status = :status) + ORDER BY latest.submitted_at DESC, latest.id DESC + OFFSET :offset ROWS FETCH NEXT :size ROWS ONLY + """; + + private static final String MY_PROGRESS_SUMMARY_SQL = RANKED_CTE + """ + SELECT COUNT(*) FILTER (WHERE :status = '' OR latest.status = :status) AS filtered_total, + COUNT(*) FILTER (WHERE latest.status = 'PENDING') AS pending_count, + COUNT(*) FILTER (WHERE latest.status = 'APPROVED') AS approved_count, + COUNT(*) FILTER (WHERE latest.status = 'REJECTED') AS rejected_count + FROM latest + JOIN skill ON skill.id = latest.skill_id + JOIN namespace ON namespace.id = latest.namespace_id + WHERE :query = '' + OR LOWER(skill.slug) LIKE :queryPattern + OR LOWER(namespace.slug) LIKE :queryPattern + """; + + private final EntityManager entityManager; + + public JpaReviewProgressQueryRepository(EntityManager entityManager) { + this.entityManager = entityManager; + } + + @Override + @Transactional(readOnly = true) + public ReviewProgressPageResponse findMyProgress( + String userId, + ReviewTaskStatus status, + String query, + int page, + int size) { + String normalizedQuery = query == null ? "" : query.trim().toLowerCase(java.util.Locale.ROOT); + String statusName = status != null ? status.name() : ""; + String queryPattern = "%" + normalizedQuery + "%"; + Query nativeQuery = bindFilters( + entityManager.createNativeQuery(MY_PROGRESS_SQL), + userId, + statusName, + normalizedQuery, + queryPattern) + .setParameter("offset", (long) page * size) + .setParameter("size", size); + Query summaryQuery = bindFilters( + entityManager.createNativeQuery(MY_PROGRESS_SUMMARY_SQL), + userId, + statusName, + normalizedQuery, + queryPattern); + + @SuppressWarnings("unchecked") + List rows = nativeQuery.getResultList(); + List items = rows.stream().map(this::mapRow).toList(); + Object[] summary = (Object[]) summaryQuery.getSingleResult(); + long total = number(summary[0]).longValue(); + ReviewProgressStatusCounts statusCounts = new ReviewProgressStatusCounts( + number(summary[1]).longValue(), + number(summary[2]).longValue(), + number(summary[3]).longValue() + ); + return new ReviewProgressPageResponse(items, total, page, size, statusCounts); + } + + private Query bindFilters( + Query query, + String userId, + String status, + String normalizedQuery, + String queryPattern) { + return query + .setParameter("userId", userId) + .setParameter("status", status) + .setParameter("query", normalizedQuery) + .setParameter("queryPattern", queryPattern); + } + + private ReviewProgressResponse mapRow(Object[] row) { + return new ReviewProgressResponse( + number(row[0]).longValue(), + number(row[1]).longValue(), + (String) row[2], + (String) row[3], + (String) row[4], + String.valueOf(row[5]), + (String) row[6], + instant(row[7]), + instant(row[8]), + number(row[9]).longValue() + ); + } + + private Number number(Object value) { + if (value instanceof Number number) { + return number; + } + throw new IllegalStateException("Expected numeric review progress value, got " + value); + } + + private Instant instant(Object value) { + if (value == null) return null; + if (value instanceof Instant instant) return instant; + if (value instanceof OffsetDateTime offsetDateTime) return offsetDateTime.toInstant(); + if (value instanceof Timestamp timestamp) return timestamp.toInstant(); + throw new IllegalStateException("Expected review progress timestamp, got " + value.getClass().getName()); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/ReviewProgressQueryRepository.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/ReviewProgressQueryRepository.java new file mode 100644 index 00000000..535cefbe --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/repository/ReviewProgressQueryRepository.java @@ -0,0 +1,18 @@ +package com.iflytek.skillhub.repository; + +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.dto.ReviewProgressPageResponse; + +/** + * Query seam for author-facing review progress grouped by skill version. + */ +public interface ReviewProgressQueryRepository { + + ReviewProgressPageResponse findMyProgress( + String userId, + ReviewTaskStatus status, + String query, + int page, + int size + ); +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/GovernanceWorkflowAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/GovernanceWorkflowAppService.java index 9aac59d9..ae578ae8 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/GovernanceWorkflowAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/GovernanceWorkflowAppService.java @@ -8,10 +8,12 @@ import com.iflytek.skillhub.dto.NamespaceResponse; import com.iflytek.skillhub.dto.PageResponse; import com.iflytek.skillhub.dto.PromotionResponseDto; import com.iflytek.skillhub.dto.ReviewSkillDetailResponse; +import com.iflytek.skillhub.dto.ReviewProgressPageResponse; import com.iflytek.skillhub.dto.ReviewTaskResponse; import com.iflytek.skillhub.dto.SkillLifecycleMutationResponse; import com.iflytek.skillhub.dto.SkillVersionRereleaseRequest; import java.io.InputStream; +import java.util.List; import java.util.Map; import org.springframework.stereotype.Service; @@ -93,6 +95,26 @@ public class GovernanceWorkflowAppService { return reviewPortalAppService.listMySubmissions(page, size, userId); } + public ReviewProgressPageResponse listMyReviewProgress( + String status, + String query, + int page, + int size, + String userId) { + return reviewPortalAppService.listMyProgress(status, query, page, size, userId); + } + + public List listMyReviewAttempts(Long reviewTaskId, String userId) { + return reviewPortalAppService.listMyAttempts(reviewTaskId, userId); + } + + public List listReviewAttempts( + Long reviewTaskId, + String userId, + Map userNsRoles) { + return reviewPortalAppService.listReviewAttempts(reviewTaskId, userId, userNsRoles); + } + public ReviewTaskResponse getReviewDetail(Long reviewTaskId, String userId, Map userNsRoles) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java index f944f2d5..6d68193f 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java @@ -13,9 +13,11 @@ import com.iflytek.skillhub.domain.review.ReviewTaskStatus; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.dto.PageResponse; +import com.iflytek.skillhub.dto.ReviewProgressPageResponse; import com.iflytek.skillhub.dto.ReviewTaskResponse; import com.iflytek.skillhub.observability.RequestIdAccessor; import com.iflytek.skillhub.repository.GovernanceQueryRepository; +import com.iflytek.skillhub.repository.ReviewProgressQueryRepository; import java.util.List; import java.util.Map; import java.util.Set; @@ -34,6 +36,7 @@ public class ReviewPortalAppService { private final ReviewTaskRepository reviewTaskRepository; private final NamespaceRepository namespaceRepository; private final GovernanceQueryRepository governanceQueryRepository; + private final ReviewProgressQueryRepository reviewProgressQueryRepository; private final RbacService rbacService; private final AuditLogService auditLogService; private final RequestIdAccessor requestIdAccessor; @@ -42,6 +45,7 @@ public class ReviewPortalAppService { ReviewTaskRepository reviewTaskRepository, NamespaceRepository namespaceRepository, GovernanceQueryRepository governanceQueryRepository, + ReviewProgressQueryRepository reviewProgressQueryRepository, RbacService rbacService, AuditLogService auditLogService, RequestIdAccessor requestIdAccessor) { @@ -49,6 +53,7 @@ public class ReviewPortalAppService { this.reviewTaskRepository = reviewTaskRepository; this.namespaceRepository = namespaceRepository; this.governanceQueryRepository = governanceQueryRepository; + this.reviewProgressQueryRepository = reviewProgressQueryRepository; this.rbacService = rbacService; this.auditLogService = auditLogService; this.requestIdAccessor = requestIdAccessor; @@ -212,6 +217,63 @@ public class ReviewPortalAppService { )); } + public ReviewProgressPageResponse listMyProgress( + String status, + String query, + int page, + int size, + String userId) { + ReviewTaskStatus reviewStatus = status == null || status.isBlank() + ? null + : ReviewTaskStatus.valueOf(status.toUpperCase(java.util.Locale.ROOT)); + int safePage = Math.max(page, 0); + int safeSize = Math.min(Math.max(size, 1), 100); + return reviewProgressQueryRepository.findMyProgress( + userId, + reviewStatus, + query != null ? query : "", + safePage, + safeSize + ); + } + + public List listMyAttempts(Long reviewTaskId, String userId) { + ReviewTask anchor = reviewTaskRepository.findById(reviewTaskId) + .orElseThrow(() -> new DomainNotFoundException("review_task.not_found", reviewTaskId)); + if (!anchor.getSubmittedBy().equals(userId)) { + throw new DomainForbiddenException("review.no_permission"); + } + + List attempts = reviewTaskRepository + .findBySubmittedByAndSkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + userId, anchor.getSkillId(), anchor.getSkillVersion()); + return governanceQueryRepository.getReviewTaskResponses(attempts); + } + + public List listReviewAttempts( + Long reviewTaskId, + String userId, + Map userNsRoles) { + ReviewTask anchor = reviewTaskRepository.findById(reviewTaskId) + .orElseThrow(() -> new DomainNotFoundException("review_task.not_found", reviewTaskId)); + Namespace namespace = namespaceRepository.findById(anchor.getNamespaceId()) + .orElseThrow(() -> new DomainNotFoundException( + "namespace.not_found", anchor.getNamespaceId())); + if (!reviewService.canReviewNamespace( + anchor, + userId, + namespace.getType(), + normalizeRoles(userNsRoles), + platformRoles(userId))) { + throw new DomainForbiddenException("review.no_permission"); + } + + List attempts = reviewTaskRepository + .findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + anchor.getSkillId(), anchor.getSkillVersion()); + return governanceQueryRepository.getReviewTaskResponses(attempts); + } + public ReviewTaskResponse getReviewDetail(Long reviewTaskId, String userId, Map userNsRoles) { diff --git a/server/skillhub-app/src/main/resources/db/migration/V46__preserve_review_attempt_history.sql b/server/skillhub-app/src/main/resources/db/migration/V46__preserve_review_attempt_history.sql new file mode 100644 index 00000000..3249f4e8 --- /dev/null +++ b/server/skillhub-app/src/main/resources/db/migration/V46__preserve_review_attempt_history.sql @@ -0,0 +1,27 @@ +ALTER TABLE review_task + ADD COLUMN skill_id BIGINT, + ADD COLUMN skill_version VARCHAR(64); + +UPDATE review_task task +SET skill_id = version.skill_id, + skill_version = version.version +FROM skill_version version +WHERE task.skill_version_id = version.id; + +ALTER TABLE review_task + ALTER COLUMN skill_id SET NOT NULL, + ALTER COLUMN skill_version SET NOT NULL, + ALTER COLUMN skill_version_id DROP NOT NULL; + +ALTER TABLE review_task + DROP CONSTRAINT review_task_skill_version_id_fkey, + ADD CONSTRAINT fk_review_task_skill_version + FOREIGN KEY (skill_version_id) REFERENCES skill_version(id) ON DELETE SET NULL, + ADD CONSTRAINT fk_review_task_skill + FOREIGN KEY (skill_id) REFERENCES skill(id); + +CREATE INDEX idx_review_task_submitter_submitted + ON review_task(submitted_by, submitted_at DESC, id DESC); + +CREATE INDEX idx_review_task_skill_version_attempts + ON review_task(skill_id, skill_version, submitted_at DESC, id DESC); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java index 1724c786..a3016841 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java @@ -16,11 +16,15 @@ import com.iflytek.skillhub.domain.review.ReviewTaskStatus; import com.iflytek.skillhub.domain.skill.service.SkillDownloadService; import com.iflytek.skillhub.dto.ReviewTaskResponse; import com.iflytek.skillhub.dto.ReviewSkillDetailResponse; +import com.iflytek.skillhub.dto.ReviewProgressResponse; +import com.iflytek.skillhub.dto.ReviewProgressPageResponse; +import com.iflytek.skillhub.dto.ReviewProgressStatusCounts; import com.iflytek.skillhub.dto.SkillDetailResponse; import com.iflytek.skillhub.dto.SkillFileResponse; import com.iflytek.skillhub.dto.SkillLifecycleVersionResponse; import com.iflytek.skillhub.dto.SkillVersionResponse; import com.iflytek.skillhub.repository.GovernanceQueryRepository; +import com.iflytek.skillhub.repository.ReviewProgressQueryRepository; import com.iflytek.skillhub.service.ReviewSkillDetailAppService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -38,6 +42,7 @@ import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.RequestPostProcessor; import java.util.List; +import java.time.Instant; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -79,6 +84,9 @@ class ReviewPortalControllerTest { @MockBean private GovernanceQueryRepository governanceQueryRepository; + @MockBean + private ReviewProgressQueryRepository reviewProgressQueryRepository; + @MockBean private RbacService rbacService; @@ -276,6 +284,188 @@ class ReviewPortalControllerTest { verify(reviewTaskRepository, never()).findByStatus(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); } + @Test + void listMyReviewProgress_isScopedToAuthenticatedAuthor() throws Exception { + var item = new ReviewProgressResponse( + 12L, + 30L, + "team-a", + "skill-a", + "1.0.0", + "REJECTED", + "Please add tests", + Instant.parse("2026-08-31T10:00:00Z"), + Instant.parse("2026-08-31T11:00:00Z"), + 2L + ); + given(reviewProgressQueryRepository.findMyProgress("author-1", null, "", 0, 20)) + .willReturn(new ReviewProgressPageResponse( + List.of(item), + 1, + 0, + 20, + new ReviewProgressStatusCounts(0, 0, 1) + )); + + mockMvc.perform(get("/api/v1/reviews/my-progress").with(auth("author-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.items[0].skillSlug").value("skill-a")) + .andExpect(jsonPath("$.data.items[0].attemptCount").value(2)) + .andExpect(jsonPath("$.data.items[0].latestStatus").value("REJECTED")) + .andExpect(jsonPath("$.data.statusCounts.pending").value(0)) + .andExpect(jsonPath("$.data.statusCounts.rejected").value(1)); + + verify(reviewProgressQueryRepository).findMyProgress("author-1", null, "", 0, 20); + } + + @Test + void listMyReviewAttempts_returnsOnlyTheAuthorsVersionHistory() throws Exception { + ReviewTask latest = createReviewTask(12L, 20L, "author-1", ReviewTaskStatus.REJECTED); + setField(latest, "skillId", 30L); + setField(latest, "skillVersion", "1.0.0"); + ReviewTask previous = createReviewTask(8L, 20L, "author-1", ReviewTaskStatus.REJECTED); + setField(previous, "skillId", 30L); + setField(previous, "skillVersion", "1.0.0"); + ReviewTask otherAuthor = createReviewTask(7L, 20L, "author-2", ReviewTaskStatus.REJECTED); + setField(otherAuthor, "skillId", 30L); + setField(otherAuthor, "skillVersion", "1.0.0"); + given(reviewTaskRepository.findById(12L)).willReturn(Optional.of(latest)); + given(reviewTaskRepository.findBySubmittedByAndSkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + "author-1", 30L, "1.0.0")) + .willReturn(List.of(latest, previous)); + given(reviewTaskRepository.findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc(30L, "1.0.0")) + .willReturn(List.of(latest, previous, otherAuthor)); + given(governanceQueryRepository.getReviewTaskResponses(List.of(latest, previous))) + .willReturn(List.of(toReviewResponse(latest), toReviewResponse(previous))); + + mockMvc.perform(get("/api/v1/reviews/my-progress/12/attempts").with(auth("author-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.length()").value(2)) + .andExpect(jsonPath("$.data[0].id").value(12)) + .andExpect(jsonPath("$.data[1].id").value(8)) + .andExpect(jsonPath("$.data[0].submittedBy").value("author-1")) + .andExpect(jsonPath("$.data[1].submittedBy").value("author-1")); + + verify(reviewTaskRepository, never()) + .findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc(30L, "1.0.0"); + } + + @Test + void listReviewAttempts_allowsAuthorizedReviewerToReadVersionHistory() throws Exception { + ReviewTask latest = createReviewTask(12L, 20L, "author-1", ReviewTaskStatus.PENDING); + setField(latest, "skillId", 30L); + setField(latest, "skillVersion", "1.0.0"); + ReviewTask previous = createReviewTask(8L, 20L, "author-2", ReviewTaskStatus.REJECTED); + setField(previous, "skillId", 30L); + setField(previous, "skillVersion", "1.0.0"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("reviewer-1", List.of()); + given(rbacService.getUserRoleCodes("reviewer-1")).willReturn(Set.of("SKILL_ADMIN")); + given(reviewTaskRepository.findById(12L)).willReturn(Optional.of(latest)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(reviewService.canReviewNamespace( + latest, + "reviewer-1", + namespace.getType(), + Map.of(), + Set.of("SKILL_ADMIN"))).willReturn(true); + given(reviewTaskRepository.findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc(30L, "1.0.0")) + .willReturn(List.of(latest, previous)); + given(governanceQueryRepository.getReviewTaskResponses(List.of(latest, previous))) + .willReturn(List.of(toReviewResponse(latest), toReviewResponse(previous))); + + mockMvc.perform(get("/api/v1/reviews/12/attempts").with(auth("reviewer-1"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.length()").value(2)) + .andExpect(jsonPath("$.data[0].id").value(12)) + .andExpect(jsonPath("$.data[1].id").value(8)) + .andExpect(jsonPath("$.data[0].submittedBy").value("author-1")) + .andExpect(jsonPath("$.data[1].submittedBy").value("author-2")); + } + + @Test + void listReviewAttempts_allowsNamespaceAdminToReadCrossAuthorHistory() throws Exception { + ReviewTask latest = createReviewTask(12L, 20L, "author-1", ReviewTaskStatus.PENDING); + setField(latest, "skillId", 30L); + setField(latest, "skillVersion", "1.0.0"); + ReviewTask previous = createReviewTask(8L, 20L, "author-2", ReviewTaskStatus.REJECTED); + setField(previous, "skillId", 30L); + setField(previous, "skillVersion", "1.0.0"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("namespace-admin", List.of(new NamespaceMember( + 20L, "namespace-admin", NamespaceRole.ADMIN))); + given(rbacService.getUserRoleCodes("namespace-admin")).willReturn(Set.of()); + given(reviewTaskRepository.findById(12L)).willReturn(Optional.of(latest)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(reviewService.canReviewNamespace( + latest, + "namespace-admin", + namespace.getType(), + Map.of(20L, NamespaceRole.ADMIN), + Set.of())).willReturn(true); + given(reviewTaskRepository.findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc(30L, "1.0.0")) + .willReturn(List.of(latest, previous)); + given(governanceQueryRepository.getReviewTaskResponses(List.of(latest, previous))) + .willReturn(List.of(toReviewResponse(latest), toReviewResponse(previous))); + + mockMvc.perform(get("/api/v1/reviews/12/attempts").with(auth("namespace-admin"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.length()").value(2)) + .andExpect(jsonPath("$.data[0].submittedBy").value("author-1")) + .andExpect(jsonPath("$.data[1].submittedBy").value("author-2")); + } + + @Test + void listReviewAttempts_forbidsSubmitterWithoutReviewerRole() throws Exception { + ReviewTask ownAttempt = createReviewTask(12L, 20L, "author-1", ReviewTaskStatus.REJECTED); + setField(ownAttempt, "skillId", 30L); + setField(ownAttempt, "skillVersion", "1.0.0"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("author-1", List.of(new NamespaceMember( + 20L, "author-1", NamespaceRole.MEMBER))); + given(rbacService.getUserRoleCodes("author-1")).willReturn(Set.of()); + given(reviewTaskRepository.findById(12L)).willReturn(Optional.of(ownAttempt)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(reviewService.canReviewNamespace( + ownAttempt, + "author-1", + namespace.getType(), + Map.of(20L, NamespaceRole.MEMBER), + Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/reviews/12/attempts").with(auth("author-1"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + + verify(reviewTaskRepository, never()) + .findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc(30L, "1.0.0"); + } + + @Test + void listReviewAttempts_forbidsUnrelatedUser() throws Exception { + ReviewTask latest = createReviewTask(12L, 20L, "author-1", ReviewTaskStatus.PENDING); + setField(latest, "skillId", 30L); + setField(latest, "skillVersion", "1.0.0"); + Namespace namespace = createNamespace(20L, "team-a"); + stubNamespaceRoles("other-user", List.of()); + given(rbacService.getUserRoleCodes("other-user")).willReturn(Set.of()); + given(reviewTaskRepository.findById(12L)).willReturn(Optional.of(latest)); + given(namespaceRepository.findById(20L)).willReturn(Optional.of(namespace)); + given(reviewService.canReviewNamespace( + latest, + "other-user", + namespace.getType(), + Map.of(), + Set.of())).willReturn(false); + + mockMvc.perform(get("/api/v1/reviews/12/attempts").with(auth("other-user"))) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + + verify(reviewTaskRepository, never()) + .findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc(30L, "1.0.0"); + } + @Test void downloadReviewVersion_streamsZipForAuthorizedReviewer() throws Exception { stubNamespaceRoles("admin", List.of()); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillApprovalVisibilityFlowIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillApprovalVisibilityFlowIntegrationTest.java index 081f5133..ee467c27 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillApprovalVisibilityFlowIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillApprovalVisibilityFlowIntegrationTest.java @@ -212,7 +212,8 @@ class SkillApprovalVisibilityFlowIntegrationTest { version = skillVersionRepository.save(version); skillVersionRepository.flush(); - ReviewTask reviewTask = reviewTaskJpaRepository.saveAndFlush(new ReviewTask(version.getId(), namespace.getId(), ownerId)); + ReviewTask reviewTask = reviewTaskJpaRepository.saveAndFlush(new ReviewTask( + version.getId(), skill.getId(), namespace.getId(), version.getVersion(), ownerId)); return new PendingSkillGraph(namespace, skill, version, reviewTask); } @@ -237,7 +238,8 @@ class SkillApprovalVisibilityFlowIntegrationTest { version = skillVersionRepository.save(version); skillVersionRepository.flush(); - ReviewTask reviewTask = reviewTaskJpaRepository.saveAndFlush(new ReviewTask(version.getId(), namespace.getId(), ownerId)); + ReviewTask reviewTask = reviewTaskJpaRepository.saveAndFlush(new ReviewTask( + version.getId(), skill.getId(), namespace.getId(), version.getVersion(), ownerId)); return new PendingSkillGraph(namespace, skill, version, reviewTask); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillVersionDeleteFlowIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillVersionDeleteFlowIntegrationTest.java index 25d585e5..65fbaf15 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillVersionDeleteFlowIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillVersionDeleteFlowIntegrationTest.java @@ -91,15 +91,18 @@ class SkillVersionDeleteFlowIntegrationTest { retainedVersion.setStatus(SkillVersionStatus.REJECTED); retainedVersion = skillVersionRepository.save(retainedVersion); - ReviewTask rejectedTask = new ReviewTask(rejectedVersion.getId(), namespace.getId(), ownerId); + ReviewTask rejectedTask = new ReviewTask( + rejectedVersion.getId(), skill.getId(), namespace.getId(), rejectedVersion.getVersion(), ownerId); rejectedTask.setStatus(ReviewTaskStatus.REJECTED); rejectedTask = reviewTaskRepository.save(rejectedTask); - ReviewTask approvedTask = new ReviewTask(rejectedVersion.getId(), namespace.getId(), ownerId); + ReviewTask approvedTask = new ReviewTask( + rejectedVersion.getId(), skill.getId(), namespace.getId(), rejectedVersion.getVersion(), ownerId); approvedTask.setStatus(ReviewTaskStatus.APPROVED); approvedTask = reviewTaskRepository.save(approvedTask); - ReviewTask retainedTask = new ReviewTask(retainedVersion.getId(), namespace.getId(), ownerId); + ReviewTask retainedTask = new ReviewTask( + retainedVersion.getId(), skill.getId(), namespace.getId(), retainedVersion.getVersion(), ownerId); retainedTask.setStatus(ReviewTaskStatus.REJECTED); retainedTask = reviewTaskRepository.save(retainedTask); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaGovernanceQueryRepositoryTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaGovernanceQueryRepositoryTest.java index beb53df9..0d019550 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaGovernanceQueryRepositoryTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaGovernanceQueryRepositoryTest.java @@ -86,6 +86,32 @@ class JpaGovernanceQueryRepositoryTest { assertThat(responses.get(0).reviewedByName()).isEqualTo("Reviewer"); } + @Test + void getReviewTaskResponses_usesSnapshotAfterReviewedVersionIsReplaced() { + ReviewTask task = new ReviewTask(null, 201L, 11L, "1.2.0", "submitter"); + setField(task, "id", 6L); + setField(task, "status", ReviewTaskStatus.REJECTED); + + Skill skill = new Skill(11L, "skill-a", "submitter", SkillVisibility.PUBLIC); + setField(skill, "id", 201L); + Namespace namespace = new Namespace("team-a", "Team A", "submitter"); + setField(namespace, "id", 11L); + UserAccount submitter = new UserAccount("submitter", "Submitter", "submitter@example.com", null); + + given(skillRepository.findByIdIn(List.of(201L))).willReturn(List.of(skill)); + given(namespaceRepository.findByIdIn(List.of(11L))).willReturn(List.of(namespace)); + given(userAccountRepository.findByIdIn(List.of("submitter"))).willReturn(List.of(submitter)); + + var responses = repository.getReviewTaskResponses(List.of(task)); + + assertThat(responses).singleElement().satisfies(response -> { + assertThat(response.skillVersionId()).isNull(); + assertThat(response.namespace()).isEqualTo("team-a"); + assertThat(response.skillSlug()).isEqualTo("skill-a"); + assertThat(response.version()).isEqualTo("1.2.0"); + }); + } + @Test void getPromotionResponses_assemblesPromotionReadModel() { PromotionRequest request = new PromotionRequest(201L, 101L, 12L, "submitter"); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaReviewProgressQueryRepositoryTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaReviewProgressQueryRepositoryTest.java new file mode 100644 index 00000000..b08edf39 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/JpaReviewProgressQueryRepositoryTest.java @@ -0,0 +1,159 @@ +package com.iflytek.skillhub.repository; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.review.ReviewTask; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import java.time.Instant; +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.boot.test.autoconfigure.orm.jpa.TestEntityManager; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.test.context.ActiveProfiles; +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") +@Import(JpaReviewProgressQueryRepository.class) +@Testcontainers +class JpaReviewProgressQueryRepositoryTest { + + @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 TestEntityManager entityManager; + + @Autowired + private JpaReviewProgressQueryRepository repository; + + @Test + void groupsAttemptsFiltersLatestStatusAndKeepsTotalsOnEmptyPage() { + Namespace namespace = entityManager.persistFlushFind( + new Namespace("team-review", "Review Team", "owner")); + Skill alpha = entityManager.persistFlushFind( + new Skill(namespace.getId(), "alpha-skill", "author-1", SkillVisibility.PUBLIC)); + Skill beta = entityManager.persistFlushFind( + new Skill(namespace.getId(), "beta-skill", "author-1", SkillVisibility.PUBLIC)); + Skill gamma = entityManager.persistFlushFind( + new Skill(namespace.getId(), "gamma-skill", "author-1", SkillVisibility.PUBLIC)); + + persistAttempt( + alpha, + namespace, + "author-1", + "1.0.0", + ReviewTaskStatus.REJECTED, + Instant.parse("2026-08-30T10:00:00Z")); + persistAttempt( + alpha, + namespace, + "author-1", + "1.0.0", + ReviewTaskStatus.PENDING, + Instant.parse("2026-08-31T10:00:00Z")); + persistAttempt( + beta, + namespace, + "author-1", + "2.0.0", + ReviewTaskStatus.APPROVED, + Instant.parse("2026-08-29T10:00:00Z")); + persistAttempt( + gamma, + namespace, + "author-1", + "3.0.0", + ReviewTaskStatus.REJECTED, + Instant.parse("2026-08-28T10:00:00Z")); + persistAttempt( + beta, + namespace, + "other-author", + "3.0.0", + ReviewTaskStatus.REJECTED, + Instant.parse("2026-08-31T11:00:00Z")); + entityManager.flush(); + entityManager.clear(); + + var firstPage = repository.findMyProgress("author-1", null, "", 0, 1); + + assertThat(firstPage.items()).hasSize(1); + assertThat(firstPage.total()).isEqualTo(3); + assertThat(firstPage.items()).singleElement().satisfies(item -> { + assertThat(item.skillSlug()).isEqualTo("alpha-skill"); + assertThat(item.latestStatus()).isEqualTo("PENDING"); + assertThat(item.attemptCount()).isEqualTo(2); + }); + assertThat(firstPage.statusCounts().pending()).isEqualTo(1); + assertThat(firstPage.statusCounts().approved()).isEqualTo(1); + assertThat(firstPage.statusCounts().rejected()).isEqualTo(1); + + var emptyPage = repository.findMyProgress("author-1", null, "", 8, 1); + assertThat(emptyPage.items()).isEmpty(); + assertThat(emptyPage.total()).isEqualTo(3); + + var maximumPage = repository.findMyProgress( + "author-1", null, "", Integer.MAX_VALUE, 100); + assertThat(maximumPage.items()).isEmpty(); + assertThat(maximumPage.total()).isEqualTo(3); + + var searchedAndFiltered = repository.findMyProgress( + "author-1", ReviewTaskStatus.APPROVED, "BETA", 0, 20); + assertThat(searchedAndFiltered.items()).singleElement() + .satisfies(item -> assertThat(item.skillSlug()).isEqualTo("beta-skill")); + assertThat(searchedAndFiltered.total()).isEqualTo(1); + assertThat(searchedAndFiltered.statusCounts().approved()).isEqualTo(1); + + var searchMiss = repository.findMyProgress("author-1", null, "missing", 0, 20); + assertThat(searchMiss.items()).isEmpty(); + assertThat(searchMiss.total()).isZero(); + assertThat(searchMiss.statusCounts().pending()).isZero(); + assertThat(searchMiss.statusCounts().approved()).isZero(); + assertThat(searchMiss.statusCounts().rejected()).isZero(); + } + + private void persistAttempt( + Skill skill, + Namespace namespace, + String author, + String version, + ReviewTaskStatus status, + Instant submittedAt) { + ReviewTask task = new ReviewTask( + null, skill.getId(), namespace.getId(), version, author); + task.setStatus(status); + setField(task, "submittedAt", submittedAt); + entityManager.persist(task); + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (ReflectiveOperationException error) { + throw new AssertionError(error); + } + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java index 07d7d9d6..b277f484 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java @@ -611,6 +611,18 @@ class ScanTaskConsumerTest { throw unsupported(); } + @Override + public List findBySubmittedByAndSkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + String submittedBy, Long skillId, String skillVersion) { + throw unsupported(); + } + + @Override + public List findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + Long skillId, String skillVersion) { + throw unsupported(); + } + @Override public boolean existsByNamespaceId(Long namespaceId) { return false; @@ -621,6 +633,11 @@ class ScanTaskConsumerTest { throw unsupported(); } + @Override + public void deleteBySkillId(Long skillId) { + throw unsupported(); + } + @Override public void delete(ReviewTask reviewTask) { this.deletedTask = reviewTask; diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java index 18275f3e..98b19649 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java @@ -111,7 +111,8 @@ public class ReviewService { skillVersion.setStatus(SkillVersionStatus.PENDING_REVIEW); skillVersionRepository.save(skillVersion); - ReviewTask task = new ReviewTask(skillVersionId, skill.getNamespaceId(), userId); + ReviewTask task = new ReviewTask( + skillVersionId, skill.getId(), skill.getNamespaceId(), skillVersion.getVersion(), userId); try { ReviewTask saved = reviewTaskRepository.save(task); eventPublisher.publishEvent(new ReviewSubmittedEvent( @@ -153,7 +154,8 @@ public class ReviewService { skillVersion.setStatus(SkillVersionStatus.PENDING_REVIEW); skillVersionRepository.save(skillVersion); - ReviewTask task = new ReviewTask(skillVersionId, skill.getNamespaceId(), userId); + ReviewTask task = new ReviewTask( + skillVersionId, skill.getId(), skill.getNamespaceId(), skillVersion.getVersion(), userId); try { ReviewTask saved = reviewTaskRepository.save(task); eventPublisher.publishEvent(new ReviewSubmittedEvent( diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTask.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTask.java index 4ccd6786..9b908939 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTask.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTask.java @@ -11,9 +11,15 @@ public class ReviewTask { @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; - @Column(name = "skill_version_id", nullable = false) + @Column(name = "skill_version_id") private Long skillVersionId; + @Column(name = "skill_id", nullable = false) + private Long skillId; + + @Column(name = "skill_version", nullable = false, length = 64) + private String skillVersion; + @Column(name = "namespace_id", nullable = false) private Long namespaceId; @@ -49,10 +55,23 @@ public class ReviewTask { this.submittedBy = submittedBy; } + public ReviewTask(Long skillVersionId, Long skillId, Long namespaceId, + String skillVersion, String submittedBy) { + this.skillVersionId = skillVersionId; + this.skillId = skillId; + this.namespaceId = namespaceId; + this.skillVersion = skillVersion; + this.submittedBy = submittedBy; + } + public Long getId() { return id; } public Long getSkillVersionId() { return skillVersionId; } + public Long getSkillId() { return skillId; } + + public String getSkillVersion() { return skillVersion; } + public Long getNamespaceId() { return namespaceId; } public ReviewTaskStatus getStatus() { return status; } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTaskRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTaskRepository.java index 25c7d9f0..3f0faccb 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTaskRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTaskRepository.java @@ -3,6 +3,7 @@ package com.iflytek.skillhub.domain.review; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import java.util.Collection; +import java.util.List; import java.util.Optional; /** @@ -15,8 +16,13 @@ public interface ReviewTaskRepository { Page findByStatus(ReviewTaskStatus status, Pageable pageable); Page findByNamespaceIdAndStatus(Long namespaceId, ReviewTaskStatus status, Pageable pageable); Page findBySubmittedByAndStatus(String submittedBy, ReviewTaskStatus status, Pageable pageable); + List findBySubmittedByAndSkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + String submittedBy, Long skillId, String skillVersion); + List findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + Long skillId, String skillVersion); boolean existsByNamespaceId(Long namespaceId); void deleteBySkillVersionIdIn(Collection skillVersionIds); + void deleteBySkillId(Long skillId); void delete(ReviewTask reviewTask); int updateStatusWithVersion(Long id, ReviewTaskStatus status, String reviewedBy, String reviewComment, Integer expectedVersion); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteService.java index dbfd9206..e548a608 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteService.java @@ -107,9 +107,8 @@ public class SkillHardDeleteService { skillRepository.save(skill); skillRepository.flush(); - if (!versionIds.isEmpty()) { - reviewTaskRepository.deleteBySkillVersionIdIn(versionIds); - } + // Also removes detached historical attempts whose replaced skill version no longer exists. + reviewTaskRepository.deleteBySkillId(skill.getId()); promotionRequestRepository.deleteBySourceSkillIdOrTargetSkillId(skill.getId(), skill.getId()); skillTagRepository.deleteBySkillId(skill.getId()); skillStarRepository.deleteBySkillId(skill.getId()); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java index bc6e0b0a..8313534f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java @@ -561,7 +561,8 @@ public class SkillPublishService { // Create review task for PUBLIC/NAMESPACE_ONLY (not PRIVATE) if (!autoPublish && visibility != SkillVisibility.PRIVATE) { - ReviewTask reviewTask = new ReviewTask(version.getId(), namespace.getId(), publisherId); + ReviewTask reviewTask = new ReviewTask( + version.getId(), skill.getId(), namespace.getId(), version.getVersion(), publisherId); ReviewTask savedReviewTask = reviewTaskRepository.save(reviewTask); eventPublisher.publishEvent(new ReviewSubmittedEvent( savedReviewTask.getId(), @@ -608,10 +609,11 @@ public class SkillPublishService { skillRepository.flush(); } - // Every review task referencing this version has to go, not just a PENDING one: - // a rejected version still owns a REJECTED task whose foreign key blocks the - // skill_version delete below, which surfaces to the caller as an HTTP 500. - reviewTaskRepository.deleteBySkillVersionIdIn(List.of(version.getId())); + // A replaceable version may still have one obsolete pending task, but settled attempts are + // durable governance history. The database detaches those settled attempts from the + // replaced version while retaining their skill/version snapshot. + reviewTaskRepository.findBySkillVersionIdAndStatus(version.getId(), ReviewTaskStatus.PENDING) + .ifPresent(reviewTaskRepository::delete); List files = skillFileRepository.findByVersionId(version.getId()); List storageKeys = new ArrayList<>(); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitService.java index 0b2e396b..348693a1 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitService.java @@ -95,7 +95,8 @@ public class SkillReviewSubmitService { skillVersionRepository.save(version); // Create review task - ReviewTask reviewTask = new ReviewTask(versionId, skill.getNamespaceId(), actorUserId); + ReviewTask reviewTask = new ReviewTask( + versionId, skill.getId(), skill.getNamespaceId(), version.getVersion(), actorUserId); reviewTaskRepository.save(reviewTask); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteServiceTest.java index 8808acd4..d50c40b0 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteServiceTest.java @@ -124,7 +124,7 @@ class SkillHardDeleteServiceTest { inOrder.verify(skillRepository).save(skill); inOrder.verify(skillRepository).flush(); inOrder.verify(skillVersionRepository).deleteBySkillId(7L); - verify(reviewTaskRepository).deleteBySkillVersionIdIn(List.of(21L, 22L)); + verify(reviewTaskRepository).deleteBySkillId(7L); verify(promotionRequestRepository).deleteBySourceSkillIdOrTargetSkillId(7L, 7L); verify(skillTagRepository).deleteBySkillId(7L); verify(skillStarRepository).deleteBySkillId(7L); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java index a680e320..c0fda2b6 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java @@ -424,7 +424,7 @@ class SkillPublishServiceTest { } @Test - void testPublishFromEntries_ShouldReplaceRejectedVersionWithSameVersion() throws Exception { + void testPublishFromEntries_ShouldPreserveSettledReviewHistoryWhenReplacingRejectedVersion() throws Exception { String namespaceSlug = "test-ns"; String publisherId = "user-100"; String skillMdContent = "---\nname: test-skill\ndescription: Test\nversion: 1.0.0\n---\nBody"; @@ -473,7 +473,7 @@ class SkillPublishServiceTest { assertEquals("1.0.0", result.version().getVersion()); assertEquals(SkillVersionStatus.PENDING_REVIEW, result.version().getStatus()); - verify(reviewTaskRepository).deleteBySkillVersionIdIn(List.of(8L)); + verify(reviewTaskRepository, never()).deleteBySkillVersionIdIn(List.of(8L)); verify(skillFileRepository).deleteByVersionId(8L); verify(skillVersionRepository).delete(rejectedVersion); verify(skillVersionRepository, times(2)).flush(); @@ -482,6 +482,8 @@ class SkillPublishServiceTest { ArgumentCaptor reviewTaskCaptor = ArgumentCaptor.forClass(ReviewTask.class); verify(reviewTaskRepository).save(reviewTaskCaptor.capture()); assertEquals(result.version().getId(), reviewTaskCaptor.getValue().getSkillVersionId()); + assertEquals(skill.getId(), reviewTaskCaptor.getValue().getSkillId()); + assertEquals("1.0.0", reviewTaskCaptor.getValue().getSkillVersion()); assertEquals(publisherId, reviewTaskCaptor.getValue().getSubmittedBy()); } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/ReviewTaskJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/ReviewTaskJpaRepository.java index 286c0eab..d742064c 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/ReviewTaskJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/ReviewTaskJpaRepository.java @@ -11,6 +11,7 @@ import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; import java.util.Collection; +import java.util.List; import java.util.Optional; /** @@ -28,10 +29,18 @@ public interface ReviewTaskJpaRepository extends JpaRepository Page findBySubmittedByAndStatus(String submittedBy, ReviewTaskStatus status, Pageable pageable); + List findBySubmittedByAndSkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + String submittedBy, Long skillId, String skillVersion); + + List findBySkillIdAndSkillVersionOrderBySubmittedAtDescIdDesc( + Long skillId, String skillVersion); + boolean existsByNamespaceId(Long namespaceId); void deleteBySkillVersionIdIn(Collection skillVersionIds); + void deleteBySkillId(Long skillId); + @Modifying @Query(""" UPDATE ReviewTask t diff --git a/web/e2e/rejected-version-republish.spec.ts b/web/e2e/rejected-version-republish.spec.ts index cbbcada4..8f4911e6 100644 --- a/web/e2e/rejected-version-republish.spec.ts +++ b/web/e2e/rejected-version-republish.spec.ts @@ -1,6 +1,6 @@ import { expect, test } from '@playwright/test' import { setEnglishLocale } from './helpers/auth-fixtures' -import { loginWithCredentials, registerSession } from './helpers/session' +import { createFreshSession, loginWithCredentials } from './helpers/session' import { E2eTestDataBuilder } from './helpers/test-data-builder' function getOptionalEnv(name: string): string | undefined { @@ -15,20 +15,39 @@ function adminCredentials() { } } +function withoutKnownMetaCspWarning(messages: string[]): string[] { + return messages.filter((message) => ( + !message.includes("frame-ancestors' is ignored when delivered via a element") + )) +} + test.describe('Rejected version replacement (Real API)', () => { test.describe.configure({ timeout: 150_000 }) test.beforeEach(async ({ page }, testInfo) => { await setEnglishLocale(page) - await registerSession(page, testInfo) + await createFreshSession(page, testInfo) }) test('re-publishes the same version after rejection', async ({ page, browser }, testInfo) => { + const consoleErrors: string[] = [] + const pageErrors: string[] = [] + const adminConsoleErrors: string[] = [] + const adminPageErrors: string[] = [] + page.on('console', (message) => { + if (message.type() === 'error') consoleErrors.push(message.text()) + }) + page.on('pageerror', (error) => pageErrors.push(error.message)) + const publisherBuilder = new E2eTestDataBuilder(page, testInfo) await publisherBuilder.init() const adminContext = await browser.newContext() const adminPage = await adminContext.newPage() + adminPage.on('console', (message) => { + if (message.type() === 'error') adminConsoleErrors.push(message.text()) + }) + adminPage.on('pageerror', (error) => adminPageErrors.push(error.message)) const adminBuilder = new E2eTestDataBuilder(adminPage, testInfo) await loginWithCredentials(adminPage, adminCredentials(), testInfo) await adminBuilder.init() @@ -52,6 +71,23 @@ test.describe('Rejected version replacement (Real API)', () => { 'PENDING_REVIEW', ) await adminBuilder.rejectReview(rejectedReviewId) + await publisherBuilder.waitForVersionStatus( + namespace.slug, + firstPublish.slug, + firstPublish.version, + 'REJECTED', + ) + + await page.goto('/dashboard/review-progress') + await expect(page.getByRole('heading', { name: 'My Review Progress' })).toBeVisible() + const rejectedCard = page.locator('article').filter({ hasText: firstPublish.slug }) + await expect(rejectedCard).toContainText('Rejected') + await expect(rejectedCard).toContainText('1 submission') + await rejectedCard.getByRole('button', { name: 'Submission history' }).click() + await expect(rejectedCard).toContainText('Rejected by Playwright E2E') + await rejectedCard.getByRole('link', { name: 'Edit and resubmit' }).click() + await expect(page).toHaveURL(/\/dashboard\/publish/) + await expect(page.getByText(new RegExp(`Resubmit .* v${firstPublish.version}`))).toBeVisible() const replacement = await publisherBuilder.publishSkill(namespace.slug, { name: skillName, @@ -74,8 +110,122 @@ test.describe('Rejected version replacement (Real API)', () => { expect(replacement.version).toBe(firstPublish.version) expect(replacementReviewId).not.toBe(rejectedReviewId) + const progressResponse = await page.request.get( + `/api/web/reviews/my-progress?q=${encodeURIComponent(replacement.slug)}&page=0&size=20`, + ) + expect(progressResponse.status()).toBe(200) + const progressBody = await progressResponse.json() as { + data: { + items: Array<{ latestStatus: string; attemptCount: number }> + total: number + statusCounts: { pending: number; approved: number; rejected: number } + } + } + expect(progressBody.data.total).toBe(1) + expect(progressBody.data.items).toHaveLength(1) + expect(progressBody.data.items[0]).toMatchObject({ latestStatus: 'PENDING', attemptCount: 2 }) + expect(progressBody.data.statusCounts).toEqual({ pending: 1, approved: 0, rejected: 0 }) + + const rejectedFilterResponse = await page.request.get( + `/api/web/reviews/my-progress?q=${encodeURIComponent(replacement.slug)}&status=REJECTED&page=0&size=20`, + ) + expect(rejectedFilterResponse.status()).toBe(200) + const rejectedFilterBody = await rejectedFilterResponse.json() as { + data: { items: unknown[]; total: number; statusCounts: { pending: number } } + } + expect(rejectedFilterBody.data.items).toEqual([]) + expect(rejectedFilterBody.data.total).toBe(0) + expect(rejectedFilterBody.data.statusCounts.pending).toBe(1) + + const missingSearchResponse = await page.request.get( + '/api/web/reviews/my-progress?q=definitely-missing-review-progress&page=0&size=20', + ) + expect(missingSearchResponse.status()).toBe(200) + const missingSearchBody = await missingSearchResponse.json() as { + data: { items: unknown[]; total: number; statusCounts: { pending: number; approved: number; rejected: number } } + } + expect(missingSearchBody.data.items).toEqual([]) + expect(missingSearchBody.data.total).toBe(0) + expect(missingSearchBody.data.statusCounts).toEqual({ pending: 0, approved: 0, rejected: 0 }) + + const outOfRangeResponse = await page.request.get( + `/api/web/reviews/my-progress?q=${encodeURIComponent(replacement.slug)}&page=99&size=1`, + ) + expect(outOfRangeResponse.status()).toBe(200) + const outOfRangeBody = await outOfRangeResponse.json() as { + data: { items: unknown[]; total: number; statusCounts: { pending: number } } + } + expect(outOfRangeBody.data.items).toEqual([]) + expect(outOfRangeBody.data.total).toBe(1) + expect(outOfRangeBody.data.statusCounts.pending).toBe(1) + + const maximumPageResponse = await page.request.get( + `/api/web/reviews/my-progress?q=${encodeURIComponent(replacement.slug)}&page=2147483647&size=100`, + ) + expect(maximumPageResponse.status()).toBe(200) + const maximumPageBody = await maximumPageResponse.json() as { + data: { items: unknown[]; total: number; statusCounts: { pending: number } } + } + expect(maximumPageBody.data.items).toEqual([]) + expect(maximumPageBody.data.total).toBe(1) + expect(maximumPageBody.data.statusCounts.pending).toBe(1) + + const attemptsResponse = await page.request.get( + `/api/web/reviews/my-progress/${replacementReviewId}/attempts`, + ) + expect(attemptsResponse.status()).toBe(200) + const attemptsBody = await attemptsResponse.json() as { + data: Array<{ id: number; status: string; skillVersionId: number | null }> + } + expect(attemptsBody.data.map((attempt) => attempt.id)).toEqual([ + replacementReviewId, + rejectedReviewId, + ]) + expect(attemptsBody.data.map((attempt) => attempt.status)).toEqual(['PENDING', 'REJECTED']) + expect(attemptsBody.data[1]?.skillVersionId).toBeNull() + + const reviewerAttemptsResponse = await adminPage.request.get( + `/api/web/reviews/${replacementReviewId}/attempts`, + ) + expect(reviewerAttemptsResponse.status()).toBe(200) + const reviewerAttemptsBody = await reviewerAttemptsResponse.json() as { + data: Array<{ id: number; status: string }> + } + expect(reviewerAttemptsBody.data.map((attempt) => attempt.id)).toEqual([ + replacementReviewId, + rejectedReviewId, + ]) + const replacedReviewResponse = await adminPage.request.get(`/api/web/reviews/${rejectedReviewId}`) - expect(replacedReviewResponse.status()).toBe(404) + expect(replacedReviewResponse.status()).toBe(200) + + await page.goto('/dashboard/review-progress') + await expect(page.getByRole('heading', { name: 'My Review Progress' })).toBeVisible() + await expect(page.getByRole('button', { name: /In review/ })).toContainText('1') + const progressCard = page.locator('article').filter({ hasText: replacement.slug }) + await expect(progressCard).toContainText('In review') + await expect(progressCard).toContainText('2 submissions') + await progressCard.getByRole('button', { name: 'Submission history' }).click() + await expect(progressCard).toContainText('Attempt 2') + await expect(progressCard).toContainText('Attempt 1') + await expect(progressCard).toContainText('Rejected by Playwright E2E') + await expect(progressCard).toContainText('Reviewed by') + await expect(progressCard.getByRole('link', { name: 'Edit and resubmit' })).toHaveCount(0) + await page.screenshot({ path: testInfo.outputPath('author-review-progress-desktop.png'), fullPage: true }) + + await page.setViewportSize({ width: 390, height: 844 }) + await expect(progressCard).toBeVisible() + await expect.poll(() => page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true) + await page.screenshot({ path: testInfo.outputPath('author-review-progress-mobile.png'), fullPage: true }) + + await adminPage.goto(`/dashboard/reviews/${replacementReviewId}`) + await expect(adminPage.getByRole('heading', { name: 'Submission History' })).toBeVisible() + await expect(adminPage.getByText('Attempt 2')).toBeVisible() + await expect(adminPage.getByText('Attempt 1')).toBeVisible() + expect(withoutKnownMetaCspWarning(consoleErrors)).toEqual([]) + expect(pageErrors).toEqual([]) + expect(withoutKnownMetaCspWarning(adminConsoleErrors)).toEqual([]) + expect(adminPageErrors).toEqual([]) } finally { await adminBuilder.cleanup() await adminContext.close() diff --git a/web/e2e/theme-toggle.spec.ts b/web/e2e/theme-toggle.spec.ts new file mode 100644 index 00000000..fa32cb97 --- /dev/null +++ b/web/e2e/theme-toggle.spec.ts @@ -0,0 +1,213 @@ +import { expect, test } from '@playwright/test' +import { setEnglishLocale } from './helpers/auth-fixtures' + +test.describe('Light and dark theme', () => { + test.beforeEach(async ({ page }) => { + await setEnglishLocale(page) + await page.context().setExtraHTTPHeaders({ 'X-Mock-User-Id': 'local-user' }) + await page.route('**/api/v1/auth/me', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + code: 0, + msg: 'success', + data: { + userId: 'theme-layout-user', + displayName: 'Theme Layout User', + avatarUrl: 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///ywAAAAAAQABAAACAUwAOw==', + platformRoles: [], + oauthProvider: 'local', + canChangePassword: true, + }, + timestamp: '2026-09-01T00:00:00Z', + requestId: 'theme-auth-fixture', + }), + }) + }) + await page.route('**/api/web/me/namespaces', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + code: 0, + msg: 'success', + data: [], + timestamp: '2026-09-01T00:00:00Z', + requestId: 'theme-namespace-fixture', + }), + }) + }) + await page.route('**/api/web/notifications/unread-count', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + code: 0, + msg: 'success', + data: { count: 1 }, + timestamp: '2026-09-01T00:00:00Z', + requestId: 'theme-unread-fixture', + }), + }) + }) + await page.route('**/api/web/notifications/sse', async (route) => { + await route.fulfill({ status: 204 }) + }) + await page.route('**/api/web/me/stars?*', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + code: 0, + msg: 'success', + data: { items: [], total: 0, page: 0, size: 100 }, + timestamp: '2026-09-01T00:00:00Z', + requestId: 'theme-stars-fixture', + }), + }) + }) + await page.addInitScript(() => { + const observedWindow = window as Window & { __themeAtFirstReactContent?: boolean } + const observer = new MutationObserver(() => { + const root = document.querySelector('#root') + if (root?.childElementCount) { + observedWindow.__themeAtFirstReactContent = document.documentElement.classList.contains('dark') + observer.disconnect() + } + }) + observer.observe(document, { childList: true, subtree: true }) + if (!window.sessionStorage.getItem('theme-test-initialized')) { + window.localStorage.removeItem('skillhub-theme') + window.sessionStorage.setItem('theme-test-initialized', 'true') + } + }) + }) + + test('switches themes and restores only the browser-local selection', async ({ page }, testInfo) => { + const consoleErrors: string[] = [] + const pageErrors: string[] = [] + page.on('console', (message) => { + if (message.type() === 'error') consoleErrors.push(message.text()) + }) + page.on('pageerror', (error) => pageErrors.push(error.stack ?? error.message)) + + await page.route('**/api/web/notifications?*', async (route) => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + code: 0, + msg: 'success', + data: { + items: [{ + id: 9001, + category: 'REVIEW', + eventType: 'REVIEW_SUBMITTED', + title: 'Theme notification fixture', + bodyJson: JSON.stringify({ skillName: 'Theme preview', version: '1.0.0' }), + targetRoute: '/search', + status: 'UNREAD', + createdAt: '2026-09-01T00:00:00Z', + }], + total: 1, + page: 0, + size: 5, + }, + timestamp: '2026-09-01T00:00:00Z', + requestId: 'theme-notification-fixture', + }), + }) + }) + + await page.goto('/') + await expect(page.locator('html')).not.toHaveClass(/dark/) + const header = page.locator('header') + const lightHeaderBackground = await header.evaluate((element) => getComputedStyle(element).backgroundColor) + + const themeSwitch = page.getByRole('switch', { name: 'Dark theme' }) + await expect(themeSwitch).toHaveAttribute('aria-checked', 'false') + await themeSwitch.click() + await expect(page.locator('html')).toHaveClass(/dark/) + await expect(themeSwitch).toHaveAttribute('aria-checked', 'true') + await expect.poll(() => header.evaluate((element) => getComputedStyle(element).backgroundColor)) + .not.toBe(lightHeaderBackground) + await expect.poll(() => page.evaluate(() => window.localStorage.getItem('skillhub-theme'))).toBe('dark') + const destructiveContrast = await page.evaluate(() => { + const probe = document.createElement('button') + probe.className = 'bg-destructive text-destructive-foreground' + probe.textContent = 'Destructive contrast probe' + document.body.append(probe) + const styles = getComputedStyle(probe) + + const luminance = (color: string) => { + const channels = color.match(/[\d.]+/g)?.slice(0, 3).map(Number) + if (!channels || channels.length !== 3) { + throw new Error(`Unable to parse computed color: ${color}`) + } + const linear = channels.map((channel) => { + const normalized = channel / 255 + return normalized <= 0.04045 + ? normalized / 12.92 + : ((normalized + 0.055) / 1.055) ** 2.4 + }) + return 0.2126 * linear[0] + 0.7152 * linear[1] + 0.0722 * linear[2] + } + + const background = luminance(styles.backgroundColor) + const foreground = luminance(styles.color) + probe.remove() + return (Math.max(background, foreground) + 0.05) / (Math.min(background, foreground) + 0.05) + }) + expect(destructiveContrast).toBeGreaterThanOrEqual(4.5) + + await page.reload() + await expect(page.locator('html')).toHaveClass(/dark/) + await expect(page.getByRole('switch', { name: 'Dark theme' })).toHaveAttribute('aria-checked', 'true') + await expect(page.getByRole('heading', { name: 'SkillHub', exact: true })).toBeVisible() + await expect.poll(() => page.evaluate(() => ( + window as Window & { __themeAtFirstReactContent?: boolean } + ).__themeAtFirstReactContent)).toBe(true) + + await page.getByRole('link', { name: 'Search', exact: true }).first().click() + await expect(page).toHaveURL(/\/search(?:\?|$)/) + await expect(page.locator('html')).toHaveClass(/dark/) + await expect(page.getByPlaceholder('Search skills...')).toBeVisible() + await expect.poll(() => page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true) + await page.screenshot({ path: testInfo.outputPath('dark-desktop.png'), fullPage: true }) + + const notificationButton = page.getByRole('button', { name: 'Notifications' }) + await notificationButton.click() + await expect(page.getByText('Notifications', { exact: true })).toBeVisible() + const firstNotification = page.getByRole('link').filter({ hasText: 'Review submitted' }) + await expect(firstNotification).toBeVisible() + const backgroundBeforeHover = await firstNotification.evaluate((element) => getComputedStyle(element).backgroundColor) + await firstNotification.hover() + await expect.poll(() => firstNotification.evaluate((element) => getComputedStyle(element).backgroundColor)) + .not.toBe(backgroundBeforeHover) + await page.screenshot({ path: testInfo.outputPath('dark-notifications.png'), fullPage: true }) + await notificationButton.click() + + await page.setViewportSize({ width: 390, height: 844 }) + await expect(page.getByRole('switch', { name: 'Dark theme' })).toBeVisible() + await expect.poll(() => page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true) + await page.screenshot({ path: testInfo.outputPath('dark-mobile.png'), fullPage: true }) + + await page.setViewportSize({ width: 320, height: 568 }) + const headerControls = page.locator('header > div') + const [headerBox, controlsBox] = await Promise.all([header.boundingBox(), headerControls.boundingBox()]) + expect(headerBox).not.toBeNull() + expect(controlsBox).not.toBeNull() + expect((controlsBox?.x ?? 0) + (controlsBox?.width ?? 0)).toBeLessThanOrEqual( + (headerBox?.x ?? 0) + (headerBox?.width ?? 0), + ) + await expect.poll(() => page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true) + await page.screenshot({ path: testInfo.outputPath('dark-mobile-320.png'), fullPage: true }) + + const unexpectedConsoleErrors = consoleErrors.filter((message) => ( + !message.includes("frame-ancestors' is ignored when delivered via a element") + )) + expect(unexpectedConsoleErrors).toEqual([]) + expect(pageErrors).toEqual([]) + }) +}) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 5e4c7756..c503257f 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -15,6 +15,7 @@ import type { MergeInitiateResponse, MergeVerifyRequest, ReviewSkillDetail, + ReviewProgressPage, ReviewTask, PromotionSortBy, PromotionSortDirection, @@ -882,6 +883,25 @@ export const reviewApi = { return fetchJson(`${WEB_API_PREFIX}/reviews/${id}`) }, + async listMyProgress(params: { status?: string; q?: string; page?: number; size?: number }) { + const searchParams = new URLSearchParams() + if (params.status) searchParams.set('status', params.status) + if (params.q) searchParams.set('q', params.q) + searchParams.set('page', String(params.page ?? 0)) + searchParams.set('size', String(params.size ?? 20)) + return fetchJson( + `${WEB_API_PREFIX}/reviews/my-progress?${searchParams.toString()}`, + ) + }, + + async listMyAttempts(reviewTaskId: number): Promise { + return fetchJson(`${WEB_API_PREFIX}/reviews/my-progress/${reviewTaskId}/attempts`) + }, + + async listAttempts(reviewTaskId: number): Promise { + return fetchJson(`${WEB_API_PREFIX}/reviews/${reviewTaskId}/attempts`) + }, + async getSkillDetail(id: number): Promise { return fetchJson(`${WEB_API_PREFIX}/reviews/${id}/skill-detail`) }, diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index 69d55df9..03786be7 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -2404,6 +2404,38 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/reviews/{id}/attempts": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listReviewAttempts"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/reviews/{id}/attempts": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listReviewAttempts_1"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/reviews/{id}": { parameters: { query?: never; @@ -2500,6 +2532,70 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/reviews/my-progress/{id}/attempts": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listMyAttempts"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/reviews/my-progress/{id}/attempts": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listMyAttempts_1"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/reviews/my-progress": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listMyProgress"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/reviews/my-progress": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations["listMyProgress_1"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/web/promotions/{id}": { parameters: { query?: never; @@ -4629,6 +4725,15 @@ export interface components { downloadUrl?: string; activeVersion?: string; }; + ApiResponseListReviewTaskResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["ReviewTaskResponse"][]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; ApiResponsePageResponseReviewTaskResponse: { /** Format: int32 */ code?: number; @@ -4647,6 +4752,50 @@ export interface components { /** Format: int32 */ size?: number; }; + ApiResponseReviewProgressPageResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["ReviewProgressPageResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + ReviewProgressPageResponse: { + items?: components["schemas"]["ReviewProgressResponse"][]; + /** Format: int64 */ + total?: number; + /** Format: int32 */ + page?: number; + /** Format: int32 */ + size?: number; + statusCounts?: components["schemas"]["ReviewProgressStatusCounts"]; + }; + ReviewProgressResponse: { + /** Format: int64 */ + latestReviewTaskId?: number; + /** Format: int64 */ + skillId?: number; + namespace?: string; + skillSlug?: string; + skillVersion?: string; + latestStatus?: string; + latestReviewComment?: string; + /** Format: date-time */ + latestSubmittedAt?: string; + /** Format: date-time */ + latestReviewedAt?: string; + /** Format: int64 */ + attemptCount?: number; + }; + ReviewProgressStatusCounts: { + /** Format: int64 */ + pending?: number; + /** Format: int64 */ + approved?: number; + /** Format: int64 */ + rejected?: number; + }; ApiResponsePageResponsePromotionResponseDto: { /** Format: int32 */ code?: number; @@ -10029,6 +10178,50 @@ export interface operations { }; }; }; + listReviewAttempts: { + parameters: { + query?: never; + header?: never; + path: { + id: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseListReviewTaskResponse"]; + }; + }; + }; + }; + listReviewAttempts_1: { + parameters: { + query?: never; + header?: never; + path: { + id: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseListReviewTaskResponse"]; + }; + }; + }; + }; getReviewDetail: { parameters: { query?: never; @@ -10167,6 +10360,100 @@ export interface operations { }; }; }; + listMyAttempts: { + parameters: { + query?: never; + header?: never; + path: { + id: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseListReviewTaskResponse"]; + }; + }; + }; + }; + listMyAttempts_1: { + parameters: { + query?: never; + header?: never; + path: { + id: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseListReviewTaskResponse"]; + }; + }; + }; + }; + listMyProgress: { + parameters: { + query?: { + status?: string; + q?: string; + page?: number; + size?: number; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseReviewProgressPageResponse"]; + }; + }; + }; + }; + listMyProgress_1: { + parameters: { + query?: { + status?: string; + q?: string; + page?: number; + size?: number; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseReviewProgressPageResponse"]; + }; + }; + }; + }; getPromotionDetail: { parameters: { query?: never; diff --git a/web/src/api/types.ts b/web/src/api/types.ts index e5d62e12..4db8e8db 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -427,7 +427,7 @@ export interface SkillDeleteResult { export interface ReviewTask { id: number - skillVersionId: number + skillVersionId: number | null namespace: string skillSlug: string version: string @@ -441,6 +441,33 @@ export interface ReviewTask { reviewedAt?: string } +export interface ReviewProgress { + latestReviewTaskId: number + skillId: number + namespace: string + skillSlug: string + skillVersion: string + latestStatus: 'PENDING' | 'APPROVED' | 'REJECTED' + latestReviewComment?: string + latestSubmittedAt: string + latestReviewedAt?: string + attemptCount: number +} + +export interface ReviewProgressStatusCounts { + pending: number + approved: number + rejected: number +} + +export interface ReviewProgressPage { + items: ReviewProgress[] + total: number + page: number + size: number + statusCounts: ReviewProgressStatusCounts +} + export interface ReviewSkillDetail { skill: SkillDetail versions: SkillVersion[] diff --git a/web/src/app/layout-header-style.test.ts b/web/src/app/layout-header-style.test.ts index 25337244..680aa658 100644 --- a/web/src/app/layout-header-style.test.ts +++ b/web/src/app/layout-header-style.test.ts @@ -3,7 +3,11 @@ import { APP_HEADER_ELEVATED_CLASS_NAME, getAppHeaderClassName } from './layout- describe('getAppHeaderClassName', () => { it('keeps the header flat before the page starts scrolling', () => { - expect(getAppHeaderClassName(false)).not.toContain(APP_HEADER_ELEVATED_CLASS_NAME) + const className = getAppHeaderClassName(false) + + expect(className).not.toContain(APP_HEADER_ELEVATED_CLASS_NAME) + expect(className).toContain('bg-background/90') + expect(className).not.toContain('bg-white') }) it('adds a subtle drop shadow after the header becomes sticky', () => { diff --git a/web/src/app/layout-header-style.ts b/web/src/app/layout-header-style.ts index 3ae7e8fc..49288285 100644 --- a/web/src/app/layout-header-style.ts +++ b/web/src/app/layout-header-style.ts @@ -1,9 +1,10 @@ import { cn } from '@/shared/lib/utils' export const APP_HEADER_BASE_CLASS_NAME = - 'sticky top-0 z-50 flex items-center justify-between border-b bg-white px-6 py-4 transition-shadow duration-200 md:px-12' + 'sticky top-0 z-50 flex items-center justify-between border-b border-border/70 bg-background/90 px-4 py-4 backdrop-blur-xl transition-[background-color,border-color,box-shadow] duration-200 supports-[backdrop-filter]:bg-background/80 sm:px-6 md:px-12' -export const APP_HEADER_ELEVATED_CLASS_NAME = 'shadow-[0_10px_24px_-20px_rgba(15,23,42,0.32)]' +export const APP_HEADER_ELEVATED_CLASS_NAME = + 'shadow-[0_12px_30px_-24px_hsl(var(--foreground)/0.45)]' export function getAppHeaderClassName(isElevated: boolean): string { return cn(APP_HEADER_BASE_CLASS_NAME, isElevated && APP_HEADER_ELEVATED_CLASS_NAME) diff --git a/web/src/app/layout.tsx b/web/src/app/layout.tsx index 7a52720b..7d5000a8 100644 --- a/web/src/app/layout.tsx +++ b/web/src/app/layout.tsx @@ -3,6 +3,7 @@ import { Outlet, Link, useRouterState } from '@tanstack/react-router' import { useTranslation } from 'react-i18next' import { useAuth } from '@/features/auth/use-auth' import { LanguageSwitcher } from '@/shared/components/language-switcher' +import { ThemeToggle } from '@/shared/components/theme-toggle' import { UserMenu } from '@/shared/components/user-menu' import { NotificationBell } from '@/features/notification/notification-bell' import { dismissOpenOverlays } from '@/shared/lib/dismiss-open-overlays' @@ -82,7 +83,7 @@ export function Layout() {
@@ -115,7 +116,8 @@ export function Layout() { })} -
+
+ {user && } {isLoading ? null : user ? ( @@ -149,7 +151,7 @@ export function Layout() { {/* Footer */} -