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 :/

Files: 
CommentFileSizeAuthor
#14 1619628-14.entity.entity-add-ui-bundles.patch8.88 KBdasjo
FAILED: [[SimpleTest]]: [MySQL] 5 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#14 1619628-14.entity.entity-add-ui-bundles_interdiff.txt513 bytesdasjo
#12 1619628-12.entity.entity-add-ui-bundles.patch8.85 KBdasjo
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]
#12 1619628-12.entity.entity-add-ui-bundles-interdiff.txt993 bytesdasjo
#10 1619628-10.entity.entity-add-ui-bundles.patch8.95 KBdasjo
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]
#10 1619628-10.entity.entity-add-ui-bundles-interdiff.txt1.42 KBdasjo
#7 entity-1619628-7-ui_for_bundles_add_page.diff8.55 KBPatchRanger
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]
#7 interdiff.txt1.98 KBPatchRanger
#6 1619628-6.entity.entity-add-ui-bundles.patch8.35 KBjoachim
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]
#1 1619628.entity.entity-add-ui-bundles.patch6.21 KBjoachim
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new6.21 KB
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]

Here'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.

Thanks, joachim. Works fine for me.

Status:Needs review» Reviewed & tested by the community

awesome!
thanks, exactly what i needed:)

Status:Reviewed & tested by the community» Needs work

While 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.

Status:Needs work» Needs review
StatusFileSize
new8.35 KB
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]

That 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.

StatusFileSize
new1.98 KB
new8.55 KB
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]

@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.

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.

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.

Status:Needs review» Needs work

+++ b/entity.module
@@ -114,6 +114,54 @@ function entity_object_load($entity_id, $entity_type) {
+ * This shows links to all bundles, as we don't have access permissions at a
+ * bundle granularity.

That 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.

+++ b/includes/entity.ui.inc
@@ -30,7 +30,8 @@ class EntityDefaultUIController {
+    // Set this on the object so classes that extend hook_menu() can use it.
+    $this->id_count = count(explode('/', $this->path));

We should define the protected variable then.

EntityBundlesUIController

Given 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.

Status:Needs work» Needs review
StatusFileSize
new1.42 KB
new8.95 KB
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]

i 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!

Status:Needs review» Needs work

indeed, thanks.

+++ b/entity.module
@@ -114,6 +114,62 @@ function entity_object_load($entity_id, $entity_type) {
+ * This shows links to all bundles, as we don't have access permissions at a
+ * bundle granularity.

Needs to be updated to reflect latest code changes.

+++ b/includes/entity.ui.inc
@@ -493,6 +495,50 @@ class EntityDefaultUIController {
+class EntityBundlesUIController extends EntityDefaultUIController {

We should rename the controller - it's not for entity bundles but for bundlable entities. -> EntityBundleableUIController ? -> See #9.

Status:Needs work» Needs review
StatusFileSize
new993 bytes
new8.85 KB
PASSED: [[SimpleTest]]: [MySQL] 374 pass(es).
[ View ]

thanks for the review fago!

i have incorporated your comments into the attached patch.

Status:Needs review» Postponed

ok, 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.

StatusFileSize
new513 bytes
new8.88 KB
FAILED: [[SimpleTest]]: [MySQL] 5 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Status:Postponed» Needs review

Status:Needs review» Needs work

The last submitted patch, 1619628-14.entity.entity-add-ui-bundles.patch, failed testing.

Status:Needs work» Fixed

Thanks, I improved documentation a bit and committed it.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.