Problem/Motivation

Using todays 8.x dev, I added an Entity Reference field, and on the relevant Field Settings form is Type of item to reference* select box. The options within that select box do not have consistent character casing, as seen in the attacked image.

For example, there is Custom Block and Custom block type.
The only other similar problem was Unbound Display. There could be more, but not for the entities I had available (pretty much a default standard profile installation).

Im guessing its not Entity References problem, but each entities implementation of a hook? Anyway, this is as good of a place as any to start a fix.

Steps to reproduce

Proposed resolution

Use sentence case for entity labels. See Page titles, links, and headings

Remaining tasks

Review
Commit

User interface changes

Yes.

Before

After

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#47 interdiff-47.txt612 bytesxjm
#47 entity-label-1987504-47.patch3.68 KBxjm
#46 interdiff-entity-labels.txt1.61 KBxjm
#46 entity-label-1987504-46.patch3.68 KBxjm
#43 1987504-34.patch3.54 KBquietone
#37 After.png31.02 KBquietone
#34 interdiff-1987504-31-34.txt800 bytesmohit_aghera
#34 1987504-34.patch3.54 KBmohit_aghera
#31 interdiff-1987504-27-31.txt4.68 KBmohit_aghera
#31 1987504-31.patch3.56 KBmohit_aghera
#26 interdiff-1987504-25-27.txt786 bytesmohit_aghera
#26 test-only-1987504-27.patch1.74 KBmohit_aghera
#26 1987504-27.patch2.36 KBmohit_aghera
#25 interdiff-1987504-21-25.txt1.73 KBmohit_aghera
#25 test-only-1987504-25.patch1.73 KBmohit_aghera
#25 1987504-25.patch2.36 KBmohit_aghera
#21 1987504-21.patch636 bytesquietone
#21 Reference_1.png29.79 KBquietone
#21 Reference_0.png29.75 KBquietone
#7 entity_module-inconsistent-character-1987504-7.patch778 bytesmandarmbhagwat78
entity-ref.jpg29.49 KBneRok
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rszrama’s picture

I'd add that the Entity Reference field name itself is also inconsistently cased when compared with other multi-word field names. I began a patch to update all instances of "Entity Reference" to "Entity reference" with exceptions for when the string referenced the module name, but then it's not clear in some parts of the module's Views integration where it might be doing so. Custom Block may be a little more straightforward.

amateescu’s picture

Can you please post the patch so everyone can chime in about the parts where you're in doubt?

rszrama’s picture

Oh, I started but then trashed it when I realized I was in over my head. ; )

mandarmbhagwat78’s picture

There are two ways to address this issue:
1. Adding a class to the $form element in entity_reference_field_settings_form() defined in entity_reference.module

  $form['target_type'] = array(
    '#type' => 'select',
    '#title' => t('Type of item to reference'),
    '#options' => $entity_type_options,
    '#default_value' => $field['settings']['target_type'],
    '#required' => TRUE,
    '#disabled' => $field->hasData(),
    '#size' => 1,
<strong>    '#attributes' => array(
      'class' => array('target_type_capitalize'),
    ),
</strong>  );

add below css change into style.css
.target_type_capitalize{text-transform:capitalize;}

2. Using ucwords(strtolower($entity_info['label']))
entity_reference.module line #143

      $entity_type_options[$entity_type] =ucwords(strtolower($entity_info['label']));

@neRok let me know if either of above solution help

neRok’s picture

@mandarmbhagwat78, such a 'fix' is only fixing the problem for this select box. I think it should be fixed 'deeper', ie whichever module is creating the entity and providing the name, should do so consistently (at least with core modules). This way the name is consistent across all areas of Drupal.

Or, whichever core module is implementing the hook to 'call' the entities should modify the received name to ensure it is consistent.

I think ucfirst is the correct function (most names are ucfirst style , only some are ucword style)

mandarmbhagwat78’s picture

Assigned: Unassigned » mandarmbhagwat78

