pluck change from array access to using methods.

[iberianpig]

Restore ActiveHash::Relation#pluck to execute the specified method on the record.

Background:

• Through v3.1.x: Executed method names given as symbols.
• In v3.2.x: Change impacted data retrieval by directly referencing attributes[key], affecting compatibility.

https://github.com/active-hash/active_hash/pull/269 broke functionality of gems like enumerize which rely on casting values accessed by column names.

Reproduce

1. Save following file as issue.rb.
2. Please switch version with uncomment line of gem "active_hash", "3.2.1" # fail test.
3. Run in terminal $ ruby issue.rb

#!/usr/bin/env ruby

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  gem 'enumerize'
  # gem "active_hash", "3.1.1" # pass test
  # gem "active_hash", "3.2.1" # fail test
  gem "active_hash", github: "active-hash/active_hash", ref: "6c51fa6d" # == master # fail test
  # gem "active_hash", github: "iberianpig/active_hash", branch: "fix/revert-pluck-behavior" # pass test
end

require "active_hash"
require "minitest/autorun"
require "logger"

class Country < ActiveHash::Base
  extend Enumerize
  enumerize :continent, in: { north_america: 1, south_america: 2, europe: 3, asia: 4, africa: 5, oceania: 6 }
  self.data = [
      {
        "id": 1,
        "name": "US",
        "custom_field_1": 1,
        "continent": 1
      },
      {
        "id": 2,
        "name": "Canada",
        "custom_field_2": 2,
        "continent": 1
      }
    ]
end
class BugTest < Minitest::Test
  def test_pluck_with_enumerize
    assert_equal "north_america", Country.find(1).continent  # continent is enumerized
    assert_equal [["US", "north_america"], ["Canada", "north_america"]], Country.all.pluck(:name, :continent)
  end
end

3.1.1

Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Run options: --seed 37444

# Running:

.

Finished in 0.000997s, 1002.6591 runs/s, 2005.3181 assertions/s.

1 runs, 2 assertions, 0 failures, 0 errors, 0 skips

3.2.1

Fetching gem metadata from https://rubygems.org/.........
Resolving dependencies...
Run options: --seed 14751

# Running:

F

Finished in 0.004705s, 212.5528 runs/s, 425.1055 assertions/s.

  1) Failure:
BugTest#test_pluck_with_enumerize [issue.rb:43]:
--- expected
+++ actual
@@ -1 +1 @@
-[["US", "north_america"], ["Canada", "north_america"]]
+[["US", 1], ["Canada", 1]]

1 runs, 2 assertions, 1 failures, 0 errors, 0 skips

[iberianpig]

(CI failed)It looks like Rails 7.1.3 breaks polymorphic associations ... :thinking:

[flavorjones]

@iberianpig see #300 for the Rails 7.1.3 fix

[kbrock]

kicking now that 300 was merged

[kbrock]

looking like the tests introduced by 299 are still passing. We had introduced a few other changes around a symbol or string column access, so maybe the changes in 299 are no longer needed?

[iberianpig]

@kbrock Please confirm Reproduce section in this issue. You can reproduce this issue in your terminal.

looking like the tests introduced by 299 are still passing.

I rebased and pushed this branch, then All checks in CI have all passed. However it's still failing with $ ruby issue.rb .

  1) Failure:
BugTest#test_pluck_with_enumerize [issue.rb:44]:
--- expected
+++ actual
@@ -1 +1 @@
-[["US", "north_america"], ["Canada", "north_america"]]
+[["US", 1], ["Canada", 1]]

1 runs, 2 assertions, 1 failures, 0 errors, 0 skips

[kbrock]

@iberianpig Sorry, I was not clear. Let me try again.

Good job. The tests introduced by #269 are still passing. I said this because I thought this reverted the code introduced in #269 but it turns out this is not a revert, but rather a fix for the regression/bug introduced by #269

Request:

Is there a way to introduce a test (that does not rely upon enumerize) to our code base? I would like to protect us from changing this code and breaking this for you in the future.

[iberianpig]

Thank you for your feedback. I've already add tests for #pluck that it does not rely on enumerize. Could you please take a moment to review them in This PR?

[iberianpig]

@kbrock When is the release likely to include the contents of this pull request? :smile: