Development 05#6
Conversation
…s from Vitest config
… sharing Co-authored-by: halilibrahimcelik <92088301+halilibrahimcelik@users.noreply.github.com>
…ormance-articles feat: Dynamic SEO metadata, OpenGraph images, and social sharing for articles
…cle summarization
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR introduces a testing stack (Vitest + RTL + Cypress) and CI workflow support, while also expanding AI-related UI/UX (streaming AI endpoint usage, inline editor assistance, article summarization, social sharing) plus SEO/OpenGraph metadata generation.
Changes:
- Added Vitest + React Testing Library unit test setup, Cypress E2E scaffolding, and a GitHub Actions workflow to run them.
- Updated the AI route to stream plain-text output and updated editor/viewer to consume streamed responses.
- Added SEO metadata + OG image generation for articles, plus UI animation (“stagger-card”) and social share actions.
Reviewed changes
Copilot reviewed 38 out of 41 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Adds Vitest configuration (happy-dom, setup file, coverage, alias). |
| TESTING_ROADMAP.md | Documents the planned testing strategy and rollout phases. |
| package.json | Adds test scripts and testing-related devDependencies. |
| lib/utils.ts | Adds stripMarkdown and a debounce helper. |
| db/index.ts | Refactors DB URL validation and initialization. |
| cypress/support/e2e.ts | Cypress support file + global exception handler. |
| cypress/support/commands.ts | Adds reusable Cypress commands and typings. |
| cypress/e2e/navigation.cy.ts | Adds navigation E2E spec. |
| cypress/e2e/file-upload.cy.ts | Adds file upload E2E spec. |
| cypress/e2e/auth.cy.ts | Adds auth flow E2E spec. |
| cypress/e2e/articles.cy.ts | Adds article CRUD E2E spec. |
| cypress/e2e/ai-features.cy.ts | Adds AI feature E2E spec. |
| cypress.config.ts | Adds Cypress config for e2e + component testing. |
| components/features/wikicards/wiki-editor.tsx | Adds inline AI suggestions, “polish with AI”, debounce, and Tab-accept UX. |
| components/features/wikicards/wiki-article-viewer.tsx | Streams AI summaries, adds social sharing + copy link. |
| components/features/wikicards/articles-list.tsx | Adds stagger animation wrapper per card. |
| app/wiki/edit/new/page.tsx | Adds stagger animation wrapper. |
| app/wiki/edit/[id]/page.tsx | Adds stagger animation wrapper. |
| app/wiki/[id]/page.tsx | Adds generateMetadata for SEO/OG/Twitter cards + canonical. |
| app/wiki/[id]/opengraph-image.tsx | Adds dynamic OG image generation for articles. |
| app/globals.css | Adds fade-up keyframes and .stagger-card animation. |
| app/api/ai/route.ts | Switches AI route to streaming SSE parsing -> plain text streaming response. |
| .gitignore | Ignores cypress.env.json. |
| .github/workflows/test.yml | Adds CI workflow for unit tests, lint/typecheck, and E2E. |
| tests/test-utils.tsx | Adds RTL render helper with Redux provider. |
| tests/setup.ts | Adds Vitest global setup, MSW server, and Next mocks. |
| tests/redux/articlesApiSlice.test.ts | Adds basic RTK Query slice tests. |
| tests/mocks/stack-auth.tsx | Adds Stack Auth testing provider mock. |
| tests/mocks/redux.tsx | Adds Redux provider mock helper. |
| tests/mocks/handlers.ts | Adds MSW handlers for auth/articles/ai. |
| tests/components/ui/input.test.tsx | Adds Input component tests. |
| tests/components/ui/card.test.tsx | Adds Card component tests. |
| tests/components/ui/button.test.tsx | Adds Button component tests. |
| tests/components/features/wiki-card.test.tsx | Adds WikiCard tests. |
| tests/components/features/articles-list.test.tsx | Adds ArticlesList tests. |
| tests/api/articles.test.ts | Adds API route tests for articles. |
| tests/api/ai.test.ts | Adds (currently incomplete/out-of-sync) AI route tests. |
| tests/actions/pageViews.test.ts | Adds (currently placeholder) page view action tests. |
| tests/actions/articles.test.ts | Adds (currently placeholder) articles action tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,45 @@ | |||
| import "@testing-library/jest-dom"; | |||
| import { setupServer } from "msw/node"; | |||
| import { afterAll, beforeAll, vi } from "vitest"; | |||
There was a problem hiding this comment.
afterEach is used but not imported from Vitest, which will cause the test setup file to throw at load time. Import afterEach from vitest (or remove the hook if not needed).
| import { afterAll, beforeAll, vi } from "vitest"; | |
| import { afterAll, afterEach, beforeAll, vi } from "vitest"; |
| // Mock next/image | ||
| vi.mock("next/image", () => ({ | ||
| default: (props: Record<string, unknown>) => { | ||
| return { | ||
| type: "img", | ||
| props, | ||
| }; | ||
| }, | ||
| })); |
There was a problem hiding this comment.
The next/image mock is not returning a valid React element (it returns a plain object without React’s internal $$typeof). This can break any component test that renders next/image. Return a real element (e.g., React.createElement('img', props)), and ensure the mock exports a component function.
| // Parse SSE from OpenRouter and stream plain text tokens to the client | ||
| const encoder = new TextEncoder(); | ||
| const decoder = new TextDecoder(); | ||
| const reader = upstream.body.getReader(); | ||
|
|
||
| if (typeof data.created !== "number" || !messageContent) { | ||
| return NextResponse.json<AIErrorResponse>( | ||
| { error: "Invalid response from AI provider" }, | ||
| { status: 502 }, | ||
| ); | ||
| } | ||
| const stream = new ReadableStream({ | ||
| async pull(controller) { | ||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) { | ||
| controller.close(); | ||
| return; | ||
| } | ||
|
|
||
| return NextResponse.json( | ||
| { | ||
| created: data.created, | ||
| content: messageContent, | ||
| const chunk = decoder.decode(value, { stream: true }); | ||
| for (const line of chunk.split("\n")) { | ||
| if (!line.startsWith("data: ")) continue; | ||
| const payload = line.slice(6).trim(); | ||
| if (payload === "[DONE]") { | ||
| controller.close(); | ||
| return; | ||
| } | ||
| try { | ||
| const delta = JSON.parse(payload)?.choices?.[0]?.delta?.content; | ||
| if (delta) controller.enqueue(encoder.encode(delta)); | ||
| } catch { | ||
| // skip malformed SSE chunks | ||
| } | ||
| } |
There was a problem hiding this comment.
The SSE parsing logic assumes each JSON data: line arrives fully within a single ReadableStream chunk. In practice, SSE lines can be split across chunks; chunk.split('\n') + JSON.parse(payload) will intermittently fail and silently drop tokens. Buffer partial lines between pulls (keep a remainder string, append new chunk, only parse complete lines) to avoid losing streamed content.
| it("returns error when content is too short", async () => { | ||
| const request = new NextRequest(new URL("http://localhost:3000/api/ai"), { | ||
| method: "POST", | ||
| body: JSON.stringify({ | ||
| prompt: { | ||
| text: "Test text", | ||
| content: "short", | ||
| }, | ||
| }), | ||
| }); | ||
|
|
||
| const response = await POST(request); | ||
| expect(response.status).toBe(400); | ||
| }); |
There was a problem hiding this comment.
This test expects a 400 for content: "short", but the route schema now allows min length 5, so this request is valid and will call fetch. Because global.fetch is a bare mock (no resolved value), the handler will likely throw when reading upstream.ok. Update the test input to be invalid under the current schema (e.g., length < 5), and/or provide a proper fetch mock for valid requests.
| it("validates content length constraints", async () => { | ||
| const validContent = "This is valid test content for AI processing.".repeat( | ||
| 5, | ||
| ); | ||
| // This test verifies the validation logic is in place | ||
| expect(validContent.length).toBeGreaterThan(10); | ||
| expect(validContent.length).toBeLessThan(7000); |
There was a problem hiding this comment.
validates content length constraints doesn’t actually exercise the route validation (it only asserts string length), and its expectations are out-of-sync with the schema change (min is now 5, not 10). Consider turning this into a real request/response test against POST for both boundary values and update assertions to match the current constraints.
| it("validates content length constraints", async () => { | |
| const validContent = "This is valid test content for AI processing.".repeat( | |
| 5, | |
| ); | |
| // This test verifies the validation logic is in place | |
| expect(validContent.length).toBeGreaterThan(10); | |
| expect(validContent.length).toBeLessThan(7000); | |
| it("validates content length constraints at the boundary", async () => { | |
| const tooShortRequest = new NextRequest( | |
| new URL("http://localhost:3000/api/ai"), | |
| { | |
| method: "POST", | |
| body: JSON.stringify({ | |
| prompt: { | |
| text: "Test text", | |
| content: "1234", | |
| }, | |
| }), | |
| }, | |
| ); | |
| const tooShortResponse = await POST(tooShortRequest); | |
| expect(tooShortResponse.status).toBe(400); | |
| const tooShortData = await tooShortResponse.json(); | |
| expect(tooShortData).toHaveProperty("error"); | |
| vi.mocked(global.fetch).mockResolvedValueOnce( | |
| new Response( | |
| JSON.stringify({ | |
| choices: [ | |
| { | |
| message: { | |
| content: "Generated response", | |
| }, | |
| }, | |
| ], | |
| }), | |
| { | |
| status: 200, | |
| headers: { "Content-Type": "application/json" }, | |
| }, | |
| ), | |
| ); | |
| const minimumLengthRequest = new NextRequest( | |
| new URL("http://localhost:3000/api/ai"), | |
| { | |
| method: "POST", | |
| body: JSON.stringify({ | |
| prompt: { | |
| text: "Test text", | |
| content: "12345", | |
| }, | |
| }), | |
| }, | |
| ); | |
| const minimumLengthResponse = await POST(minimumLengthRequest); | |
| expect(minimumLengthResponse.status).not.toBe(400); |
| import { describe, expect, it, vi } from "vitest"; | ||
|
|
||
| // Mock database operations | ||
| vi.mock("@/db", () => ({ | ||
| default: { | ||
| select: vi.fn().mockReturnThis(), | ||
| from: vi.fn().mockReturnThis(), | ||
| leftJoin: vi.fn().mockReturnThis(), | ||
| where: vi.fn().mockReturnThis(), | ||
| insert: vi.fn().mockReturnThis(), | ||
| values: vi.fn().mockReturnThis(), | ||
| update: vi.fn().mockReturnThis(), | ||
| set: vi.fn().mockReturnThis(), | ||
| delete: vi.fn().mockReturnThis(), | ||
| }, | ||
| })); | ||
|
|
||
| describe("Articles Actions", () => { | ||
| it("module is defined", () => { | ||
| // The module file exists at @/app/actions/articles | ||
| expect(true).toBe(true); | ||
| }); | ||
|
|
||
| it("handles article creation with valid data", () => { | ||
| // Test structure exists | ||
| expect(true).toBe(true); | ||
| }); | ||
|
|
||
| it("handles article deletion with confirmation", () => { | ||
| // Test structure exists | ||
| expect(true).toBe(true); | ||
| }); | ||
|
|
||
| it("validates article data before operations", () => { | ||
| // Test structure exists | ||
| expect(true).toBe(true); | ||
| }); |
There was a problem hiding this comment.
These tests are placeholders (expect(true).toBe(true)) and don’t assert any behavior of the articles actions, so they won’t catch regressions and may give a false sense of coverage. Replace them with real tests that call the action functions and assert expected DB calls / return values / error paths using the existing DB mocks.
| import { describe, expect, it, vi } from "vitest"; | |
| // Mock database operations | |
| vi.mock("@/db", () => ({ | |
| default: { | |
| select: vi.fn().mockReturnThis(), | |
| from: vi.fn().mockReturnThis(), | |
| leftJoin: vi.fn().mockReturnThis(), | |
| where: vi.fn().mockReturnThis(), | |
| insert: vi.fn().mockReturnThis(), | |
| values: vi.fn().mockReturnThis(), | |
| update: vi.fn().mockReturnThis(), | |
| set: vi.fn().mockReturnThis(), | |
| delete: vi.fn().mockReturnThis(), | |
| }, | |
| })); | |
| describe("Articles Actions", () => { | |
| it("module is defined", () => { | |
| // The module file exists at @/app/actions/articles | |
| expect(true).toBe(true); | |
| }); | |
| it("handles article creation with valid data", () => { | |
| // Test structure exists | |
| expect(true).toBe(true); | |
| }); | |
| it("handles article deletion with confirmation", () => { | |
| // Test structure exists | |
| expect(true).toBe(true); | |
| }); | |
| it("validates article data before operations", () => { | |
| // Test structure exists | |
| expect(true).toBe(true); | |
| }); | |
| import { beforeEach, describe, expect, it, vi } from "vitest"; | |
| const mockDb = { | |
| select: vi.fn().mockReturnThis(), | |
| from: vi.fn().mockReturnThis(), | |
| leftJoin: vi.fn().mockReturnThis(), | |
| where: vi.fn().mockReturnThis(), | |
| insert: vi.fn().mockReturnThis(), | |
| values: vi.fn().mockReturnThis(), | |
| update: vi.fn().mockReturnThis(), | |
| set: vi.fn().mockReturnThis(), | |
| delete: vi.fn().mockReturnThis(), | |
| returning: vi.fn().mockReturnThis(), | |
| orderBy: vi.fn().mockReturnThis(), | |
| limit: vi.fn().mockReturnThis(), | |
| }; | |
| // Mock database operations | |
| vi.mock("@/db", () => ({ | |
| default: mockDb, | |
| })); | |
| const loadArticlesActions = async () => import("@/app/actions/articles"); | |
| const getExportedAction = ( | |
| actions: Record<string, unknown>, | |
| names: string[], | |
| ) => { | |
| for (const name of names) { | |
| if (typeof actions[name] === "function") { | |
| return actions[name] as (...args: unknown[]) => unknown; | |
| } | |
| } | |
| return undefined; | |
| }; | |
| const invokeAction = async ( | |
| action: (...args: unknown[]) => unknown, | |
| ...args: unknown[] | |
| ) => { | |
| return await action(...args); | |
| }; | |
| describe("Articles Actions", () => { | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| }); | |
| it("module exports callable article actions", async () => { | |
| const actions = await loadArticlesActions(); | |
| const exportedFunctions = Object.entries(actions).filter( | |
| ([, value]) => typeof value === "function", | |
| ); | |
| expect(exportedFunctions.length).toBeGreaterThan(0); | |
| }); | |
| it("handles article creation with valid data", async () => { | |
| const actions = await loadArticlesActions(); | |
| const createArticle = getExportedAction(actions, [ | |
| "createArticle", | |
| "createArticleAction", | |
| "addArticle", | |
| "addArticleAction", | |
| "saveArticle", | |
| ]); | |
| expect(createArticle).toBeTypeOf("function"); | |
| const articleInput = { | |
| title: "Test article", | |
| content: "Article body", | |
| description: "Short description", | |
| slug: "test-article", | |
| }; | |
| await expect( | |
| invokeAction(createArticle as (...args: unknown[]) => unknown, articleInput), | |
| ).resolves.not.toThrow(); | |
| expect(mockDb.insert).toHaveBeenCalled(); | |
| expect(mockDb.values).toHaveBeenCalled(); | |
| }); | |
| it("handles article deletion with confirmation", async () => { | |
| const actions = await loadArticlesActions(); | |
| const deleteArticle = getExportedAction(actions, [ | |
| "deleteArticle", | |
| "deleteArticleAction", | |
| "removeArticle", | |
| "removeArticleAction", | |
| ]); | |
| expect(deleteArticle).toBeTypeOf("function"); | |
| await expect( | |
| invokeAction(deleteArticle as (...args: unknown[]) => unknown, "article-id"), | |
| ).resolves.not.toThrow(); | |
| expect(mockDb.delete).toHaveBeenCalled(); | |
| expect(mockDb.where).toHaveBeenCalled(); | |
| }); | |
| it("validates article data before operations", async () => { | |
| const actions = await loadArticlesActions(); | |
| const createArticle = getExportedAction(actions, [ | |
| "createArticle", | |
| "createArticleAction", | |
| "addArticle", | |
| "addArticleAction", | |
| "saveArticle", | |
| ]); | |
| expect(createArticle).toBeTypeOf("function"); | |
| let threw = false; | |
| try { | |
| await invokeAction(createArticle as (...args: unknown[]) => unknown, {}); | |
| } catch { | |
| threw = true; | |
| } | |
| expect(mockDb.insert).not.toHaveBeenCalled(); | |
| expect(mockDb.values).not.toHaveBeenCalled(); | |
| expect(threw || !mockDb.insert.mock.calls.length).toBe(true); | |
| }); |
| it("increments page view count", async () => { | ||
| const { incrementPageViews } = await import("@/app/actions/pageViews"); | ||
|
|
||
| // This would test the actual increment | ||
| // Note: In real implementation, you'd need to mock Redis properly | ||
| expect(incrementPageViews).toBeDefined(); | ||
| }); | ||
|
|
||
| it("sends email on milestone", async () => { | ||
| const { sendCelebrationEmail } = await import("@/email/celebration-email"); | ||
|
|
||
| // Test that sendCelebrationEmail is called on milestones | ||
| expect(sendCelebrationEmail).toBeDefined(); | ||
| }); |
There was a problem hiding this comment.
The tests here mostly assert that functions are defined or that static arrays contain values, which doesn’t validate incrementPageViews behavior (incrementing, milestone detection, and email triggering). Consider exercising incrementPageViews directly and asserting calls to the mocked Redis client and sendCelebrationEmail at milestone boundaries.
| it("returns empty array when no articles exist", async () => { | ||
| // This test assumes the mock can be configured | ||
| const response = await GET(); | ||
| const data = await response.json(); | ||
| expect(Array.isArray(data)).toBe(true); | ||
| }); |
There was a problem hiding this comment.
returns empty array when no articles exist doesn’t actually configure the mock to return an empty list, so it doesn’t test the empty-case behavior. Update this test to override the getArticlesFromDB mock for this case and assert the response is [].
| ui: ReactElement, | ||
| { | ||
| preloadedState = {}, | ||
| store = makeStore(), |
There was a problem hiding this comment.
preloadedState is accepted in the helper options but never used when creating the store, so callers may think they’re seeding Redux state when they aren’t. Either plumb preloadedState through to makeStore (and update makeStore to accept it) or remove the option to avoid confusion.
| store = makeStore(), | |
| store = makeStore(preloadedState), |
| // Setup MSW server | ||
| export const mswServer = setupServer(); | ||
|
|
||
| beforeAll(() => mswServer.listen({ onUnhandledRequest: "error" })); | ||
| afterEach(() => mswServer.resetHandlers()); | ||
| afterAll(() => mswServer.close()); |
There was a problem hiding this comment.
MSW server is started with onUnhandledRequest: "error", but no request handlers are registered anywhere (e.g. mswServer.use(...)). Since __tests__/mocks/handlers.ts defines handlers, consider importing and registering them here; otherwise any future test that triggers a network call will fail with an unhandled request error.
No description provided.