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

Support Rails 6.1.3.2 & 6.0.3.7 and use symbols for polymorphic routes #1972

Merged
merged 1 commit into from
May 6, 2021

Conversation

lostmahbles
Copy link
Contributor

@lostmahbles lostmahbles commented May 5, 2021

This potentially fixes #1971. I didn't do a thorough dive, but these changes got our admin back up and running. This also encompasses the changes in #1970.

@@ -4,7 +4,7 @@ class Resource
attr_reader :namespace, :resource

def initialize(namespace, resource)
@namespace = namespace
@namespace = namespace.to_sym

Choose a reason for hiding this comment

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

question: is this necessary?

I have the feeling that Resource as an inner class of Namespace is only ever initialized from there. At least I found now other places when grepping the codebase. And in Namespace, namespace is already made sure to be a symbol.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch! I removed this in the branch I'm trying to test and I think you're right.

The security updates bumping Rails for 6.1.3.2 and 6.0.3.7 due to a set
of CVEs also meant we needed to switch to using symbols for polymorphic
routes. This commit bumps the version of Rails dependencies to 6.1.3.2
and also switches to using symbols.
@nickcharlton nickcharlton changed the title #1971 Use symbols for polymorphic routes Support Rails 6.1.3.2 & 6.0.3.7 and use symbols for polymorphic routes May 6, 2021
@nickcharlton
Copy link
Member

I've tested this on a thoughtbot project which uses Administrate fairly extensively, plus I'm trusting the now passing test suite to be capturing the original error before the symbol change successfully. I'm going to merge this once CI passes and then sort out a new release today.

If anyone's following along and having other issues relating to these Rails releases (both 6.1.3.2 and to 6.0.3.7) I'd love to hear about them!

n-b added a commit to betagouv/rdv-service-public that referenced this pull request May 6, 2021
administrate currently crashes in rails 6.0.3.7 and 6.1.3.2 with the error “Please use symbols for polymorphic route arguments.”. A fix is coming in thoughtbot/administrate#1972, but we can’t really wait, and we can’t rollback rails either.

👋 Coucou la team DS demarches-simplifiees/demarches-simplifiees.fr#6178
n-b added a commit to betagouv/rdv-service-public that referenced this pull request May 6, 2021
administrate currently crashes in rails 6.0.3.7 and 6.1.3.2 with the error “Please use symbols for polymorphic route arguments.”. A fix is coming in thoughtbot/administrate#1972, but we can’t really wait, and we can’t rollback rails either.

👋 Coucou la team DS demarches-simplifiees/demarches-simplifiees.fr#6178
@nickcharlton nickcharlton merged commit a31b058 into thoughtbot:main May 6, 2021
@ELepolt
Copy link

ELepolt commented May 6, 2021

Utilizing this commit I have the following two lines, and I am still getting this error. Any thoughts on which part is still upset?

- Administrate::Namespace.new(namespace).resources.each do |resource|
    = link_to(display_resource_name(resource), [namespace.to_sym, resource.path], class: "navigation__link navigation__link--#{nav_link_state(resource)}")

@cgrothaus
Copy link

I think resource.path is a String, and that is causing the error in Rails routing. It must be a Symbol instead, try resource.path.to_sym. Also, I think that namespace is already a Symbol, thanks to the fix, so you can omit .to_sym there.

@kemenaran
Copy link

kemenaran commented May 6, 2021

You may need to update your _navigation.html.erb file according to the latest template from administrate :

  <%= link_to(
      display_resource_name(resource),
-      [namespace, resource.path],
+      resource_index_route(resource),
      class: "navigation__link navigation__link--#{nav_link_state(resource)}"
    ) %>

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.

Rails 6.1.3.2 breaks Administrate
5 participants