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

Allow model option for associations to be passed as string or constant. #16

Conversation

asedge
Copy link
Collaborator

@asedge asedge commented Mar 22, 2023

Passing model as a string is preferred so that the order of dependent class definitions doesn't matter.

Defining the model as a constant like this will fail with a NameError:

class Foo
  belongs_to :bar, model: Bar
end
class Bar; end

This could be problematic in Rails with the load order being unknown. By defining the model as a string we can lazy evaluate the constant which should be safer:

class Foo
  belongs_to :bar, model: 'Bar'
end
class Bar; end

@asedge asedge force-pushed the feature/allow_association_model_to_be_passed_as_string_or_constant branch from 9795cd6 to 659dc4b Compare March 22, 2023 15:30
Copy link
Contributor

@JeffLuckett JeffLuckett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it.

Copy link
Contributor

@TABeauchat TABeauchat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No comments. All looks good.

@TABeauchat
Copy link
Contributor

On a side note...we will probably want to alert everyone that we will need to update all our current AF models for this.

@asedge
Copy link
Collaborator Author

asedge commented Mar 23, 2023

On a side note...we will probably want to alert everyone that we will need to update all our current AF models for this.

It's probably not a bad idea to throw the information in one of our larger slack channels (I can do that) to make everyone aware. It's a backwards compatible change so no immediate action is needed.

@JeffLuckett JeffLuckett merged commit 1f31c9a into main Mar 23, 2023
@JeffLuckett JeffLuckett deleted the feature/allow_association_model_to_be_passed_as_string_or_constant branch March 23, 2023 14:32
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

Successfully merging this pull request may close these issues.

3 participants