CRUD
CRUD copied to clipboard
Fix for MongoDB: Configurable order by primary key check
WHY
BEFORE - What was wrong? What was happening before this PR?
We know there are some mongdb users using Backpack. We don't optimize specifically for mongodb nor "official" support it, but usually if we properly write our sql stuff and leave some bits extensible, mongodb and other dbms can work with backpack too.
It was reported here https://github.com/Laravel-Backpack/CRUD/issues/4601 that some of our code is conflicting for mongodb users because we don't leave an open door for them to work with this bit.
So what I did was:
- moved the
check for primary key orderinto a function - added the possibility for developer to define a model method that determines if there is primary key order or not
- added a constrain that only runs our code for sure in sql dbs
- if we can't determine it, we bail with "true" so that we don't try to apply the crud order as we can't determine if it has order or not.
I tought about other solution:
- introducing a ListOperation config, let's say
addPrimaryKeyOrder => true, that if dev changed to false we would not apply any order.
The reason why I didn't choose this solution was because applying our default primary key order, on mongodb for example, is not the issue, the issue is how we determine if it has order or not, that's why I opted for the second one that allow developer to keep using Backpack default functionality, just providing a different way to check if it has primary key order or not.
@tabacitu what you think in terms of architecture and solution ? Is checking for sqlDriver too much ? Technically $order[sql] would only be available with sql drivers... At the current moment, we will return that there is no order, but the truth is that we don't know if there is order or not. (there is, we just cannot find it properly).
Cheers