geekq / workflow

Ruby finite-state-machine-inspired API for modeling workflow
MIT License
1.75k stars 207 forks source link

Rename module Scopes to the more conventional ClassMethods #193

Closed Ferdy89 closed 5 years ago

Ferdy89 commented 7 years ago

To avoid altering module hierarchies on people's apps, the module named Scope can be renamed to ClassMethods, which is pretty conventional and is less likely to cause name collisions.

Here's the issue we've experienced in our Rails app, where we use Rails' autoloading algorithm and we have a convention of having scopes on a separate module for each model that we call Scopes, like:

# app/models/blog/scopes.rb
module Blog::Scopes
  extend ActiveSupport::Concern

  included do
    scope :recent, -> { order('created_at DESC').limit(10) }
  end
end
# app/models/blog.rb
class Blog < ActiveRecord::Base
  include Workflow
  include Blog::Scopes
end

Then we run into this issue

Blog.recent
# => NoMethodError: undefined method `recent' for #<Class:0x007fe5c0d95ca8>

When the line that includes Blog::Scopes is executed, we expect Rails' autoloader to go and load app/models/blog/scopes.rb. However, this does not happen because the constant already exists on Workflow::Adapter::ActiveRecord::Scopes which was included in Blog.

We started experiencing this issue when we upgraded from version 0.8.7 to 1.2.0. Before, the constant Blog::Scopes could not be resolved so it was autoloaded.

We can fix this by explicitly requiring blog/scopes. However, this a surprising behavior that requires a comment in code to be explained. We believe it'd be less surprising if the gem's constant was named ClassMethods, like many other modules conventionally do in the Rails community.

The change is very mild and involves no functional changes, just a rename to avoid name collision.