Rails 5 upgrade

[Matt-Yorkley]

WIP quick test of the build...

[luisramos0]

nice, 76 broken specs. Looks like a lot of small different issues. What's your perception of complexity of this one Matt?

[Matt-Yorkley]

I think it's pretty close. 76 out of 4282 so... 1.8% failing?

I think some of the tricky bits are around modifying the params in weird ways. They've made it much harder to do (to discourage it), but there's a few places where we really mess with the params object a lot.

The other bit is the way we use collections and the ModelSet classes for bulk updates. The places where we deviate from Rails conventions and have made some messy code is the stuff that's that failing. I think by the time the build is green we will have to have tidied up the codebase quite a bit (which is actually a good thing).

[Matt-Yorkley]

I just fixed another 12 or so errors over morning coffee, we should be at around ~64 remaining.

[luisramos0]

this abstraction makes it very easy to move easily to master on dual boot:

def raw_params
    if ENV['DEPENDENCIES_NEXT']=1
       @raw_params ||= params.to_unsafe_h
    else
        params
    end
end

[Matt-Yorkley]

This raw_params thing was a bit of a rough experiment, I assumed this would be cleaned up later. I'm not even sure this fix is the way to go...? 🤷‍♂️

It's a tricky one. The new object returned by params behaves totally differently in Rails 5.

[luisramos0]

Ok. I think the pattern of having a service or concern to handle it as an adapter is a good one 👍

[Matt-Yorkley]

It's not a simple problem. In places where we use strong params in a fairly simple way, everything works as before ✔️

In places where we try to modify the params in any way or expect it to behave like a hash, everything is broken ❌

For example we might try to delete a params key in a before_filter callback for permissions reasons like:

params.delete(:order_id) unless has_permission?

Nope. ❌

😞

[luisramos0]

This PR is looking great! Only 24 broken specs 🎉
If you make a PR with all the calls to raw_params and use that if statement I put above. That will make this rails 5 PR surprisingly small! Awesome news!

[Matt-Yorkley]

Only 14 left!

[luisramos0]

This can be closed now, right @Matt-Yorkley?