Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
After #1893820: Manage entity field definitions in the entity manager and #1893772: Move entity-type specific storage logic into entity classes the next step is to move the baseFieldDefinitions to the entity classes themselves.
Comment | File | Size | Author |
---|---|---|---|
#15 | move-basefielddefinitions-2024963-15.patch | 72.25 KB | Berdir |
#15 | move-basefielddefinitions-2024963-15-interdiff.txt | 1.3 KB | Berdir |
#10 | move-basefielddefinitions-2024963-10.patch | 71.99 KB | Berdir |
#10 | move-basefielddefinitions-2024963-10-interdiff.txt | 859 bytes | Berdir |
#8 | move-basefielddefinitions-2024963-8.patch | 71.15 KB | Berdir |
Comments
Comment #1
BerdirThe attached patch basically does that except that I haven't moved the implementations yet, I'm forwarding the call to the storage controller.
Comment #2
Berdir#1: move-basefielddefinitions-2024963-1.patch queued for re-testing.
Comment #4
BerdirRe-roll and moved definitions around. Allows us to get rid of a storage controller or two, but most still have some methods in there. Noticed a problem in EntityTestStorageController, we can't remove that one because it needs access to the entity type in create(). We should pass that through to the static functions...
Comment #6
BerdirAn unrelated change ended up being in the patch.
Comment #8
BerdirRemoved the call to the new removed method on the storage controller.
Comment #10
BerdirThis should be green.
Comment #11
chx CreditAttribution: chx commentedThere is no doubt that baseFieldDefinitions have absolutely nothing to do with storage.
Comment #12
BerdirThis is a not yet approved API change.
This only impacts entities that already switched to EntityNG, of which I doubt there are many. Fetching the field definitions works just as it did before. Updating for this is easy, just move the method from the storage controller to the entity class and make it static.
Comment #13
alexpottThis change makes total sense - approving.
Let's delegate this to the real View entity as ViewUI is a decorator used to add functionality when editing a View in the UI. Just in case config entities start to use this. How about something like
return View::baseFieldDefinitions($entity_type);
?I think this should just be
The entity type to return properties for.
as this is consistent with other similar function params. I also think it might worth documenting the use case as it is never used in core AFAICS.Comment #14
alexpottBerdir mentioned on IRC that once #2004244: Move entity revision, content translation, validation and field methods to ContentEntityInterface lands we will be able to remove the ViewUI code.
In that case I think we should only fix up the comment and outline the use-case for passing in $entity_type.
Comment #15
BerdirSomething like this?
Comment #16
chx CreditAttribution: chx commentedYES.
Comment #17
Berdir#15: move-basefielddefinitions-2024963-15.patch queued for re-testing.
Comment #18
alexpottCommitted 48ddeb4 and pushed to 8.x. Thanks!
Comment #19
BerdirWill provide a change notice around the whole field definition/base field definition stuff asap.
Comment #20
tstoecklerQuick follow-up: #2075503: Wrong issue linked in @todo in ViewUI::baseFieldDefinitions()
Comment #21
BerdirAdded this to https://drupal.org/node/1806650, and more importantly, wrote a documentation page about all this at https://drupal.org/node/2078241. Reviews and improvements welcome!