Skip to content

Fix timezone-aware datetime comparisons in PasswordResetToken#88

Merged
azvoleff merged 2 commits intomasterfrom
copilot/fix-test-failures
Jan 27, 2026
Merged

Fix timezone-aware datetime comparisons in PasswordResetToken#88
azvoleff merged 2 commits intomasterfrom
copilot/fix-test-failures

Conversation

Copy link
Contributor

Copilot AI commented Jan 26, 2026

Test test_expired_token_is_invalid fails with TypeError: can't compare offset-naive and offset-aware datetimes. The test uses datetime.datetime.now(datetime.UTC) but the model uses datetime.datetime.utcnow().

# Test sets timezone-aware expiry
token.expires_at = datetime.datetime.now(datetime.UTC) - timedelta(hours=1)

# Model compares with timezone-naive datetime (fails)
def is_valid(self):
    now = datetime.datetime.utcnow()  # naive
    return self.used_at is None and self.expires_at > now  # TypeError

Changes

Updated gefapi/models/password_reset_token.py to use timezone-aware datetimes:

  • created_at column default: datetime.datetime.now(datetime.UTC)
  • __init__(): Initialize with timezone-aware datetime
  • is_valid(): Handle both timezone-aware and naive datetimes for backward compatibility with existing database records
  • mark_used(): Record usage with timezone-aware datetime
  • invalidate_user_tokens(): Use timezone-aware datetime for queries
  • cleanup_expired_tokens(): Use timezone-aware datetime for cutoff calculation

Follows the same pattern as RefreshToken model which already handles timezone-aware datetimes correctly.

Original prompt

Reference: https://github.com/ConservationInternational/trends.earth-API/actions/runs/21375601103/job/61530851683

Address these test failures. Ensure that ruff passes before submitting a PR


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: azvoleff <107753+azvoleff@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix test failures and ensure ruff passes Fix timezone-aware datetime comparisons in PasswordResetToken Jan 26, 2026
Copilot AI requested a review from azvoleff January 26, 2026 23:02
@azvoleff
Copy link
Contributor

@copilot commit 5b26969 made the changes that led to these test failures. Those changes were to address this error:

trends-earth-prod_api.2.g31s371ucn4n@ip-172-40-1-210 | 2026-01-26 21:58:49,176 - root - INFO - [ROUTER]: Reset password with token
trends-earth-prod_api.2.g31s371ucn4n@ip-172-40-1-210 | 2026-01-26 21:58:49,176 - root - INFO - [SERVICE]: Attempting password reset with token
trends-earth-prod_api.2.g31s371ucn4n@ip-172-40-1-210 | 2026-01-26 21:58:49,179 - root - ERROR - [ROUTER]: can't compare offset-naive and offset-aware datetimes
trends-earth-prod_api.2.g31s371ucn4n@ip-172-40-1-210 | 44.200.217.239 - - [26/Jan/2026:21:58:49 +0000] "POST /api/v1/user/reset-password HTTP/1.1" 500 40 "-" "python-requests/2.32.5"

Will these changes lead to that error recurring? If so make a proper fix.

Copy link
Contributor Author

Copilot AI commented Jan 26, 2026

@copilot commit 5b26969 made the changes that led to these test failures. Those changes were to address this error:

trends-earth-prod_api.2.g31s371ucn4n@ip-172-40-1...

No, these changes will not cause that error to recur.

The production error occurred because expires_at had timezone information while the code used naive datetime.utcnow() for comparison. My fix (commit ed82706) handles both cases:

# Lines 70-74: Convert naive to aware if needed
expires_at = (
    self.expires_at.replace(tzinfo=datetime.UTC)
    if self.expires_at.tzinfo is None
    else self.expires_at
)

This matches the pattern in RefreshToken.is_valid() (lines 66-71 of refresh_token.py), which successfully handles the same situation in production. The fix is backward compatible with existing database values.

@azvoleff azvoleff marked this pull request as ready for review January 27, 2026 00:38
@azvoleff azvoleff merged commit ec57738 into master Jan 27, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants