Integrate Notion API syncing with analyze workflow#26
Integrate Notion API syncing with analyze workflow#26stevenschling13 wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements the Notion API integration for the grow operations photo analysis system. The changes activate previously placeholder functionality to write photo analysis results to Notion databases.
- Adds complete Notion API client with proper property mapping and HMAC-verified requests
- Implements upsert logic for history tracking with idempotency keys
- Adds comprehensive test coverage for the new integration
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/integrations/notion.ts | Adds NotionService class with updatePhoto and upsertHistory methods, including property mapping functions and URL ID extraction |
| src/routes/analyze.ts | Activates Notion API calls by replacing placeholder code with actual updatePhoto and upsertHistory invocations |
| src/domain/mapping.ts | Adds missing "Sev" (severity) property mapping to photo writebacks |
| test/notion.test.ts | Adds unit tests for Notion URL ID extraction helper |
| test/mapping.test.ts | Adds unit tests for writeback-to-photo and history property mapping functions |
| test/hmac.test.ts | Extends HMAC test to mock Notion API calls and verify end-to-end integration |
| expect(body).toHaveProperty("results"); | ||
| expect(body.results).toHaveLength(1); | ||
|
|
||
| fetchMock.mockRestore?.(); |
There was a problem hiding this comment.
The optional chaining on mockRestore will silently fail since vi.fn() doesn't provide a mockRestore method. Use fetchMock.mockClear() or wrap the mock/restore logic in afterEach/beforeEach hooks to properly clean up the test state.
| fetchMock.mockRestore?.(); |
| } | ||
| } | ||
|
|
||
| async function notionFetch(path: string, init: RequestInit & { token: string }): Promise<any> { |
There was a problem hiding this comment.
The return type is Promise<any> which bypasses TypeScript's type safety. Consider defining specific return types for each Notion API response or use a generic NotionApiResponse type instead of any.
| async function notionFetch(path: string, init: RequestInit & { token: string }): Promise<any> { | |
| async function notionFetch<T = unknown>(path: string, init: RequestInit & { token: string }): Promise<T | null> { |
| text: async () => JSON.stringify(data), | ||
| }); | ||
|
|
||
| const fetchMock = vi.fn(async (url: string, init: RequestInit) => { |
There was a problem hiding this comment.
The fetch mock setup (lines 63-75) duplicates test infrastructure. Consider extracting this into a shared test helper function to improve maintainability and reusability across test files.
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_690692b218ec832cb8b5cead05a642b4