Problems with ruby/rails constants can prevent the use of nested concepts

[debradley]

When using TRB alongside ActiveRecord, the TRB docs recommend following this pattern:

app/models/foo.rb # => class Foo
app/concepts/foo/cell/show.rb # => module Foo::Cell; class Show < Trailblazer::Cell

Note that, in this scenario, we end up using the name Foo as both a class and a namespace. However, this is known to be problematic.

It seems that you can work around the problem to some degree by writing your module constants in compact form, as recommended in the TRB docs i.e.:

# Note the default Rubocop rules complain about this, preferring nested style
module BlogPost::Cell
  class New < Trailblazer::Cell
  end
end

However, you will definitely run into problems if you nest your concepts. Creating something like this will not work (Rails throws an error undefined method relation_delegate_class for Foo::Bar:Class):

app/concepts/foo/bar/cell/show.rb # => module Foo::Bar::Cell; class Show < Trailblazer::Cell

There are a few possible solutions:

• Don't use nested concepts: limiting but workable. It may be that the class/module conflict causes other issues I haven't seen yet though.
• Always use a top-level namespace for your concepts:

app/concepts/my_app/foo/bar/cell/show.rb # => module MyApp::Foo::Bar::Cell; class Show < Trailblazer::Cell

• Follow a convention of pluralizing your concepts - supported in TRB but generally discouraged in the docs; singular names are considered a best practice:

app/concepts/foos/bar/cell/show.rb # => module Foos::Bar::Cell; class Show < Trailblazer::Cell

Changes in Ruby 2.5 may make this moot. Or not. Rails constant autoloading may continue to cause problems. I don't understand the issue well enough to say.

Currently, my thinking is that TRB should document this issue clearly and promote the use of pluralized concepts over singular.

[apotonick]

I use nested concepts all the time, they shouldn't cause you a headache at all.

Can you show me what your nested namespaces are? Is Foo an AR class, what is Bar, etc? Just make some examples so I can setup a test case. :beers:

[debradley]

Here's the setup, in simplified form, though I don't know if it's sufficient to replicate the problem. It may end up being specific to ruby/rails versions or other complexities of my setup. I'm also wondering, after writing this up whether, if I nest a concept, if I also have to nest the corresponding AR model.

AR Models

# app/models/blog_post.rb
class BlogPost < ActiveRecord::Base; end

# app/models/comment.rb
class Comment < ActiveRecord::Base; end

Flat - works

# app/concepts/blog_post/cell/show.rb
module BlogPost::Cell
  class Show < Trailblazer::Cell; end
end

# app/concepts/comment/cell/show.rb
module Comment::Cell
  class Show < Trailblazer::Cell; end
end

Nested - doesn't work

# app/concepts/blog_post/comment/cell/show.rb
module BlogPost::Comment::Cell
  class Show < Trailblazer::Cell; end
end

[debradley]

I created a new app as a minimal test case and was able to replicate the issue as described above. Any attempt to reference the blog.comments relationship gave the error: undefined method relation_delegate_class for BlogPost::Comment:Module

But during this process, I also saw a warning I hadn't seen before:

app/concepts/blog_post/comment/cell/show.rb:1: warning: toplevel constant Comment referenced by BlogPost::Comment

I don't know if this warning only surfaces in Rails 5 (my problem came up in a Rails 4 app), or if, more likely, I just missed it before now.

As the warning suggested, I modified the AR Comment model to match the same namespace as in the cells and the problem was solved.

I do still think there's probably a documentation task here, with some detail on how the namespacing and nesting of AR and Cells classes need to be in sync with each other if the class names match.

[apotonick]

Of course this doesn't work: module BlogPost::Comment::Cell :laughing:

This is pure Ruby and means that BlogPost::Comment has to be defined already.

If it isn't, this will work.

module BlogPost::Comment
  module Cell
    class Show < ...

[epoberezhny]

@apotonick hi! I have the same problem undefined method 'relation_delegate_class' for Post::Comment:Module in this example project trailblazer_blog. Maybe it makes sense to abandon a convention of singularized concepts and follow a convention of pluralized concepts?

[n-rodriguez]

Maybe it makes sense to abandon a convention of singularized concepts and follow a convention of pluralized concepts?

:+1:

As a rule of thumb : always use pluralized concepts to not clash with models.

[apotonick]

Nope, I mildly disagree. Plurals make things more complicated and hard to memoize, and they're not necessary. Instead, we switched to the "Rails Way" of naming things and abandoned loader: http://trailblazer.to/2.1/index.html#trailblazer-rails :grimacing:

[n-rodriguez]

Plurals make things more complicated and hard to memoize, and they're not necessary.

lol. model => models, so hard...

