- Notifications
You must be signed in to change notification settings - Fork 688
[v5] Add missing page icon types and other misc fixes #603
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds missing page icon types and implements error handling improvements for the v5 SDK release. The changes focus on restoring previously removed page icon types and enhancing API error responses with additional metadata.
- Re-adds
ExternalPageIconRequestandCustomEmojiPageIconRequesttypes that were mistakenly removed - Implements support for the new
additional_datafield in API error responses - Commits missing build configuration files for examples
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/api-endpoints.ts | Restores missing page icon types and adds new type definitions |
| src/errors.ts | Adds support for additional_data field in API error handling |
| test/Client.test.ts | Adds test coverage for the new error response format |
| examples/generate-random-data/tsconfig.json | Adds missing TypeScript configuration file |
| examples/database-email-update/tsconfig.tsbuildinfo | Adds TypeScript build info file |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ...parsed, | ||
| code: parsed["code"], | ||
| message: parsed["message"], | ||
| additional_data: parsed["additional_data"] as AdditionalData | undefined, |
Copilot AI Aug 27, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a type assertion as AdditionalData | undefined bypasses type checking and could lead to runtime errors if the API returns additional_data with unexpected structure. Consider implementing proper validation or using a more permissive type definition.
| additional_data: parsed["additional_data"] as AdditionalData | undefined, | |
| additional_data: isAdditionalData(parsed["additional_data"]) ? parsed["additional_data"] : undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't really think of a good way to do a runtime check of the keys of an object being strings in JS :/ I feel like this is a necessary evil here (at the boundary of the web API call interface) and we kind of have to do some level of casting or ugly code and I chose the casting 😅)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yay!
…additional_data` (see makenotion/notion-sdk-js#603)
* - Added `DataSourcesEndpoint` methods - Refactored `DatabasesEndpoint` methods (note: no pytest tests included) * - Synced with commit `698cd6ca37cdc2d0757ff40b2fed492a774344c5` from `notion-sdk-js`: in `DataSourcesEndpoint.update`, added `"parent"` to the `body` parameter * - Updated to align with commit `c1fc444b556dbc7b8a9af1abd3b1447211dfb538` from `notion-sdk-js`: added support for `is_locked` in update methods of page and database APIs * - Updated response handling to include the new `data_source` field: `additional_data` (see makenotion/notion-sdk-js#603) * - Kept the current VCR version - Updated `tests\cassettes` to change `notion-version: - '2025-09-03'` - Modified `tests\conftest.py` for format compatibility * - Upgraded `vcrpy` from `<6` to `vcrpy==6.0.2` - Added `"is_locked"` to the `body` of the `update` function in `DatabasesEndpoint` * ruff format * ruff format * - Added type annotations to ensure code consistency and clarity - Hardcoded `page_id` * - Temporarily removed Python 3.7 version testing - Normalize code formatting * Temporarily removed Python 3.7 version testing(I missed submitting last time) * - Added `request_id` field - Improved tests for `request_id` and `additional_data` - Enhanced `_parse_response` to handle `additional_data` and `request_id` - Improved initialization tests for `APIResponseError` * - Kept only data source and database update changes - Removed mentions of Python 3.7 support from `README.md` and other docs - Updated minimum supported Python version to 3.8 * - Updated `page_id` retrieval path in `tests\conftest.py` - Set `"decode_compressed_response"` back to `True` to ensure complete content in `tests\cassettes` - Fixed typo: renamed `test_file_uploads_list_with_stat_cursor` to `test_file_uploads_list_with_start_cursor` * - Added `data_source_id` in `tests\conftest.py` to reduce redundant test code in `tests\test_endpoints.py`
This PR adds a few small fixes to our planned upcoming release of v5 of the SDK:
ExternalPageIconRequestandCustomEmojiPageIconRequesttypes underPageIconRequestadditional_datafield in public API error responses toAPIResponseErrorexamples/builds, e.g.examples/generate-random-data/tsconfig.json, that I forgot to commit up in [v5] Sync API shape changes fordata_sourceparents and search filter #602