Fix EntityController::addPage so bundles are sorted by label and not ID

I added a patch to sort the bundles. The patch contains the fix and the test cases.
I also changed the issue summary as it was referencing to the wrong issue.

Regarding #3:

+++ b/core/lib/Drupal/Core/Entity/Controller/EntityController.php
@@ -336,6 +336,8 @@ protected function loadBundleDescriptions(array $bundles, EntityTypeInterface $b
+    $ordered_labels = array_column($bundles, 'label');
+    array_multisort($ordered_labels, $bundles);

This custom sort logic probably won't be suitable. I believe the sorting needs to be done using sort() method of the bundle entity type.

One idea suggested in the parent issue (here) was to use loadBundleDescriptions() also for sorting the bundles. I'll give that a go, patch coming up shortly.

This patch implements the idea brought up by alexpott in the parent ticket comment #132.

Not doing this as part of loadBundleDescriptions means we have to load the bundles twice which feels wasteful. It's tempting to make loadBundleDescriptions return the sorted list? If it did then contrib/custom usages would be fixed - see http://grep.xnddx.ru/search?text=loadBundleDescriptions&filename= But that has the downside of then becoming a really bad method name.

One concern in my implementation: I'm assuming bundle entity types always extend ConfigEntityBase. So I didn't add an if-clause to check whether sort() is callable or not. But I'm not sure if that's the case.

---

I'd like to ask about the bundle information array though:

Is it mandatory to always use EntityTypeBundleInfo::getBundleInfo() to get the list of bundles?

Seems like addPage() code could be simpler if bundle information array was used only when there is no bundle entity type. That would probably break some backwards compatibility though. Before returning the results, the bundle information service allows modules to add/remove some bundles by triggering alter('entity_bundle_info'). So I suppose addPage() must keep using getBundleInfo() in all situations?

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

#5 failed to apply on 10.1x

error: patch failed: core/modules/system/tests/src/Functional/Entity/EntityAddUITest.php:72
error: core/modules/system/tests/src/Functional/Entity/EntityAddUITest.php: patch does not apply

Tried testing this without the patch by going to node/add
I updated the labels multiple times to change the order and each time the order updates correctly.

Not entirely clear what this could be fixing? Could someone elaborate please.

Nothing the change actually on admin/structure/types screen.

+++ b/core/lib/Drupal/Core/Entity/Controller/EntityController.php
@@ -340,22 +340,29 @@ protected function doGetEntity(RouteMatchInterface $route_match, EntityInterface
    */
   protected function loadBundleDescriptions(array $bundles, EntityTypeInterface $bundle_entity_type) {
-    if (!$bundle_entity_type->entityClassImplements(EntityDescriptionInterface::class)) {
-      return $bundles;
-    }
     $bundle_names = array_keys($bundles);

I don't think the early return should have been changed here. If for some reason we need to sort in this case to, it might need to happen separately.

This will be required for #3346394: Replace BlockContentController::add with EntityController::addPage via the AdminHtmlRouteProvider route provider

Ran the test-only job here https://git.drupalcode.org/issue/drupal-3196798/-/jobs/5609439 which gave

1) Drupal\Tests\system\Functional\Entity\EntityAddUITest::testAddPageWithBundleEntities
Behat\Mink\Exception\ResponseTextException: The pattern /Aaa Test2 label(.*)Test label/ was not found anywhere in the text of the current page.
/builds/issue/drupal-3196798/vendor/behat/mink/src/WebAssert.php:907
/builds/issue/drupal-3196798/vendor/behat/mink/src/WebAssert.php:329
/builds/issue/drupal-3196798/core/modules/system/tests/src/Functional/Entity/EntityAddUITest.php:79
FAILURES!

Awesome use of extending an existing test!

Fix to core/lib/Drupal/Core/Entity/Controller/EntityController.php seems pretty straight forward with the uasort. No objections

LGTM

Committed to 11.x and backported to 11.2.x as there is minimal risk of disruption here.

Thanks folks