[apotonick]

Haha, not the plural itself, where to apply it!!! :joy: (routes => plural, controller => plural, model => singular, table => singular, route helper => singular, etc etc etc)

[debradley]

Instead, we switched to the "Rails Way" of naming things and abandoned loader

What's the current estimate on a 2.1 release?

[n-rodriguez]

Ok. And what about zeitwerk?

[apotonick]

March 31

[n-rodriguez]

@apotonick the big deal is to rename all operations to the new naming scheme :/

[apotonick]

@n-rodriguez That is correct, but development mode gets a lot faster since now Rails' magic is applied where it loads only what you need... at the moment... Also, reloading works "more reliable".

[n-rodriguez]

@apotonick renaming done (693 changed files with 5116 additions and 4360 deletions) :tada:

Also, reloading works "more reliable".

reloading and loading : some files had incorrect names. Trailblazer loader was more permissive in this case. With Rails loader bin/rails s crash with uninitialized constant.

Also the Trailblazer documentation is wrong. This doesn't work :

# config/initializers/trailblazer.rb

YourApp::Application.config.trailblazer.enable_loader = false

You must disable Trailblazer loader in config/application.rb :

# config/application.rb

config.trailblazer.enable_loader = false

[n-rodriguez]

As a side effect of this change, simplecov has a better detection of what is really tested : the code coverage decreased from 91% to 86% :rofl:

[apotonick]

Hahahaha, but your coverage looks quite well - when will you show me that app??

[n-rodriguez]

but your coverage looks quite well

Thanks to you and Trailblazer :+1:

when will you show me that app??

I'd love to but it's a customer application (a CRM actually) :/

[n-rodriguez]

[unrooty]

Had the same issues. Used the solution that proposed by @n-rodriguez plural form of contracts like

# app/concerns/account/contacts/contracts
module Account::Contacts::Contracts

and it works.

[n-rodriguez]

@apotonick

Haha, not the plural itself, where to apply it!!! joy (routes => plural, controller => plural, model => singular, table => singular, route helper => singular, etc etc etc)

I understand the intention.

But in Rails, controllers are plural because they can apply on a single object or on a collection of objects. (index, new, create, etc) : UsersController, PostsController, etc... (Like the DB : tables are plural for the same reason)

Of course you can have singular controllers (SessionController, ProfileController, etc...), but the convention is to use plural controllers.

I tend to use the same logic to name Trailblazer operations. As already said, it also helped me in the early days to not clash with models.

Example :

# app/concepts/application_operation.rb
class ApplicationOperation < Trailblazer::Operation
end

# app/concepts/accounts/operation/index.rb
module Accounts::Operation
  class Index < ApplicationOperation
  end
end

# app/concepts/accounts/operation/create.rb
module Accounts::Operation
  class Create < ApplicationOperation
  end
end

# app/concepts/accounts/contract/create.rb
module Accounts::Contract
  class Create < ApplicationForm
  end
end

[apotonick]

TBH, I never understood the reasoning behind the "controller operates on a collection" argument. When I create a user, I add to a user collection, but I create one user. When I create a session, I add to a session collection, because there are many, but I create one session, etc, and so on bla bla.

I chose to simplify things and make everything singular. I dont understand why I would have Accounts::Operation::Create. I create an account. "Create account" => Create::Operation::Account. :thinking:

[unrooty]

@apotonick I think that you'r probably right, but as long as there's problem with correlation of models and nested contracts we need to use plural names for nested concepts :(

[apotonick]

@unrooty Hm, ok, now I'm confused. Is the problem that you have a model called Contract?

[unrooty]

@apotonick I have nested contract named

# app/concepts/account/contact
module Account::Contact::Contract
  class Create > Reform::Form
    ...
  end
end

And I have model named Contact Then I write following code in Account::Create operation:

class Account::Create < Trailblazer::Operation
  step Model(Account, :find)
  ........................
  def load_contacts!(context, *)
    context[:contacts] = context[:model].contacts
  end
end

And I have ecxeption undefined method relation_delegate_class for Account::Contact:Module.

When I rename contract module name Contact to Contacts then issue disappears.

Also when I add follwing code to Account model then issue disappears:

class Account < ActiveRecord::Base
  has_many :contacts, class_name: '::Contact'
end

I think that Rails tries to use Account::Contact:Module as AR model and throws exception.

[nbulaj]

problem that you have a model called Contract

We discussed this with you in late 2016, this issue still valid AFAIR.

[nbulaj]

Seems like @unrooty problem related to Rails dependency management and constant resolution. AR association couldn't resolve proper constant name under namespaces (and because he has Contact class and Account::Contact module), so renaming it to plural name allows to find a proper constant.