Fixes SQL exception from rails 5.2 upgrade #479
Conversation
lizzyaustad
changed the title
lizzyaustad
force-pushed
the
rails_5_2_upgrade_fixes
branch
from
2edaeea to
048c516
Compare
There was a problem hiding this comment.
Hey @lizzyaustad , thanks for investigating that.
I would like to keep this PR open so that we are ready when Rails 5.2 is out of beta (I don't plan to release a version of cancancan to support Rails 5.2 until is out of beta).
When we are ready I'd prefer to have a activerecord_5_adapter.rb rather than extend the version 4 of it. this part you could already do if you have time. Thanks, nice job!
5.2.0.rc1 is out if that counts 😉 |
lizzyaustad
force-pushed
the
rails_5_2_upgrade_fixes
branch
from
435ef86 to
c80a563
Compare
|
5.2.0.rc2 is now out. Is there anything blocking this from getting merged? |
|
@lizzyaustad would you mind updating the PR to rc2? I would like to release this when also final 5.2 of Rails is out but we can merge it into a separate branch for the moment. |
lizzyaustad
force-pushed
the
rails_5_2_upgrade_fixes
branch
from
c80a563 to
81a73c7
Compare
|
@coorasse updated to rc2 |
@coorasse Rails 5.2 was released today. Any plans on releasing this? Thanks |
|
please @lizzyaustad , can you use the final 5.2 and add your changes to the CHANGELOG? Then I can merge and release a new 2.2.0 version. Thank you very much for your work, really appreciated 👍 |
lizzyaustad
force-pushed
the
rails_5_2_upgrade_fixes
branch
from
81a73c7 to
8b32f03
Compare
|
Hi @coorasse, I've updated to the final 5.2 version of rails and added my change to the "Unreleased" section of the changelog. Let me know if there's anything else I should do. Thanks! |
|
Thank you! Nice job! |
Add support for Rails 5.2
Add support for Rails 5.2
Add support for Rails 5.2
Required for the Rails 5.2 upgrade; specifically, CanCanCommunity/cancancan#479 is required to fix an ActiveRecord query issue that we're seeing on controllers with autoloaded resources. Reading through the changelog, it looks like the only major changes between this and our previous version are [several deprecations](https://github.com/CanCanCommunity/cancancan/blob/develop/CHANGELOG.md#200-may-18th-2017) for situations we aren't using.
A change in the most recent beta version of Rails 5.2 wraps the query attribute in a BindParam node, making the attribute's value inaccessible when it comes back to cancancan.
If the change persists in the final version of Rails 5.2,
SubstituteBindscould be a possible solution for avoiding the SQL error that is otherwise raised. Without this change, current active record adapter tests would fail for the 5.2.0.beta2 appraisal. I can update the gem to the non-beta version so this PR can be up to date once the final version is released if the issue still exists at that point.