The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.If you look at the class 'EntityDBExtendable' you extend, all the CRUD stuff is already implemented their. We should fix up the code to make use of that.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | search_api_crud.patch | 1.84 KB | fago |
Comments
Comment #1
fagohere is a small patch showing how this could work. If you prefer you could stay with functions for CRUD too, I've done so for profile2 too. Still they should $entity->delete() then or better don't use the entity DB class at all. (You may just invoke the controller yourself too or use the provided entity_delete()).
Comment #2
fagoComment #3
drunken monkeyI can understand that you hate to see duplication of code here, as you have implemented already this generically for all kinds of entities, I myself don't really like the current CRUD workflow in the module and would readily switch to using the whole functionality of the CRUD API. However, as an outsider, your module is too sparely documented and uses too much indirection (with controller classes, entity classes, and back again) to be really understood, so I would have to always do some digging before being able to use some function/method with confidence. (I know, core does the same indirection stuff – I don't like it there either, but at least it's better documented, and there are of course more examples for those.)
It was too much of a hassle to do that when first implementing this functionality, and it also now keeps me from diving into this.
Also, this is neither urgent nor can't be done after a release, so this will have to wait either way. As you know, I don't have much spare time at the moment. Or is there any real benefit, besides the usual when removing duplication?
Comment #4
fagoAs said in the exportable issue, it's an CRUD API - so of course the API assumes it is used when it is used :P Not using it doesn't play with the rest of the module like the generated rules integration and modules like entity metadata would use the "wrong" CRUD callbacks. Thus setting back to bug report.
>Also, this is neither urgent nor can't be done after a release, so this will have to wait either way.
You could fix it without any API change. Just fix the methods in your EntityDB class and let your functions point to it.
Well, it was an alpha release at that time and I offered my help if you have questions. Still, there always was the README what explains the basics and I don't think the indirection of invoking a controller class is something that hard to get. Anyway, now there are more docs. ;)
@benefits:
Well, first off your API would be consistent with how the rest of drupal works - involving the CRUD functions itself, but also the associated hooks. Then the CRUD API has stuff implemented like exportables, or transaction support you'll get. Lastly, it's less work to maintain that way.
Comment #5
drunken monkeyOK, I didn't know that things would fall apart if I didn't use the CRUD functionality. I've seen the handbook page you linked to, but it doesn't really mention this either. It also lack examples on how to actually use this functionality, but just quickly goes over what you need to define.
But as mentioned in #939482: Fix exportables, I'll have to rewrite most of this stuff anyways, so I can as well fix this.
Comment #6
drunken monkeySince this is also needed to correctly support exportables, I'm marking this as a duplicate of #939482: Fix exportables. That way, all documentation about the upcoming API changes are contained in a single issue/comment, too.
Comment #7
fagoIt has:
Anyway, I'try to clarify that better.