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

Fix association names when scaffolding join models if they don't match the table name #848

Merged
merged 12 commits into from
Jun 13, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,11 @@ def run
transformer.suppress_could_not_find = false

# Add the `has_many ... through:` association in both directions.
transformer.add_has_many_through_associations(has_many_through_transformer)
inverse_transformer.add_has_many_through_associations(inverse_has_many_through_transformer)
# We pass the "opposing" attribute so that both the association name and the
# class name get wired up correctly in cases where they don't match. For instance
# if you want an `assigned_to_membership` relationship to the `memberships` table.
transformer.add_has_many_through_associations(has_many_through_transformer, attributes[1])
inverse_transformer.add_has_many_through_associations(inverse_has_many_through_transformer, attributes[0])

additional_steps = (transformer.additional_steps + has_many_through_transformer.additional_steps + inverse_transformer.additional_steps + inverse_has_many_through_transformer.additional_steps).uniq

Expand Down
14 changes: 14 additions & 0 deletions bullet_train-super_scaffolding/lib/scaffolding/attribute.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ def association_class_name
name.split("_id").first
end

def plural_association_name
association_class_name.tableize
Copy link
Contributor

Choose a reason for hiding this comment

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

tableize will also pluralize, which I don't know if we want for belongs_to/has_one associations. For those, we'd probably want underscore.tr("/", "_").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 Hmm, good point. Currently I think this method is only being called in places where we have a has_many or has_many through: relationship, but I can see how this would be confusing and potentially lead to problems down the road. I'll clean it up a bit.

end

def class_name_matches?
# if namespaces are involved, just don't...
# TODO: I'm not entirely sure that extracting this conditional was the right thing to do.
# Are there scenarios where we want to assume a match even when namespaces are involved?
if options[:class_name].include?("::")
return false
end
name_without_id.tableize == options[:class_name].tableize.tr("/", "_")
end
Comment on lines +65 to +73
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the cases that this logic is for, I'm having a bit of a hard time understanding it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kaspth This is used in situations where you're scaffolding a relationship with a name that doesn't match the name of the table/model you're relating to. See the main description in this issue for an example: bullet-train-co/bullet_train#1508 (comment)


def is_association?
is_belongs_to? || is_has_many?
end
Expand Down
34 changes: 22 additions & 12 deletions bullet_train-super_scaffolding/lib/scaffolding/transformer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -603,10 +603,27 @@ def add_has_many_association
has_many_string.split(",").first.split(":").last
end

def add_has_many_through_associations(has_many_through_transformer)
def add_has_many_through_associations(has_many_through_transformer, attribute_definition)
attribute = Scaffolding::Attribute.new(attribute_definition, :crud_field, 0)
has_many_association = add_has_many_association
has_many_through_string = has_many_through_transformer.transform_string("has_many :completely_concrete_tangible_things, through: :$HAS_MANY_ASSOCIATION")
has_many_through_parts = [
"has_many :completely_concrete_tangible_things",
"through: :$HAS_MANY_ASSOCIATION"
]

unless attribute.class_name_matches?
has_many_through_parts << "class_name: \"Scaffolding::CompletelyConcrete::TangibleThing\""
end
has_many_through_string = has_many_through_transformer.transform_string(has_many_through_parts.join(", "))
has_many_through_string.gsub!("$HAS_MANY_ASSOCIATION", has_many_association)
unless attribute.class_name_matches?
# This handles the case where you're generating a join model where you want association names
# to be different than the class name, so it'll transform something like this:
# has_many :memberships, through: :assignments, class_name: "Membership"
# into something like this:
# has_many :assigned_to_memberships, through: :assignments, class_name: "Membership"
has_many_through_string.gsub!("has_many :#{attribute.plural_association_name}", "has_many :#{attribute.name_without_id.tableize}")
end
add_line_to_file(transform_string("./app/models/scaffolding/absolutely_abstract/creative_concept.rb"), has_many_through_string, HAS_MANY_HOOK, prepend: true)
end

Expand Down Expand Up @@ -1142,15 +1159,8 @@ def set_default_#{attribute.name}

end

class_name_matches = attribute.name_without_id.tableize == attribute.options[:class_name].tableize.tr("/", "_")

# but also, if namespaces are involved, just don't...
if attribute.options[:class_name].include?("::")
class_name_matches = false
end

# unless the table name matches the association name.
unless class_name_matches
unless attribute.class_name_matches?
if migration_file_name
# There are two forms this association creation can take.
replace_in_file(migration_file_name, "foreign_key: true", "foreign_key: {to_table: \"#{attribute.options[:class_name].tableize.tr("/", "_")}\"}", /t\.references :#{attribute.name_without_id}/)
Expand All @@ -1167,7 +1177,7 @@ def set_default_#{attribute.name}
# if the `belongs_to` is already there from `rails g model`..
scaffold_replace_line_in_file(
"./app/models/scaffolding/completely_concrete/tangible_thing.rb",
class_name_matches ?
attribute.class_name_matches? ?
"belongs_to :#{attribute.name_without_id}#{optional_line}" :
"belongs_to :#{attribute.name_without_id}, class_name: \"#{attribute.options[:class_name]}\"#{optional_line}",
"belongs_to :#{attribute.name_without_id}"
Expand All @@ -1177,7 +1187,7 @@ def set_default_#{attribute.name}
# however, this won't do anything if the association is already there.
scaffold_add_line_to_file(
"./app/models/scaffolding/completely_concrete/tangible_thing.rb",
class_name_matches ?
attribute.class_name_matches? ?
"belongs_to :#{attribute.name_without_id}#{optional_line}" :
"belongs_to :#{attribute.name_without_id}, class_name: \"#{attribute.options[:class_name]}\"#{optional_line}",
BELONGS_TO_HOOK,
Expand Down
Loading