Allow to change the group title label per group type

Patch attached, please check.

Almost everything is in the title.

This is where I've gone wrong. As with ECK and other custom entities, the first thing I do is remove the title field. Would it be possible to use an entity label rather than title? The title field can be removed in from various entity types, including group entities, and the field seems to be left over from Node days. Being able to select any appropriate field as the entity title would allow more options. E.g. text, list (text), email, date etc.

That said, groups may not generally require that kind of flexibility.

Re #3: Content entities definitely need a label field. You are free to hide it, though.

Patch looks good at first sight, I'd like to add it with a test included. A web test that submits the form and then retrieves the field from the field manager would do. Anyone care to tackle this?

I will take care about this.

New patch attached please check.

#4,

Re #3: Content entities definitely need a label field. You are free to hide it, though.

Config entities can also have a label field. (Sorry, a bit off topic now.)

Test looks good, thanks. Could you rename it to GroupTypeWebTest and write it as a PHPUnit test? I've been trying to avoid adding any Simpletest tests to the module as it seems like it will be unsupported soon(ish).

1. +++ b/src/Entity/Form/GroupTypeForm.php
   @@ -93,6 +102,13 @@ class GroupTypeForm extends BundleEntityFormBase {
   +    $form['title_label'] = array(
   +      '#title' => t('Title field label'),
   +      '#type' => 'textfield',
   +      '#default_value' => $fields['label']->getLabel(),
   +      '#required' => TRUE,
   +    );
   

   Drupal now uses short array syntax. See coding standards There are more places where long array syntax is being used. Please fix all of them

2. +++ b/src/Entity/Form/GroupTypeForm.php
   @@ -137,6 +157,13 @@ class GroupTypeForm extends BundleEntityFormBase {
   +    if ($title_field->getLabel() != $title_label) {
   

   Probably !== can be used here

3. +++ b/src/Tests/GroupTypeTest.php
   @@ -0,0 +1,50 @@
   +use Drupal\simpletest\WebTestBase;
   

   Simpletest is deprecated. Use Drupal\Tests\BrowserTestBase

   Change record

4. +++ b/src/Tests/GroupTypeTest.php
   @@ -0,0 +1,50 @@
   + * Ensures that group type functions work correctly.
   

   Comment does not reflect what exactly is being tested

5. +++ b/src/Tests/GroupTypeTest.php
   @@ -0,0 +1,50 @@
   +  /**
   +   * Modules to enable.
   +   *
   +   * @var array
   +   */
   

   Use {@inheritdoc}

@Taran2L thanks for review, I agree with yours remarks.
@kristiaanvandeneynde are you agree with using BrowserTestBase? We need to test form submit because we have logic in the Form save.

@ozin I think BrowserTestBase supports POST, so that should be fine yeah.

Added test and fixed remarks. Please check.

Code looks good, test needs some love regarding where to put what code. But the overall patch is something I can work with. Thanks!

The patch in #13 worked for me: boldly marking as RTBC.
Also un-assigning as last activity was "about a year ago".

@kristiaanvandeneynde, I could fix the patch to fix the test, but I'm not sure what needs to be done.

This is clean enough. No interdiff as the patch is small and I moved a lot of code around.

Awesome, thanks!