enriclluelles/route_translator

Not working with Rails > 6.0.0.rc1

louim opened this issue · 8 comments

louim commented

Hey!

We are using the rails 6-0-stable branch while waiting for Rails to cut another Release candidate.

This PR rails/rails#34656 has recently been merged to rails master and backported to the rails 6-0-stable branch. It added an extra argument to the Mapper::Mapping.new rails/rails@85855d6#diff-2cfaf53c860732fea8689d6f2002594bR60 which makes it incompatible with the current version of route_translator here:

::ActionDispatch::Routing::Mapper::Mapping.new(route_set, translated_path_ast, defaults, controller, default_action, scope[:module], to, formatted, scope_constraints, blocks, via, translated_options_constraints, anchor, options)

I would be happy to submit a patch, but i'm not quite sure how you would like to handle the case to keep backward compatibility.

To summarize:

😩

I'm going to wait for rc2 release

@louim I've found another way to implement this

Could you please giva a try to the rails-gt-6.0.0.rc1-compatibility branch?

louim commented

@tagliala I can confirm it's working as expected with the rails 6 stable branch!

louim commented

rc2 is now out 🎉, @tagliala were you planning to merge #197 in the master branch?

@louim

Since this is potentially a breaking change if someone did some monkey patching, I'm going to release a new major version.

Since we are potentially breaking things and there is a potential DOS issue with route translator, I would like to merge #200 and #198 too.

I was waiting for an answer on both those branches but I didn't get one

Anyway... The only required change for a standard usage of this gem with both #198 and #200 merged, is to explicitly add around_action :set_locale_from_url to the ApplicationController

If you can also run your application specs against

and

I would be happy to merge everything and release version7 of route translator

louim commented

@tagliala I merged both PR locally on the develop branch https://github.com/hooktstudios/route_translator/tree/develop and tested with our app. I saw no problems. We were already skipping the around_action , I was able to remove the skip_around_action in our app and confirm that the application still worked correctly.

@louim thanks for the heads-up

I will release a new version in the next few hours

7.0.0 released