Insert new model adapters at the start of the list #640
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes implementing new model adapters easier. The example in the docs adds an adapter for non-activerecord. That works fine - none of the builtin adapters will work with it. But consider the use case where you want to add a more specific adapter for activerecord, and don't want to fork cancancan.
Currently this will not work, because
IDProxyAdapter
will come afterActiveRecord5Adapter
inCanCan::ModelAdapters::AbstractAdapters#subclasses
. SoActiveRecord5Adapter
will always get used.By adding new adapters to the start of the subclasses list, we inverse this. All the builtin adapters will end up last in the list; all adapters added in userland will be considered first.
This is safe with all built in adapters because they are written in an exclusive way (AR4, AR5). As a result, no tests should break here. However, this could cause problems for users who have added AR adapters in the wild if they haven't added to them the start of the
subclasses
list. For that reason I think this should be considered a breaking change.If this is accepted I'm happy to also tidy up the docs to make it more clear how easy adapters are to add.