Skip to content

Commit

Permalink
Fix add_column behavior for missing type
Browse files Browse the repository at this point in the history
When using `add_column`, the definition methods for `ksuid` and
`ksuid_binary` are not used like when using `table.ksuid` since
`add_column` creates the column directly.

This change makes it so any column definition can handle both `ksuid`
and `ksuid_binary` correctly by monkey-patching the method that creates
the column definitions.
  • Loading branch information
michaelherold committed Oct 21, 2023
1 parent 568f28d commit 14e859c
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 3 deletions.
6 changes: 6 additions & 0 deletions activerecord-ksuid/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased](https://github.com/michaelherold/ksuid-ruby/compare/v1.0.0...main)

### Fixed

- Using a `ksuid` or `ksuid_binary` type with `add_column` in migrations will now work correctly without throwing an error about an unknown column type.

## [1.0.0](https://github.com/michaelherold/ksuid-ruby/compare/v0.5.0...v1.0.0) - 2023-02-25

### Added
Expand Down
8 changes: 5 additions & 3 deletions activerecord-ksuid/lib/active_record/ksuid/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ class Railtie < ::Rails::Railtie
ActiveSupport.on_load :active_record do
require 'active_record/ksuid/table_definition'

ActiveRecord::ConnectionAdapters::TableDefinition.include(
ActiveRecord::KSUID::TableDefinition
)
ActiveRecord::ConnectionAdapters::TableDefinition.descendants.each do |defn|
next unless defn.name

defn.prepend(ActiveRecord::KSUID::TableDefinition)
end
end
end
end
Expand Down
22 changes: 22 additions & 0 deletions activerecord-ksuid/lib/active_record/ksuid/table_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,28 @@ def ksuid(*args, **options)
def ksuid_binary(*args, **options)
args.each { |name| column(name, :binary, **options.merge(limit: 20)) }
end

# Monkey-patches defining a new column within a table
#
# @api private
# @private
#
# @param name [String, Symbol] the name of the column
# @param type [String, Symbol] the type of the column
# @param options [Hash<Symbol, Object>] options for the definition
# @return [ActiveRecord::ConnectionAdapters::ColumnDefinition]
def new_column_definition(name, type, **options)
case type.to_s
when 'ksuid'
prefix_length = options.delete(:prefix)&.length || 0

super(name, :string, **options.merge(limit: 27 + prefix_length))
when 'ksuid_binary'
super(name, :binary, **options.merge(limit: 20))
else
super
end
end
end
end
end
17 changes: 17 additions & 0 deletions activerecord-ksuid/spec/active_record/ksuid/railtie_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,23 @@ class EventPrefix < ActiveRecord::Base
end
end

context 'when writing a migration that adds KSUID fields' do
it 'can use the ksuid and ksuid_binary field types' do
connection = ActiveRecord::Base.connection

connection.create_table :field_tests do |t|
t.string :name
end

expect do
connection.add_column :field_tests, :id_one, :ksuid
connection.add_column :field_tests, :id_two, :ksuid_binary
end.not_to raise_error
ensure
connection.drop_table :field_tests, if_exists: true
end
end

matcher :issue_sql_queries do |expected|
supports_block_expectations

Expand Down

0 comments on commit 14e859c

Please sign in to comment.