Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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.