@neRok yes you are right. ucfirst() option is the correct solution. If user want to have ucword() style(that will be considered as project specific requirement), he/she can change it by applying additional class using hook_form_alter() as mentioned in my first option. I will test this out and commit the patch.

mandarmbhagwat78’s picture

rszrama’s picture

Yeah, I don't think ucfirst() is worth pursuing here. Really, these names should be fixed in the various plugins that are using Camel Casing instead of Sentence capitalization. Why cover up the problem when you can fix it? : )

amateescu’s picture

Status: Active » Closed (works as designed)

That's right, the label property from entity info annotations have to be fixed, not Entity reference.

neRok’s picture

Were new issues opened to pursue each label fix?

mandarmbhagwat78’s picture

@amateescu, I think there should be uniformity at the core while rendering the content.

amateescu’s picture

Title: Inconsistent character casing for Entity References. » Inconsistent character casing for entity type labels
Component: entity_reference.module » entity system
Status: Closed (works as designed) » Active

Ok, then lets keep this issue to fix them.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

Version: 8.9.x-dev » 9.3.x-dev
Assigned: mandarmbhagwat78 » Unassigned
Issue summary: View changes
Status: Active » Needs review
Issue tags: +Bug Smash Initiative
FileSize
29.75 KB
29.79 KB
636 bytes

I tested this on Drupal 9.3.x, standard install. Only one of the labels is not sentence case, 'Text Editor'. That was introduced in 2204129#2 and I didn't find anything in that issue questioning the case.

The attach patch changes the label and and label_collection to sentence case.

Spokje’s picture

Status: Needs review » Reviewed & tested by the community

- Labels changed now match other cAsInGs

If TestBot returns green, RTBC for me.

Spokje’s picture

Retesting (the already green) patch to make it the default for 2-daily retesting.

larowlan’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Thanks for the work here folks, but I think we need a tests here to assert the case so that we prevent new instances creeping in

mohit_aghera’s picture

Trying to add a test case to ensure that titles are in proper casing.
Test only patch is failing on local.

Also, this seems bit hacky testing approach to me so I would need some suggestions whether it is fine or not.

mohit_aghera’s picture

Status: Needs review » Needs work

The last submitted patch, 26: test-only-1987504-27.patch, failed testing. View results

mohit_aghera’s picture

Status: Needs work » Needs review
larowlan’s picture

