Skip to content

Conversation

@sirwolfgang
Copy link

@sirwolfgang sirwolfgang commented Sep 20, 2025

This Add/Fixes support for Postgres Range types: #1486

Check the following boxes:

  • Wrote good commit messages.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new
    code introduces user-observable changes.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.

Related: #1487

Copy link

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 adds support for PostgreSQL range column types (daterange, numrange, tsrange, tstzrange, int4range, int8range) to PaperTrail's versioning system. Previously, these types were not properly serialized/deserialized when tracking changes to model attributes containing range values.

Key changes:

  • Introduced PostgresRangeSerializer to handle serialization/deserialization of PostgreSQL range types
  • Modified attribute serializers to detect and process range columns even when using JSON storage
  • Refactored variable naming from klass/item_class to model_class for consistency

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
lib/paper_trail/type_serializers/postgres_range_serializer.rb New serializer class that handles PostgreSQL range types with deserialize logic for Ruby-formatted range strings
lib/paper_trail/attribute_serializers/attribute_serializer_factory.rb Added range type detection and factory method to instantiate PostgresRangeSerializer
lib/paper_trail/attribute_serializers/object_attribute.rb Added logic to serialize range columns even in JSON mode; renamed variables to model_class
lib/paper_trail/attribute_serializers/object_changes_attribute.rb Added logic to serialize range columns even in JSON mode; renamed variables to model_class
lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb Renamed variables from klass to model_class for consistency
spec/paper_trail/type_serializers/postgres_range_serializer_spec.rb Comprehensive tests for deserializing various PostgreSQL range types
spec/paper_trail/attribute_serializers/object_attribute_spec.rb Integration tests for range serialization/deserialization; updated test descriptions
CHANGELOG.md Added entry documenting the fix for range type support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

### Fixed

- None
- Fixed errors when deserializing Range types from Ruby style strings to Postgres
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The CHANGELOG entry states "Fixed errors when deserializing Range types from Ruby style strings to Postgres" but this is confusing. It should clarify whether it's deserializing FROM Ruby strings or FROM Postgres format strings. Based on the code, it appears to deserialize Ruby-style range strings (e.g., "1..5") to Ruby Range objects. Consider rewording to: "Fixed support for PostgreSQL range column types in object/object_changes serialization"

Suggested change
- Fixed errors when deserializing Range types from Ruby style strings to Postgres
- Fixed support for PostgreSQL range column types in object/object_changes serialization

Copilot uses AI. Check for mistakes.
expect(attrs["post_ids"]).to eq [1, 2, 3]
end

it "serializes a postgres range into a ruby array" do
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The test description says "serializes a postgres range into a ruby array" but it should say "serializes a postgres range into a ruby range" since ranges are being serialized as ranges, not arrays.

Suggested change
it "serializes a postgres range into a ruby array" do
it "serializes a postgres range into a ruby range" do

Copilot uses AI. Check for mistakes.
def deserialize_with_ar(string)
return nil if string.blank?

delimiter = string[/\.{2,3}/]
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The regex pattern /\.{2,3}/ will match 2 or more consecutive dots, which means it could match "....", ".....", etc. This should be /\.{2}|\.{3}/ or more simply /(\.\.\.?)/ to match only ".." or "..." exactly.

Suggested change
delimiter = string[/\.{2,3}/]
delimiter = string[/(\.\.\.?)/]

Copilot uses AI. Check for mistakes.
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.

1 participant