fix: allow timezone-aware datetime in Block schema#63
fix: allow timezone-aware datetime in Block schema#63
Conversation
This change adds a validator to the `Block` schema to handle timezone-aware datetimes. It converts any aware datetime to UTC and then makes it naive, which is required by the `TIMESTAMP WITHOUT TIME ZONE` column in the database. This allows the API to accept ISO 8601 strings with 'Z' or other offsets, while maintaining compatibility with the existing naive datetime behavior. - Modified `server/app/schemas/block.py` to add `validate_timezone` validator. - Added `server/tests/schemas/test_block.py` to test the validator. Co-authored-by: kuu13580 <46004336+kuu13580@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello @kuu13580, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue where sending timezone-aware datetimes to block creation/update endpoints would result in a database error due to the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Visit the preview URL for this PR (updated for commit 7d6fe8e): https://tabi-share-8ef6b--pr63-fix-block-schema-tim-l0devlsj.web.app (expires Sun, 15 Feb 2026 02:21:40 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 9f2a87ede127df7673322845e34cf22c1372d720 |
There was a problem hiding this comment.
Code Review
このプルリクエストは、タイムゾーンを持つdatetimeが送信された場合に発生するデータベースエラーを修正するものです。Pydanticのfield_validatorを使用して、タイムゾーン付きの日時をUTCに変換し、タイムゾーン情報を削除してnaiveなdatetimeとしてデータベースに保存するアプローチは適切です。また、この変更を検証するためのテストケースが追加されている点も素晴らしいです。スタイルガイドに従い、日本語でレビューします。1点、テストコードの保守性を向上させるための提案をさせていただきました。
| from datetime import datetime, timezone | ||
| from app.schemas.block import BlockCreate, BlockType | ||
|
|
||
| def test_timezone_naive_input(): | ||
| # Naive string: 2023-01-01T10:00:00 | ||
| data = { | ||
| "title": "Naive", | ||
| "start_time": "2023-01-01T10:00:00", | ||
| "end_time": "2023-01-01T12:00:00", | ||
| "detail": "detail", | ||
| "block_type": "event", | ||
| } | ||
| block = BlockCreate(**data) | ||
| assert block.start_time.tzinfo is None | ||
| assert block.start_time == datetime(2023, 1, 1, 10, 0, 0) | ||
| assert block.end_time.tzinfo is None | ||
| assert block.end_time == datetime(2023, 1, 1, 12, 0, 0) | ||
|
|
||
| def test_timezone_aware_utc_input(): | ||
| # Aware UTC string: 2023-01-01T10:00:00Z | ||
| data = { | ||
| "title": "Aware UTC", | ||
| "start_time": "2023-01-01T10:00:00Z", | ||
| "end_time": "2023-01-01T12:00:00Z", | ||
| "detail": "detail", | ||
| "block_type": "event", | ||
| } | ||
| block = BlockCreate(**data) | ||
| # Pydantic parses Z as UTC aware. The validator should convert to naive UTC. | ||
| assert block.start_time.tzinfo is None | ||
| assert block.start_time == datetime(2023, 1, 1, 10, 0, 0) | ||
| assert block.end_time.tzinfo is None | ||
| assert block.end_time == datetime(2023, 1, 1, 12, 0, 0) | ||
|
|
||
| def test_timezone_aware_offset_input(): | ||
| # Aware Offset string: 2023-01-01T19:00:00+09:00 (JST) -> 10:00 UTC | ||
| data = { | ||
| "title": "Aware Offset", | ||
| "start_time": "2023-01-01T19:00:00+09:00", | ||
| "end_time": "2023-01-01T21:00:00+09:00", | ||
| "detail": "detail", | ||
| "block_type": "event", | ||
| } | ||
| block = BlockCreate(**data) | ||
| # The validator should convert to UTC and make naive. | ||
| assert block.start_time.tzinfo is None | ||
| assert block.start_time == datetime(2023, 1, 1, 10, 0, 0) | ||
| assert block.end_time.tzinfo is None | ||
| assert block.end_time == datetime(2023, 1, 1, 12, 0, 0) |
There was a problem hiding this comment.
3つのテストケース(test_timezone_naive_input, test_timezone_aware_utc_input, test_timezone_aware_offset_input)は、テストロジックがほぼ同じです。pytest.mark.parametrizeを使用することで、これらのテストを1つのパラメータ化されたテストにまとめることができます。これにより、コードの重複が減り、保守性が向上します。これは、スタイルガイドの「保守性」と「一貫性」の原則にも合致します。
提案コードでは、3つのテストを1つにまとめ、関連するimport文も整理しています。
import pytest
from datetime import datetime
from app.schemas.block import BlockCreate
@pytest.mark.parametrize(
("test_id", "start_time_in", "end_time_in", "expected_start", "expected_end"),
[
(
"naive",
"2023-01-01T10:00:00",
"2023-01-01T12:00:00",
datetime(2023, 1, 1, 10, 0, 0),
datetime(2023, 1, 1, 12, 0, 0),
),
(
"aware_utc",
"2023-01-01T10:00:00Z",
"2023-01-01T12:00:00Z",
datetime(2023, 1, 1, 10, 0, 0),
datetime(2023, 1, 1, 12, 0, 0),
),
(
"aware_offset",
"2023-01-01T19:00:00+09:00",
"2023-01-01T21:00:00+09:00",
datetime(2023, 1, 1, 10, 0, 0),
datetime(2023, 1, 1, 12, 0, 0),
),
],
)
def test_datetime_validation(test_id, start_time_in, end_time_in, expected_start, expected_end):
"""Datetime validation should handle naive, UTC, and offset-aware inputs."""
data = {
"title": f"Test {test_id}",
"start_time": start_time_in,
"end_time": end_time_in,
"detail": "detail",
"block_type": "event",
}
block = BlockCreate(**data)
assert block.start_time.tzinfo is None
assert block.start_time == expected_start
assert block.end_time.tzinfo is None
assert block.end_time == expected_end
This PR fixes an issue where sending timezone-aware datetimes (e.g. "2023-01-01T10:00:00Z") to the block creation/update endpoints would cause a database error because the column is
TIMESTAMP WITHOUT TIME ZONE.I added a Pydantic
field_validatortoBlockBasethat detects aware datetimes, converts them to UTC, and strips the timezone info to make them naive. This ensures consistent UTC storage in the database regardless of input format.I also added a new test file
server/tests/schemas/test_block.pycovering various scenarios.PR created automatically by Jules for task 12752935626976310622 started by @kuu13580