+++ b/core/modules/field_ui/tests/src/Functional/ManageFieldsTest.php
@@ -63,4 +66,32 @@ public function testFieldDropButtonOperations() {
+    foreach ($options as $label) {
+      // Keep the first word as it is. Reason is we have special cases like
+      // RDF, URL alias etc. where we can't run strtolower() for entire string.

We're testing the UI here, but equally could we just create a kernel test that called ->getDefinitions on the entity-type manager and looped over checking the case?

mohit_aghera’s picture

Assigned: Unassigned » mohit_aghera
Status: Needs review » Needs work
mohit_aghera’s picture

Assigned: mohit_aghera » Unassigned
Status: Needs work » Needs review
FileSize
3.56 KB
4.68 KB

- Updating test case to use Kernel tests.
- Refactor test case to use Annotations Discovery approach and validate strings.
- Fix one more occurrence found while running the test cases on local

Status: Needs review » Needs work

The last submitted patch, 31: 1987504-31.patch, failed testing. View results

larowlan’s picture

- Fix one more occurrence found while running the test cases on local

💪 tests++

+++ b/core/modules/system/tests/src/Kernel/Entity/EntityLabelTest.php
@@ -0,0 +1,51 @@
+    $base_directory = $this->root . '/core/modules/';
+    $modules = scandir($base_directory);
+    $paths = [];
+    foreach ($modules as $module) {
+      $paths["\Drupal\\{$module}\Entity"] = $base_directory . $module . '/src/';
+    }
+    $namespaces = new \ArrayObject($paths);
+    $discovery = new AnnotatedClassDiscovery('Entity', $namespaces, 'Drupal\Core\Entity\Annotation\EntityType');

this is neat, nice work

+++ b/core/modules/system/tests/src/Kernel/Entity/EntityLabelTest.php
@@ -0,0 +1,51 @@
+class EntityLabelTest extends DiscoveryTestBase {

Any reason to extend from DiscoveryTestBase, it has a test method in it that expects $this->discovery to be set (And is why it's failing)

mohit_aghera’s picture

Status: Needs work » Needs review
FileSize
3.54 KB
800 bytes

Thanks, @larowlan.
Updating test case to use KernelTests.

Berdir’s picture

+++ b/core/modules/system/tests/src/Kernel/Entity/EntityLabelTest.php
@@ -0,0 +1,51 @@
+      // Keep the first word as it is. Reason is we have special cases like
+      // RDF, URL alias etc. where we can't run strtolower() for entire string.
+      $first_word = strtok($label_string, " ");
+      $remaining_string = strtolower(strstr($label_string, " "));
+      $this->assertEquals($first_word . $remaining_string, $label_string);
+
+      $first_word = strtok($collection_label_string, " ");
+      $remaining_string = strtolower(strstr($collection_label_string, " "));
+      $this->assertEquals($first_word . $remaining_string, $collection_label_string);

Is this really something that we should enforce/test generically?

For modules, we have the opposite rule, we explicitly uppercase each word there. It's "Automated Cron". and "Configuration Translation".

Not a native speaker, honest question: What's the reason why these _must_ be lowercased? Not saying that the examples that are updated here are wrong, just about whether we should really enforce it like this?

I suppose it's just core and we could always add an exception to the rule, still, but kinda surprised that we go with tests here :)

mohit_aghera’s picture

Based on the screenshots mentioned in #21, it seems that entity labels are following specific casing structure. I think that's why we've thought of changing labels.

Still, @quietone or @larowlan can confirm once.

quietone’s picture

Issue summary: View changes
FileSize
31.02 KB

I searched the User interface standards for direction and found a section on Capitalization. The closest fit for this is the paragraph on Page titles, links, and headings, which says to use sentence case, which this patch is doing.

As berdir said in #35 modules use a different casing. The Module and theme names section does state that they are to use title case capitalization.

I think the change here is fine.

Berdir’s picture

To be clear, I was just surprised about the test requirement for this, I'm not aware that we do the same for modules for example. Or do we?

Just wasn't sure if this rule will really hold up all the time, but as said, it's just testing core and if we want to we could add an exception.

larowlan’s picture

I asked for a test because I didn't want to do this piecemeal, and for it to slip with any new entity types

The test caught one we missed, so that feels like it was worthwhile

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

borisson_’s picture

quietone’s picture

This is not testing the patch. I am going to re-upload it.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 43: 1987504-34.patch, failed testing. View results

borisson_’s picture

Status: Needs work » Reviewed & tested by the community

that looks like a random failure, back to rtbc.

xjm’s picture

This needs to target 10.1.x now since it includes string changes.

+++ b/core/modules/system/tests/src/Kernel/Entity/EntityLabelTest.php
@@ -0,0 +1,51 @@
+      // Keep the first word as it is. Reason is we have special cases like
+      // RDF, URL alias etc. where we can't run strtolower() for entire string.
+      $first_word = strtok($label_string, " ");

What if the entity type label is something like "Reference URL"?

I'm okay with adding the test (as mentioned, it caught an instance we missed), but there's a risk that we might have to add special cases to it in the future for specific core entity types.

There were some grammatical issues with the code comments, so I fixed those as well as adding a comment about the above. I think it'd still be okay for me to commit this since those are trivial changes.

xjm’s picture

Good thing I didn't just change that on commit. ;)

xjm’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 10.1.x. Thanks!

  • xjm committed a55bbc9 on 10.1.x
    Issue #1987504 by mohit_aghera, quietone, xjm, mandarmbhagwat78, neRok,...

Status: Fixed » Closed (fixed)

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