Skip to content

Support chages to show_helper in Rails 7 #2741

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 0 commits into from
Closed

Conversation

rickselby
Copy link

#2492 overrode show_helper, which in Rails 6 looks like:

def show_helper
  "#{singular_route_name}_url(@#{singular_table_name})"
end

But Rails 7 changed it to

def show_helper(arg = "@#{singular_table_name}", type: :url) # :doc:
  "#{singular_route_name}_#{type}(#{arg})"
end

This change supports the arguments in Rails 7 while maintaining support for Rails 6.

@rickselby rickselby changed the title Refactor show_helper to add the type argument Support chages to show_helper in Rails 7 Mar 5, 2024
@pirj
Copy link
Member

pirj commented Mar 5, 2024

Do you think it’s possible to add a failing example?

@rickselby
Copy link
Author

Not unless there's an easy way of providing a different template to a spec test.

Ideally I'd add a test template that called show_helper(type: :path) and could test the output of the generator with that template. But I can't see a way of doing that.

Also, if anyone has any preferences on how to address the rubocop failure, I'll make a change.

Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

👋 thanks for looking at this, can you provide a spec for this and see how we usually prefer to define these kind of conditionals.

If the refactoring doesn't take care of the rubocop warning then add a disable to the class.

@rickselby
Copy link
Author

I've updated the change to remove the override for Rails 6 (and have reverted show_helper to what it was before).

I'm still struggling to see a good way of setting up a spec test; but as I'm now not changing the function, just removing it from Rails 7, then the existing Rails 7 tests should show that it still works? The default templates do call show_helper.

@rickselby
Copy link
Author

Whoops! Restarted in #2833

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

Successfully merging this pull request may close these issues.

3 participants