Problem/Motivation

If overriding a content type form, you use :
hook_form_node_type_form_alter()
because the formId is node_type_form
while to change a comment type form you use:
hook_form_comment_type_edit_form_alter()
because the formId is comment_type_edit_form
From a DX percpetive, it seems unconsistant.

Proposed resolution

The proposition is to change comment_type_edit_form to comment_type_form to match with what is done for content types.

Remaining tasks

- evaluate
- patch
- review

User interface changes

None

API changes

This will not change the API itself but would impact any modules using hook_form_comment_type_edit_form_alter(), for instance Simplify module.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it is a code refactoring to make it DX friendly.
Issue priority Normal because this is somehow like improving a class naming.
Prioritized changes The main goal of this issue is DX usability which impacts contributed module.
Disruption Disruptive for core/contributed modules because it will require an upgrade of calls from
hook_form_comment_type_edit_form_alter()
to
hook_form_comment_type_form_alter()
CommentFileSizeAuthor
#3 2501663-3.patch1.01 KBandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dom.’s picture

Actually both node_type_edit_form or node_type_form and comment_type_edit_form or comment_type_form can be used, which is even more confusing as a DX.
I would expect
hook_form_FORM_ID_alter() works with FORM_ID being the actual form_id of the form such as what you receive from
hook_form_alter(&$form, &$form_state, $form_id)

andypost’s picture

Issue tags: +Configurables

Looking at \Drupal\Core\Entity\EntityForm::getFormId() this issue is wrong because entity form depends on operation.
I think we need to normalize all routes that uses default operation or remove the default form for all config entities

andypost’s picture

Status: Active » Needs review
FileSize
1.01 KB

A sort of the patch should be done for all config entities

Status: Needs review » Needs work

The last submitted patch, 3: 2501663-3.patch, failed testing.

Anonymous’s picture

Assigned: Unassigned »
Anonymous’s picture

Assigned: » Unassigned

Status: Needs work » Needs review

bendev queued 3: 2501663-3.patch for re-testing.

bendev’s picture

asked a retest because works locally

Status: Needs review » Needs work

The last submitted patch, 3: 2501663-3.patch, failed testing.

bendev’s picture

could not find any existing call to _form_comment_type_edit_form_alter

Pol’s picture

Status: Needs work » Closed (works as designed)

After a discussion with Tim Plunkett at DrupalCon Barcelona and regarding comment #1, this works as defined, so, there's no bug or problem here.

Reopen if needed, thanks.