Skip to content

Conversation

@minh-biocommons
Copy link
Collaborator

@minh-biocommons minh-biocommons commented Dec 5, 2025

Description

AAI-560: Simplify revocation/rejection reason handling and update timestamp to be timezone aware

Changes

  • Simplified revocation/rejection reason as the front-end can already use updated_by and updated_at from the response
  • Update the updated_at timestamp to be timezone aware so that the front-end can format it correctly

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have added unit / integration tests that prove my fix is effective or that my feature works
  • I have run all tests locally and they pass
  • I have updated the documentation (if applicable)
  • For any new secrets, I have updated the shared spreadsheet and the GitHub Secrets.

Copy link
Contributor

Copilot AI left a 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 simplifies revocation/rejection reason handling by removing automatic concatenation of timestamp and actor information, making the frontend responsible for displaying this metadata using the updated_by and updated_at fields already included in API responses. Additionally, it makes all datetime columns timezone-aware at the database level for PostgreSQL.

  • Removed the _reason_with_metadata helper function that was appending timestamp and actor email to reasons
  • Updated database schema to use timezone-aware timestamps (TIMESTAMP WITH TIME ZONE for PostgreSQL)
  • Added defensive timezone handling in get_data() methods for backward compatibility

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
db/models.py Removed _reason_with_metadata function; updated DateTime fields to use timezone=True; simplified revoke() and reject() methods to store raw reasons; added defensive timezone handling in get_data() methods
db/types.py Changed datetime type to AwareDatetime for updated_at fields in data models; removed unused datetime import
migrations/versions/c5ec222dd539_make_datetime_columns_timezone_aware.py Added Alembic migration to convert existing timestamp columns to timezone-aware for PostgreSQL while preserving SQLite compatibility
tests/test_admin.py Updated test assertions to expect simplified reasons without timestamp/actor metadata; removed unused freeze_time import and frozen_time fixture
tests/db/test_models.py Updated test assertions to expect simplified reasons; removed freeze_time usage from revoke tests while preserving it for other tests that still need it

Copy link
Contributor

@amandazhuyilan amandazhuyilan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super!

@minh-biocommons minh-biocommons merged commit 237f9f8 into main Dec 10, 2025
11 checks passed
@minh-biocommons minh-biocommons deleted the reason-timezone branch December 10, 2025 23:08
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.

4 participants