Problem/Motivation

In #3346394: Replace BlockContentController::add with EntityController::addPage via the AdminHtmlRouteProvider route provider we are moving block content to use generic entity stuff for routing, including the add page

Block content has custom code to sort its bundles by label in the current controller for the add page BlockContentController::addPage

#3196798: Fix EntityController::addPage so bundles are sorted by label and not ID will add this sorting to the generic EntityController::addPage but only for config entity types that implement EntityDescriptionInterface, plus this interface is required to add the description as well.

Steps to reproduce

N/A

Proposed resolution

Make BlockContentTypeInterface extend EntityDescriptionInterface
Remove BlockContentTypeInterface::getDescription
Implement setDescription

Remaining tasks

Do it

API changes

BlockContentTypeInterface extends EntityDescriptionInterface - I don't think this warrants a CR.

Issue fork drupal-3530654

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

acbramley created an issue. See original summary.

acbramley opened merge request !12396

acbramley’s picture

acbramley commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Assume the test-only changes should of failed?

ironnuts’s picture

ironnuts commented

Would a simple unit test provide test coverage? To mock the setDescription interface..

ironnuts’s picture

ironnuts commented

Added skeleton unit test (needs work).

acbramley’s picture

acbramley commented
Status: Needs work » Needs review

Re #4 - no since this wasn't a bug fix. I've added test coverage for setDescription now though which would fail since setDescription didn't exist before.

@oily I'm not sure what the unit test was but I've removed it, we can just use a kernel test.

ironnuts’s picture

ironnuts commented

@acbramley Okay with the test. Perhaps pedantically I thought implementing a new interface would mean a unit test. I support a kernel test if ticks the box. The unit test was incomplete. Not fluent on unit tests. Would have mocked the new interface if someone else hadnt..

danielveza’s picture

danielveza
he/him
Location Brisbane, AU
commented
Status: Needs review » Reviewed & tested by the community

Changes look good, match the IS and have test coverage. I think this is ready for RTBC.

acbramley’s picture

acbramley commented

Adding credit.

larowlan made their first commit to this issue’s fork.

larowlan credited joachim.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Committed to 11.x, nice one

  • larowlan committed 2693f0e4 on 11.x 
    Issue #3530654 by oily, acbramley, larowlan, danielveza, joachim:...

larowlan closed merge request !12396

Status: Fixed » Closed (fixed)

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