test(review): strengthen exact-sha coverage

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
2026-09-06 08:15:57 +00:00 · 2026-09-01 14:34:03 +08:00 · 2026-09-01 14:34:03 +08:00 · 39861bc6d8
commit 39861bc6d8
parent 918ef9d265
5 changed files with 173 additions and 10 deletions
--- 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
@ -326,10 +326,15 @@ class ReviewPortalControllerTest {
        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)));

@ -337,7 +342,12 @@ class ReviewPortalControllerTest {
                .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[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
@ -345,7 +355,7 @@ class ReviewPortalControllerTest {
        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-1", ReviewTaskStatus.REJECTED);
+        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");
@ -368,7 +378,41 @@ class ReviewPortalControllerTest {
                .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[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
--- 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
@ -34,6 +34,8 @@ class JpaReviewProgressQueryRepositoryTest {
                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,
@ -56,6 +58,13 @@ class JpaReviewProgressQueryRepositoryTest {
                "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,
@ -68,7 +77,7 @@ class JpaReviewProgressQueryRepositoryTest {

        var firstPage = repository.findMyProgress("author-1", null, "", 0, 1);

-        assertThat(firstPage.total()).isEqualTo(2);
+        assertThat(firstPage.total()).isEqualTo(3);
        assertThat(firstPage.items()).singleElement().satisfies(item -> {
            assertThat(item.skillSlug()).isEqualTo("alpha-skill");
            assertThat(item.latestStatus()).isEqualTo("PENDING");
@ -76,11 +85,11 @@ class JpaReviewProgressQueryRepositoryTest {
        });
        assertThat(firstPage.statusCounts().pending()).isEqualTo(1);
        assertThat(firstPage.statusCounts().approved()).isEqualTo(1);
-        assertThat(firstPage.statusCounts().rejected()).isZero();
+        assertThat(firstPage.statusCounts().rejected()).isEqualTo(1);

        var emptyPage = repository.findMyProgress("author-1", null, "", 8, 1);
        assertThat(emptyPage.items()).isEmpty();
-        assertThat(emptyPage.total()).isEqualTo(2);
+        assertThat(emptyPage.total()).isEqualTo(3);

        var searchedAndFiltered = repository.findMyProgress(
                "author-1", ReviewTaskStatus.APPROVED, "BETA", 0, 20);
@ -88,6 +97,13 @@ class JpaReviewProgressQueryRepositoryTest {
                .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(
--- a/web/e2e/rejected-version-republish.spec.ts
+++ b/web/e2e/rejected-version-republish.spec.ts
@ -93,6 +93,55 @@ 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 attemptsResponse = await page.request.get(
        `/api/web/reviews/my-progress/${replacementReviewId}/attempts`,
      )
--- a/web/e2e/theme-toggle.spec.ts
+++ b/web/e2e/theme-toggle.spec.ts
@ -6,6 +6,15 @@ test.describe('Light and dark theme', () => {
    await setEnglishLocale(page)
    await page.context().setExtraHTTPHeaders({ 'X-Mock-User-Id': 'local-user' })
    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')
@ -21,6 +30,33 @@ test.describe('Light and dark theme', () => {
    })
    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' }),
+              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/)

@ -32,15 +68,24 @@ test.describe('Light and dark theme', () => {
    await expect(page.locator('html')).toHaveClass(/dark/)
    await expect(page.getByRole('button', { name: 'Switch to light theme' })).toBeVisible()
    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 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 = notificationButton.locator('..').locator('a').first()
-    if (await firstNotification.count()) {
-      await firstNotification.hover()
-    }
+    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()

--- a/web/src/shared/lib/theme.test.ts
+++ b/web/src/shared/lib/theme.test.ts
@ -50,4 +50,13 @@ describe('theme preference', () => {
    expect(document.documentElement.classList.contains('dark')).toBe(true)
    expect(window.localStorage.getItem(THEME_STORAGE_KEY)).toBe('dark')
  })
+
+  it('ignores storage write failures', () => {
+    expect(() => saveTheme('dark', {
+      getItem: () => null,
+      setItem: () => {
+        throw new Error('blocked')
+      },
+    })).not.toThrow()
+  })
 })