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.
If an entity has bundles, then the '/add' page should be a list of bundles with links to add each.
At the moment, it crashes :/
Comments
Comment #1
joachim CreditAttribution: joachim commentedHere's a patch.
A few things I'm not sure what to do about:
- entity_ui_bundle_add_page() and entity_ui_get_bundle_add_form() could technically be moved to a .inc file, but the module doesn't have a .admin.inc or .pages.inc, so I wasn't sure about adding one
- unlike node_menu() we only make one menu item for ENTITYPATH/add/BUNDLE, rather than one item per bundle. The main reason for this seems to be that the main 'add' page can be generated with system_admin_menu_block().
- because bundles don't have descriptions, we can't follow the UI pattern of theme_node_add_list(), so I've just done a basic theme_item_list().
- there's no access granularity on the different bundle types. I'm not sure that's in the scope of EntityAPI, though I don't know how a module using this could add that in.
Comment #2
FATALIST CreditAttribution: FATALIST commentedThanks, joachim. Works fine for me.
Comment #3
ParisLiakos CreditAttribution: ParisLiakos commentedawesome!
thanks, exactly what i needed:)
Comment #4
ParisLiakos CreditAttribution: ParisLiakos commented#1: 1619628.entity.entity-add-ui-bundles.patch queued for re-testing.
Comment #5
fagoWhile I like the idea we must be careful to not break BC for modules. The patch does remove a menu item upon which current implementations may rely. Modules may rely on anything the existing UI controller does, thus I think it would be best to introduce this as another, alternate UI controller for bundled entities.
Comment #6
joachim CreditAttribution: joachim commentedThat makes sense.
Here's a patch that does the UI controller work in a new subclass.
I'm not sure whether to add the paths below BASE/add as BASE/add/%, or one menu item for each bundle as BASE/add/BUNDLE.
I've made a few more additions to the patch, based on implementations of this UI pattern I've made in custom modules.
Comment #7
PatchRanger CreditAttribution: PatchRanger commented@joachim Thanks, your patch works for me. Actually it replaced all of my custom stuff, which is doing the same thing: providing consistent add page.
My vote is for separate menu item for each bundle: it empowers Admin Menu (and other menu-related modules) to create additional menu items for adding entities of each type, which is great, because avoids custom coding to achieve that.
So (basing on your patch) I've created a new one: see attached, as well as interdiff.
Comment #8
fagoThat might differ for a specific entity type though. We should make entity-access checks on an empty entity having just the bundle set.
If modules use a uid they can default to the current-user in their create() method on the storage controller.
We should define the protected variable then.
Comment #9
fagoGiven our bundles are also entities I think the name might be confusing, maybe better call it "EntityBundleableUIController".
Also it's a bit weird is that we have to use the 'admin ui' keys in hook_entity_info() - but well, we cannot change that anymore so I guess we have to live with that.
Comment #10
dasjoi believe fago forgot to mention, that his review was based on #6.
attached is a patch that addresses the comments from #8.
#9 hasn't been implemented!
Comment #11
fagoindeed, thanks.
Needs to be updated to reflect latest code changes.
We should rename the controller - it's not for entity bundles but for bundlable entities. -> EntityBundleableUIController ? -> See #9.
Comment #12
dasjothanks for the review fago!
i have incorporated your comments into the attached patch.
Comment #13
fagook, patch looks good. However, one more thing:
This is for content entities, looking closer at it it's a bit weird to have the admin listing at the specified path and no view callback. So let's make this first, then base this upon that.
-> Let's solve #1105618: UI controller for content-entities, then inherit this class from the content UI controller provided there.
Comment #14
dasjothis patch is now based on #1105618-12: UI controller for content-entities
Comment #15
PatchRanger CreditAttribution: PatchRanger commentedComment #17
fagoThanks, I improved documentation a bit and committed it.