paper-trail-gem / paper_trail

Track changes to your rails models
MIT License
6.79k stars 896 forks source link

Postgres Date Ranges not reify-ing correctly #1486

Open sirwolfgang opened 2 months ago

sirwolfgang commented 2 months ago

Thank you for your contribution!

Due to limited volunteers, issues that do not follow these instructions will be closed without comment.

Check the following boxes:

Due to limited volunteers, we cannot answer usage questions. Please ask such questions on StackOverflow.

Bug reports must use the following template:

# frozen_string_literal: true

# Use this template to report PaperTrail bugs.
# Please include only the minimum code necessary to reproduce your issue.
require "bundler/inline"

# STEP ONE: What versions are you using?
gemfile(true) do
  ruby "3.3.0"
  source "https://rubygems.org"
  gem "activerecord", "7.1.3.4"
  gem "minitest", "5.11.3"
  gem "paper_trail", "15.1.0", require: false
  gem "pg", "1.5.7"
end

require "active_record"
require "minitest/autorun"
require "logger"

# Please use sqlite for your bug reports, if possible.
ActiveRecord::Base.establish_connection('postgres://postgres:password@localhost')
ActiveRecord::Base.connection.execute('DROP DATABASE IF EXISTS paper_trail_test')
ActiveRecord::Base.connection.execute('CREATE DATABASE paper_trail_test')
ActiveRecord::Base.establish_connection('postgres://postgres:password@localhost/paper_trail_test')
ActiveRecord::Base.logger = nil
ActiveRecord::Schema.define do
  # STEP TWO: Define your tables here.
  create_table :users, force: true do |t|
    t.string :name
    t.daterange :range
    t.timestamps null: false
  end

  create_table :versions do |t|
    t.string :item_type, null: false
    t.integer :item_id, null: false
    t.string :event, null: false
    t.string :whodunnit
    t.json :object
    t.json :object_changes
    t.datetime :created_at
  end
  add_index :versions, %i[item_type item_id]
end
ActiveRecord::Base.logger = Logger.new(STDOUT)
require "paper_trail"

# STEP FOUR: Define your AR models here.
class User < ActiveRecord::Base
  has_paper_trail
end

# STEP FIVE: Please write a test that demonstrates your issue.
class BugTest < ActiveSupport::TestCase
  def test_1
    record  = User.create(name: 'Version 0')
    record.update!(name: 'Version 1', range: 2.week.ago.to_date..2.week.from_now.to_date)
    record.update!(name: 'Version 2')

    assert_nothing_raised do
      record.versions[1].next.reify.range
    end
  end

  def test_2
    record  = User.create(name: 'Version 0', range: 1.week.ago.to_date..1.week.from_now.to_date)
    record.update!(name: 'Version 1', range: 2.week.ago.to_date..2.week.from_now.to_date)
    record.update!(name: 'Version 2')

    assert_nothing_raised do
      record.versions[0].next.reify.range
    end
  end
end

# STEP SIX: Run this script using `ruby my_bug_report.rb`
sirwolfgang commented 2 months ago

On the rails side its dying because it's failing to parse the ruby string style range from with the postgres style range splitter:

# https://github.com/rails/rails/blob/16882b886e245f363cd61f5ea553fe2bc525d070/activerecord/lib/active_record/connection_adapters/postgresql/oid/range.rb#L69
def extract_bounds(value)
  from, to = value[1..-2].split(",", 2)

  {
    from:          (from == "" || from == "-infinity") ? infinity(negative: true) : unquote(from),
    to:            (to == "" || to == "infinity") ? infinity : unquote(to),
    exclude_start: value.start_with?("("),
    exclude_end:   value.end_with?(")")
  }
end

I'm not totally sure if this is something we should fix here in papertrails, or upstream in rails.