Updated: Comment #10

Problem/Motivation

To help general DX/code organisation bring entity bundle logic into the EntityManager. This will benefit unit testing (mocking etc..) and remove the need for calling a procedural function.

Proposed resolution

Convert entity_get_bundles() function in entity.inc to methods on the EntityManager. Add getBundleInfo() and getAllBundleInfo(), the module handler is already injected into EntityManager so we can use that.

Remaining tasks

None, patch ready.

User interface changes

None

API changes

Deprecate entity_get_bundles() in favour of new methods?

Original report by @dawehner

Files: 
CommentFileSizeAuthor
#8 2030151-8.patch5 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,022 pass(es).
[ View ]
#8 interdiff-2030151-8.txt1.26 KBdamiankloip
#12 2030151-12.patch5.15 KBdamiankloip
PASSED: [[SimpleTest]]: [MySQL] 58,960 pass(es).
[ View ]
#12 interdiff-2030151-12.txt519 bytesdamiankloip
#6 2030151-6.patch4.28 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 58,056 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#4 2030151-4.patch4.27 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2030151-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#3 2030151-3.patch811.16 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2030151-3.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#3 interdiff-2030151-3.txt2.16 KBdamiankloip
#1 drupal-2030151-1.patch3.71 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 58,026 pass(es), 291 fail(s), and 85 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new3.71 KB
FAILED: [[SimpleTest]]: [MySQL] 58,026 pass(es), 291 fail(s), and 85 exception(s).
[ View ]

There we go.

Status:Needs review» Needs work

The last submitted patch, drupal-2030151-1.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.16 KB
new811.16 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2030151-3.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

The bundle labels were still using $bundles variable, and we need to clear the bundleInfo static cache when the entity info definitions are cleared.

StatusFileSize
new4.27 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2030151-4.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Tim kindly pointed out that I had accidentally included the whole world in the last patch. interdiff is still good!

Status:Needs review» Needs work

The last submitted patch, 2030151-4.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new4.28 KB
FAILED: [[SimpleTest]]: [MySQL] 58,056 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

ok, lets try once more. Interdiff in #3 is still correct.

Status:Needs review» Needs work

The last submitted patch, 2030151-6.patch, failed testing.

StatusFileSize
new1.26 KB
new5 KB
PASSED: [[SimpleTest]]: [MySQL] 58,022 pass(es).
[ View ]

Forgot about this issue, rerolled and fixed the remaining failures in EntityReferenceAdminTest. Not sure why the current tests failed without this fix though... Using key() to get the bundle after iterating over it will return NULL as there is no current key to return.

Status:Needs work» Needs review
Issue tags:+Needs issue summary update

All looks fine beside the missing issue summary.

Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs issue summary update

Thank you

Status:Reviewed & tested by the community» Needs work

+++ b/core/includes/entity.inc
@@ -58,33 +57,12 @@ function entity_info_cache_clear() {
  *   The bundle info for a specific entity type, or all entity types.
  */
function entity_get_bundles($entity_type = NULL) {

I think we need at least an @see here to the new getBundleInfo and getAllBundleInfo methods.

Status:Needs work» Needs review
StatusFileSize
new519 bytes
new5.15 KB
PASSED: [[SimpleTest]]: [MySQL] 58,960 pass(es).
[ View ]

There we go.

Status:Needs review» Reviewed & tested by the community

The feedback from alex got adressed, so back to RTBC

Status:Reviewed & tested by the community» Fixed

Committed ea8a4bd and pushed to 8.x. Thanks!

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

Issue summary:View changes

Added proper issue summary

Issue summary:View changes

Shall we have a follow-up to actually replace entity_get_bundles() that already exist in core?