Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

still polymorphic if modifying devise.rb (configuration & migrations) #828

Open
sebacruzd opened this issue Aug 5, 2020 · 1 comment
Open

Comments

@sebacruzd
Copy link

Hello,

From what I read in what's inserted in initializers/devise.rb after running

$ rails generate devise_invitable:install

If I were to modify this:

  # The class name of the inviting model. If this is nil,
  # the #invited_by association is declared to be polymorphic.
  # Default: nil
  # config.invited_by_class_name = 'User'

and uncomment the config.invited_by_class_name = 'User' line, I expected for the invited_by not to be polymorphic, since it says it will be polymorphic if it is nil, suggesting that it wouldn't be polymorphic if it wasn't nil.

To clarify, now that devise.rb block is

  # The class name of the inviting model. If this is nil,
  # the #invited_by association is declared to be polymorphic.
  # Default: nil
  config.invited_by_class_name = 'User'

As I read in the README, I manually changed my models/user.rb file and added the following line:

has_many :invitations, class_name: self.to_s, as: :invited_by

Later I ran

$ rails generate devise_invitable User

It annotated the models/user.rb file and also created a migration file (devise_invitable_add_to_users).

I expected it to have this

t.references :invited_by, foreign_key: { to_table: :users }

or something like that, but instead it had this:

t.references :invited_by, polymorphic: true

I manually edited the migration file and the migration worked as I expected, but from what the devise.rb file says I expected for that migration to be created with the foreign_key to the config.invited_by_class_name. If not, what's it for?

Am I wrong to have expected that behaviour?

@scambra
Copy link
Owner

scambra commented Aug 11, 2020

Setting is not used for migration in generator, it's only used to define association in the model:

In this line set class_name or polymorphic options for belongs_to:
https://github.com/scambra/devise_invitable/blob/master/lib/devise_invitable/models.rb#L33
Which is used in belongs_to association declaration:
https://github.com/scambra/devise_invitable/blob/master/lib/devise_invitable/models.rb#L48

But migration template has no check on that setting, neither invited_by_foreign_key or invited_by_counter_cache settings:
https://github.com/scambra/devise_invitable/blob/master/lib/generators/active_record/templates/migration.rb

You can change migration template to check that setting, or other related settings too, and send pull request. However those settings can be changed per model, as devise readme explains, if invited_by_class_name is set for one model only, generated migration must be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants