Fix table alias generation mismatch in ConditionsExtractor

[daveallie]

Previously in the provided test case, the aliases would have been generated as:

• mentions_users
• mentions_users_2
• mentions_users_2_3

This was causing issues where the join would be aliases as mentions_users_3, however the where statement would be querying a column off mentions_users_2_3 when using accessible_by

After this change, the aliases are generated as:

• mentions_users
• mentions_users_2
• mentions_users_3

This allows table names to correctly match up and the query to succeed.

---

Happy to provide a deeper example if needed, however I would need to anonymise the table names

[daveallie]

This may address #696

[daveallie]

@coorasse is there anything you'd like to discuss or need me to action here?

[daveallie]

@coorasse is there someone better to help get this merged, as far as I can tell, there shouldn't be anything blocking this

[daveallie]

@coorasse it has been over a year since I opened this PR, based on the activity on the repo, you're still engaging with it. Would you mind reviewing this? I'd love to come back onto mainline and off my fork, but can't until this is resolved.

[dramalho]

I just got bitten by this myself , super cool that it's fixed - simple enough PR - but unsure if it raises any subtle questions elsewhere that's stopping the merge?

Are you guys monkey patching in the meantime?

[daveallie]

I've just required the gem directly from my fork:

# Awaiting merge of https://github.com/CanCanCommunity/cancancan/pull/786 and subsequent release
gem 'cancancan', git: 'https://github.com/daveallie/cancancan.git',
                 ref: 'c7dbdb9bb7dae4bc290397c34d09f5b871679d5e'

[dramalho]

yeah, or that :) -- I don't want to annoy Alessandro but hoping it gets official :) -- thank you for the patch Dave

[daveallie]

Coming up to 3 years since I initially proposed this one, we have been using this patch in production since I original opened the PR, and haven't run into any issues (obviously a sample size of 1 isn't super convincing, but anecdotally it's working fine). Is there any chance of getting this one merged in @coorasse?