In #2759671: Flag form element shows on entity delete forms the flag form element incorrectly appearing on non-edit forms was solved. However, I still see the element appearing on other forms,such as the ones provided by the Scheduled Transitions module.

It's also accompanied with the following notices/warnings:

    Notice: Undefined index: actions in flag_form_alter() (line 183 of modules/contrib/flag/flag.module).

    flag_form_alter(Array, Object, 'node_product_scheduled_transitions_entity_form_form') (Line: 539)
    Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'node_product_scheduled_transitions_entity_form_form') (Line: 838)
    Drupal\Core\Form\FormBuilder->prepareForm('node_product_scheduled_transitions_entity_form_form', Array, Object) (Line: 279)
    Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 91)
    Drupal\Core\Controller\FormController->getContentResult(Object, Object)
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
    Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
    Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
    Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
    Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
    Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

    Warning: array_keys() expects parameter 1 to be array, null given in flag_form_alter() (line 183 of modules/contrib/flag/flag.module).

    flag_form_alter(Array, Object, 'node_product_scheduled_transitions_entity_form_form') (Line: 539)
    Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'node_product_scheduled_transitions_entity_form_form') (Line: 838)
    Drupal\Core\Form\FormBuilder->prepareForm('node_product_scheduled_transitions_entity_form_form', Array, Object) (Line: 279)
    Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 91)
    Drupal\Core\Controller\FormController->getContentResult(Object, Object)
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
    Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
    Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
    Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
    Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
    Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

    Warning: Invalid argument supplied for foreach() in flag_form_alter() (line 183 of modules/contrib/flag/flag.module).

    flag_form_alter(Array, Object, 'node_product_scheduled_transitions_entity_form_form') (Line: 539)
    Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'node_product_scheduled_transitions_entity_form_form') (Line: 838)
    Drupal\Core\Form\FormBuilder->prepareForm('node_product_scheduled_transitions_entity_form_form', Array, Object) (Line: 279)
    Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 91)
    Drupal\Core\Controller\FormController->getContentResult(Object, Object)
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
    Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
    Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
    Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 47)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
    Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
    Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

After some digging it appears #2867967: isAddEditForm() only needs to be called once (re-)introduced this issue, along with the notices and warnings. The problem is in the following piece of code:

  $flags = $flag_service->getAllFlags($entity->getEntityTypeId(), $entity->bundle());

  // Check the first flag and return early if the form isn't considered to be
  // an edit form.
  if (!empty($flags) && isset($flags[0]) && $flags[0] instanceof FlagInterface) {
    if (!$flags[0]->getFlagTypePlugin()->isAddEditForm($object->getOperation())) {
      return;
    }
  }

$flags is an associative array, but the code is treating it as a numeric array.

CommentFileSizeAuthor
#2 flag-form_element_on_non_edit-forms-3169627-2.patch662 bytesrp7

Issue fork flag-3169627

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

rp7 created an issue. See original summary.

rp7’s picture

rp7 commented
Status: Active » Needs review
StatusFileSize
new flag-form_element_on_non_edit-forms-3169627-2.patch662 bytes

Patch attached correctly handles the associative $flags array & fixes the issue(s) mentioned.

kle’s picture

kle commented

In combination with scheduled_transitions.module the problem is not solved.
Route /node/{node}/scheduled-transitions throws err in Line 183.
Reason:
Line 136 if (!empty($flags) && isset($flags[0]) cannot work, because indices of $flags are keys - not numbers. So it will never return here.
My solution seems to work:

  if (!empty($flags)) {
    $first_flag = reset($flags);
    if ($first_flag instanceof FlagInterface && !$first_flag->getFlagTypePlugin()->isAddEditForm($object->getOperation())) {
      return;
    }
  }
ivnish’s picture

ivnish
he/him
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Needs reroll to MR

rp7’s picture

rp7 commented

@kle (#3)
I don't understand our comment. The code you suggested is nearly identical to what's in the patch or am I missing something?

rp7 opened merge request !139

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

deaom’s picture

deaom commented

The MR for 8.x can be merged. The code from before does assume it's a keyed array and it's not. The getAllFlags returns An array of flag entities, keyed by the entity IDs. The same issue will be present for 5.x as well.

deaom opened merge request !184

deaom’s picture

deaom commented
Version: 8.x-4.x-dev » 5.x-dev
Priority: Major » Normal
Status: Needs work » Needs review

Created a new branch for 5.x with the commit from 8.x and opened a new MR. Composer previous fails because of #3564269: Composer 2.9 blocks dependencies which have security advisories and can be ignored until fixed. Changing status to needs review, but from my POV can me merged.

ivnish closed merge request !139

ivnish’s picture

ivnish
he/him
commented
Issue tags: -Needs reroll
ivnish’s picture

ivnish
he/him
commented

@deaom how can I reproduce this?

deaom’s picture

deaom commented

If you want to reproduce the error, you need to install the mentioned scheduled transitions module and set it up. First set up the workflow for content type, like article and then under configuration for scheduled transitions select that entity type under Enabled types. Click the edit for article node or visit it and then click on the Scheduled translations tab. The error happens here, as it's not an edit form and the check fails because it's not a keyed array so it does not return.
To just checking what comes in the if, you can enable debugger and see the flags are not a numbered array.

With the MR the check is actually checked, meaning the code does not execute and there is no error and you can access the Scheduled transitions tab.

  • ivnish committed 3a355372 on 5.x authored by deaom 
    fix: #3169627 [regression] Flag form element also appearing on non-edit...
ivnish’s picture

ivnish
he/him
commented
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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