diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c675535b..00ba9647 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -27,9 +27,9 @@ jobs: matrix: ruby-version: ["3.2", "3.3", "3.4", "3.5"] rails-version: - - "7.1" - "7.2" - "8.0" + - "8.1" - "main" runs-on: ubuntu-latest env: @@ -51,11 +51,11 @@ jobs: matrix: ruby-version: ["3.2", "3.3", "3.4", "3.5"] rails-version: - - "7.1" - "7.2" - "8.0" + - "8.1" - "main" - postgres-version: ["14", "15", "16", "17"] + postgres-version: ["14", "15", "16", "17", "18"] runs-on: ubuntu-latest services: postgres: @@ -92,9 +92,9 @@ jobs: matrix: ruby-version: ["3.2", "3.3", "3.4", "3.5"] rails-version: - - "7.1" - "7.2" - "8.0" + - "8.1" - "main" mysql-version: ["8.0", "8.4", "9.4"] diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..328c627c --- /dev/null +++ b/Makefile @@ -0,0 +1,26 @@ +.PHONY: spec spec-sqlite spec-postgres spec-mysql help + +# Default target +help: + @echo "Available targets:" + @echo " make spec - Run specs with all databases (default)" + @echo " make spec-sqlite - Run specs with SQLite" + @echo " make spec-postgres - Run specs with PostgreSQL" + @echo " make spec-mysql - Run specs with MySQL" + +# Run specs with all databases (default) +spec: spec-sqlite spec-postgres spec-mysql + +# Run specs with SQLite +spec-sqlite: + bundle exec rspec + +# Run specs with PostgreSQL +# Uses POSTGRES_URL, then DATABASE_URL, then defaults to local PostgreSQL +spec-postgres: + @DATABASE_URL=$${POSTGRES_URL:-$${DATABASE_URL:-postgres://postgres:statesman@localhost/statesman_test}} bundle exec rspec + +# Run specs with MySQL +# Uses MYSQL_URL, then DATABASE_URL, then defaults to local MySQL +spec-mysql: + @DATABASE_URL=$${MYSQL_URL:-$${DATABASE_URL:-mysql2://foobar:password@127.0.0.1/statesman_test}} bundle exec rspec diff --git a/lib/statesman/adapters/active_record_queries.rb b/lib/statesman/adapters/active_record_queries.rb index 6f486457..8bbedd67 100644 --- a/lib/statesman/adapters/active_record_queries.rb +++ b/lib/statesman/adapters/active_record_queries.rb @@ -75,7 +75,7 @@ def define_in_state(base, query_builder) states = states.flatten joins(most_recent_transition_join). - where(query_builder.states_where(states), states) + where(query_builder.states_where(states)) end end @@ -84,7 +84,7 @@ def define_not_in_state(base, query_builder) states = states.flatten joins(most_recent_transition_join). - where("NOT (#{query_builder.states_where(states)})", states) + where(query_builder.states_where(states).not) end end end @@ -101,20 +101,31 @@ def initialize(model, transition_class:, initial_state:, end def states_where(states) + transition_table = transition_class.arel_table + aliased_table = transition_table.alias(most_recent_transition_alias) + to_state_column = aliased_table[:to_state] + + in_states = to_state_column.in(states) + if initial_state.to_s.in?(states.map(&:to_s)) - "#{most_recent_transition_alias}.to_state IN (?) OR " \ - "#{most_recent_transition_alias}.to_state IS NULL" + in_states.or(to_state_column.eq(nil)) else - "#{most_recent_transition_alias}.to_state IN (?) AND " \ - "#{most_recent_transition_alias}.to_state IS NOT NULL" + in_states.and(to_state_column.not_eq(nil)) end end def most_recent_transition_join - "LEFT OUTER JOIN #{model_table} AS #{most_recent_transition_alias} " \ - "ON #{model.table_name}.#{model_primary_key} = " \ - "#{most_recent_transition_alias}.#{model_foreign_key} " \ - "AND #{most_recent_transition_alias}.most_recent = #{db_true}" + transition_table = transition_class.arel_table + aliased_table = transition_table.alias(most_recent_transition_alias) + + join_condition = model.arel_table[model_primary_key]. + eq(aliased_table[model_foreign_key]). + and(aliased_table[:most_recent].eq(true)) + + model.arel_table. + join(aliased_table, Arel::Nodes::OuterJoin). + on(join_condition). + join_sources end private @@ -151,10 +162,6 @@ def most_recent_transition_alias @most_recent_transition_alias || "most_recent_#{transition_name.to_s.singularize}" end - - def db_true - model.connection.quote(true) - end end end end diff --git a/spec/statesman/adapters/active_record_queries_spec.rb b/spec/statesman/adapters/active_record_queries_spec.rb index aba6ccbb..ed80dab4 100644 --- a/spec/statesman/adapters/active_record_queries_spec.rb +++ b/spec/statesman/adapters/active_record_queries_spec.rb @@ -130,6 +130,62 @@ def configure_new(klass, transition_class) end end + describe "update_all with most_recent join (Rails 8.1 compatibility)" do + let(:future_time) { Time.current + 1.day } + + context "updating records in a specific state" do + it "successfully executes update_all with in_state join" do + affected = MyActiveRecordModel.in_state(:succeeded).update_all(updated_at: future_time) + expect(affected).to eq(1) + expect(model.reload.updated_at).to be_within(1.second).of(future_time) + end + + it "correctly filters records by state" do + affected = MyActiveRecordModel.in_state(:succeeded).update_all(updated_at: future_time) + expect(affected).to eq(1) + expect(other_model.reload.updated_at).to be < future_time + end + + it "handles initial state correctly" do + affected = MyActiveRecordModel.in_state(:initial).update_all(updated_at: future_time) + expect(affected).to eq(2) # initial_state_model and returned_to_initial_model + end + end + + context "updating records not in a specific state" do + it "successfully executes update_all with not_in_state join" do + affected = MyActiveRecordModel.not_in_state(:succeeded).update_all(updated_at: future_time) + expect(affected).to be >= 1 + expect(other_model.reload.updated_at).to be_within(1.second).of(future_time) + end + + it "does not update records in the excluded state" do + before_time = model.updated_at + MyActiveRecordModel.not_in_state(:succeeded).update_all(updated_at: future_time) + expect(model.reload.updated_at).to be_within(1.second).of(before_time) + end + end + + context "with additional where clauses" do + let!(:another_succeeded_model) do + m = MyActiveRecordModel.create + m.state_machine.transition_to(:succeeded) + m + end + + it "respects additional where conditions with update_all" do + affected = MyActiveRecordModel. + in_state(:succeeded). + where(id: model.id). + update_all(updated_at: future_time) + + expect(affected).to eq(1) + expect(model.reload.updated_at).to be_within(1.second).of(future_time) + expect(another_succeeded_model.reload.updated_at).to be < future_time + end + end + end + context "with a custom name for the transition association" do before do # Switch to using OtherActiveRecordModelTransition, so the existing