Group roles can not be edited

I came with the same idea, but this approach wont work with roles created programmatically. They can have different IDs. My idea is to set the prefix for roles in the preSave method and check if it is already there or not.
Also implement the method for prefix similar to this: https://git.drupalcode.org/project/group/-/blob/3.0.x/src/Entity/Storage/GroupRelationshipTypeStorage.php#L132

It happens, because we want to have id unique per group type and we want to have group type prefix there. That is why I propose to do it in one place on preSave and use a single method like "getRelationshipTypeId" to get a correct id for the Role.

This fixes things:

        if (!$group_role->isNew() && count($existing_pairs) === 1 && reset($existing_pairs) == $group_role->getOriginalId()) {

Re #6 constraints get called from all over the place, including (and most likely) forms. In GroupRoleForm, when validation runs, the ID is not yet complete. For all intents and purposes the group role ID is consistent outside of the role form.

Switching the code to getOriginalId() is the least impactful way of fixing this bug. A browser test would be awesome, if anyone could write one that visits the role form, saves a new role, then visits the edit form and tries to save again, that would be awesome.

Could write the test as a followup and cover the entire form in one go.

It is a quite possible, that in some cases the roles can be create programmatically and in this case user can give any id to the role.
For exmaple:

    $group_role = GroupRole::create(
      [
        'id' => 'mymymy',
        'label' => 'mymymy',
        'scope' => 'outsider',
        'global_role' => 'administrator',
        'group_type' => 'gt',
      ]
    );
    $group_role->save();

As result we get a warning when edit this role:

Warning: Undefined array key 1 in Drupal\group\Entity\Form\GroupRoleForm->form() (line 43 of modules/contrib/group/src/Entity/Form/GroupRoleForm.php).

From other point of you GroupRole is config entity, we don't create config entities on the production environment (except webforms).

This what I was talking about in the chat. That is why I wanted to handle it is on the preSave. So, we will always will have the same format of IDs for the roles, generated based on ID and group type ID.

Okay, the whole ID changing is deliberately a form only behavior. So let's keep it there. You are right that the form did not support manually created roles (via code), so let's just simplify the form:

• New roles via UI get group type prefixed
• Existing roles via UI get nothing special done to them

Should leave all existing roles alone then.

@kristiaanvandeneynde, I got an error, when edit any role create through UI:

"The machine-readable name must contain only lowercase letters, numbers, and underscores.", This what we tried to avoid by replacing ID:

    if ($group_role->id()) {
      [, $group_role_id] = explode('-', $group_role->id(), 2);
    }

But this code was removed

Yeah the patch was a quick POC, might have missed sth. The goal remains the same, though: keep the form-specific stuff limited to the form. Any code outside of the form should not be aware of it.

This just turns existing roles' ID into a value field. Doesn't show in the UI at all any more as we don't allow changing it either way. No more ID voodoo other than new roles not having to type the group type ID (and it getting saved in the submit handler). Existing roles don't get any special behavior.

Everything is fine now. I added a few tests to cover all cases, what we have here.

Thanks for the tests!