NoMethodError: undefined method 'to_sym' for CustomStrategy

[rocket-turtle]

Description

After upgrading from Version 6.2.0 to 6.2.1 our custom strategy was broken. -> NoMethodError: undefined method 'to_sym' for CustomStrategy

This commit broke our code https://github.com/thoughtbot/factory_bot/commit/1b81d5dc258e5f1ad25c5109cb4bf3f6be83deb4

The change is not mentioned in the NEWS.rb nor in the GETTING_STARTED.md

https://www.rubydoc.info/gems/factory_bot/file/GETTING_STARTED.md#custom-strategies

Solution

If someone comes here with the same Problem it is super easy to fix. Just add the to_sym to your strategy. :)

---

Question

Could someone explain to me or point me in the direction where I can find more information why the custom strategies should be defined like this and not like the one underneath. There is no problem i'm just curious.

class JsonStrategy
  def initialize
    @strategy = FactoryBot.strategy_by_name(:create).new
  end

  delegate :association, to: :@strategy

  def result(evaluation)
    @strategy.result(evaluation).to_json
  end

  def to_sym
    :json
  end
end

class JsonStrategy < FactoryBot::Strategy::Create
  def result(evaluation)
    super.to_json
  end

  def to_sym
    :json
  end
end

[EdmundKorley]

Great catch! I've noted this breaking change in https://github.com/thoughtbot/factory_bot/commit/fc9bb59e434aaa5b6b96eb41015dcb093c4bfc33 for now. It seems like the now required to_sym on a custom strategy didn't get documented but we would really love an PR / update to the GETTING_STARTED guide.

For your second question, there isn't really a particular reason rather favoring "composition over inheritance". When you subclass from a framework's internal class, then users custom code can be tightly coupled to such undocumented changes in the framework code like what happened here.

[bkDJ]

This also breaks a custom strategy that inherits from an existing one, though the error will not mention to_sym since the wrong strategy will be used instead. A project I work on had ReadOnly < Create to add records to what is normally a read-only database, inspired by this, so the error message was: ActiveRecord::ReadOnlyRecord: record is marked as readonly

Adding the following to the class definition fixed it for us as well. 👍

def to_sym
  :read_only
end

[chris-at-work]

Thank you for the workaround and link to fc9bb59. However, this should have not been a patch level gem version bump.

[composerinteralia]

It sounds like this was an unintentional breaking change. It'd be good to get it fixed and send out another patch level bump including the fix.

I think to do that we need a test to catch the regression, and then a fallback to the old behavior with something like:

if @build_strategy.respond_to?(:to_sym)
   @build_strategy.to_sym
else
  @build_strategy.class
end

Then later on we can deprecate the else branch and remove it in the next major version.

[chris-at-work]

That sounds fine and explanation is appreciated. There's no impact to us now with the to_sym workaround and the if/else would continue to work. I realize now that my phrasing about patch levels was meant to be a point about semver process but not considerate that this change was unintentional. My apologies. 🌻