Ajax callbacks on paginated forms with PagerSelectExtender not updating on first callback

Created on 21 September 2023, about 1 year ago
Updated 14 March 2024, 9 months ago

Problem/Motivation

If you create a form with a pager, and a table with rows that have Ajax callbacks that alter the form render array, unexpected behavior happens. The issue concerns the behavior of Drupal when using Ajax callbacks in combination with paginated results generated using the Drupal\Core\Database\Query\PagerSelectExtender. Specifically, the problem gets reproduced when an Ajax callback is triggered on the second page of a paginated table. In such cases, the callback may not update the content correctly for the initially clicked row; but, subsequent clicks on the same non-first page work as expected (after the initial Ajax callback).

Steps to reproduce

First, you need to build:

  • Create a query that uses the Drupal\Core\Database\Query\PagerSelectExtender to get paginated results.
  • Use those results to build a table in a Drupal form.
  • For each row in the table, add a callback to alter the table render array rows (the results returned by the query).
  • Add a pager render element in the form.

To reproduce the issue:

  • Go to the form route, and go to any page but the first one.
  • Click on the callback for any row.
  • The row item it should've been altered, won't be altered.
  • Click on another callback.
  • The row will be altered.

You can check this snippet as to how it can be triggered.

You can check the first comment to get a more in-depth explanation of the issue and why it happens.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Active

Version

11.0 🔥

Component
Ajax 

Last updated about 10 hours ago

Created by

🇨🇷Costa Rica robert-arias

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Issue created by @robert-arias
  • 🇨🇷Costa Rica robert-arias

    If you build a table from the results from a query that extends the Drupal\Core\Database\Query\PagerSelectExtender to enable pagination, and add a callback to each element of the results, and that callback dynamically updates the form (i.e., the render array), the alter won’t work. This occurs in any page of the paginated form, but the unintended consequences (meaning, the callback not doing what it's supposed to do), can be easily seen in any non-first page. Let me explain.

    To easily illustrate the issue, I'm going to use an scenario. Let’s say you build a form that show a list of students, and that form will show a paginated (five students per page) table with the students. The table will have two columns: Id, Full Name, and Actions.
    In the “Full Name” column, the student’s full name will be displayed, and in the “Actions” column, a button named “Show Nickname” can be used. When the “Show Nickname” button is clicked, the “Full Name” column will be updated to show the student’s nickname next to the full name, like so:

    • John Doe (Jack)

    The button is a simple render element with an Ajax attribute that calls a custom callback on click. The callback just updates the form render array, specifically the “Full Name” column value, to append the nickname.

    This works fine on the first page (not completely, as the problem can't be easily seen; I'll explain later): when you click the callback button, it appends the nickname into the row "Full Name" column; but it doesn’t work on the next pages, at least not on the first clicked row you want to display the nickname. The other subsequent clicks (on the same n > 1 page) will work. The first one won’t work and the modification won't happen.

    To understand why this happens, we need to understand how the Drupal\Core\Database\Query\PagerSelectExtender works, and how the values are returned.

    You usually use the PagerSelectExtender along with a pager render element, to enable pagination and to render a way to move through the pages (i.e., the pager). You usually just need:

    $form['pager'] = [
      '#type' => 'pager',
    ];
    

    To make the pager work with your results, PagerSelectExtender creates a Drupal\Core\Pager\Pager when the query gets executed. But, how does this works and how does the pager picks up the correct generated pager (with the available pages) and results to show?

    Well, when using the PagerSelectExtender, this class overrides the Drupal\Core\Database\Query\SelectExtender “execute” function, to paginate the original query.

    Among the things we care, inside the PagerSelectExtender::execute, it first ensures the PagerSelectExtender (in PagerSelectExtender::ensureElement) element (which is the ID to be used for the pager). If no pager has been generated before this one, it’ll use the initial value of 0 as the element ID. The PagerSelectExtender gets the available pager element ID (which it's actually the max pager element ID) from the Drupal\Core\Pager\PagerManager (which it’s this service that handles the available pagers within the render context). Once we have the element reserved, we go ahead and create the pager (paginating the original query) through the PagerManager::createPager.

    This is where the important stuff happens. In here, we first get the current page from the current request in Drupal\Core\Pager\PagerParameters::findPage. This function takes the pager element ID to check which page we’re requesting for the current pager we’re building (which, as of now, it’s 0). We get/calculate the page we're trying to build from the request.

    Before going further, it’s important to understand how Drupal knows which pager page we’re trying to build from the request (you can have multiple pagers in the same page, so Drupal needs to know, if a new page has been requested, which pager values to update). This is all done in Drupal\Core\Pager\PagerParameters::findPage.

    This function takes the pager element, and calls the PagerParameters::getPagerQuery, which calls PagerParameters::getPagerParameter.
    getPagerParameter takes the ‘page’ query parameter from the request (the page parameter value can be just one value (such as ?page=1) or just one value separated by commas (?page=,,,4 or ?page=,1,,5)).
    Then, the getPagerQuery will explode the string by the commas to return an array.
    Then, findPage will return the correct page to paginate the results.

    How? Well, the pager element ID will always be an integer, and for Drupal to know which page we’re requesting for an specific pager, the page query parameter is divided by commas, then converted into an array to represent the pager element ID as the index within the array.

    So, for example, if we have a pager with the element ID of 7, and we request the second page, the URL will look like this: ?page=,,,,,,,1. The page value (1) is represented on the 8th spot (because we’re using the logic where the first place is in the 0th spot) and when findPage looks for the value in the array, by its index, the correct page will be returned for the pager. In this case, the 7th index in the array, which returns 1.

    After getting the current page, we go ahead and create the pager, then we set the pager in PagerManager::setPager. This function, sets the PagerManager->maxPagerElementId (so, when another pager gets built, return the current max element ID, so the extender knows to use the maxPagerElementId + 1), and add the pager we just created in the ‘pagers’ array, associated by the element ID.

    Once we’ve built the pager, going back to PagerSelectExtender::execute, there’s the following line:

    $this->range($pager->getCurrentPage() * $this->limit, $this->limit);
    

    To set the range of the query, we use the pager current page. Then we carry on and execute the query.

    After that, the pager we created is built by web/core/includes/theme.inc in template_preprocess_pager (check the code to see how it works) and the pager is retrieved by its element ID (which, in the form render array, defaults to 0 if not set).

    All looks fine, the problem arises when the form is built in Drupal\Core\Form\FormBuilder. The form gets built when: 1) processing the form for the first time; and 2) when there are Ajax callbacks that trigger the form.

    In FormBuilder::buildForm is where the form is built. In here, the code first checks if the form hasn’t being built yet, if it hasn’t, we’re hitting the form for the first time and we need to build it from scratch. In this step, our form gets built, and that’s what’s gonna happen when you first visit the list of students or when you navigate though the pager (when you go to another page, it’s as the form hasn’t being built before, so Drupal has to build it).

    After building the form, we process the form itself to return a response. In FormBuider::processForm, there’s this check:

    if ($form_state->isProcessingInput())
    

    At first, when you are just looking into the list (haven’t clicked any button in the list), this will be false, but when you click on the button with the Ajax callback, this will be true.

    When $form_state->isProcessingInput() is true, there’s another check that will trigger another rebuild, this one:

          // If $form_state->isRebuilding() has been set and input has been
          // processed without validation errors, we are in a multi-step workflow
          // that is not yet complete. A new $form needs to be constructed based on
          // the changes made to $form_state during this request. Normally, a submit
          // handler sets $form_state->isRebuilding() if a fully executed form
          // requires another step. However, for forms that have not been fully
          // executed (e.g., Ajax submissions triggered by non-buttons), there is no
          // submit handler to set $form_state->isRebuilding(). It would not make
          // sense to redisplay the identical form without an error for the user to
          // correct, so we also rebuild error-free non-executed forms, regardless
          // of $form_state->isRebuilding().
          // @todo Simplify this logic; considering Ajax and non-HTML front-ends,
          //   along with element-level #submit properties, it makes no sense to
          //   have divergent form execution based on whether the triggering element
          //   has #executes_submit_callback set to TRUE.
          if (($form_state->isRebuilding() || !$form_state->isExecuted()) && !FormState::hasAnyErrors()) {
            // Form building functions (e.g., self::handleInputElement()) may use
            // $form_state->isRebuilding() to determine if they are running in the
            // context of a rebuild, so ensure it is set.
            $form_state->setRebuild();
            $form = $this->rebuildForm($form_id, $form_state, $form);
          }
    

    See the comment above the code to understand the reason for the rebuild.

    So, we know the code won’t reach here on the first form render, but when there’s a triggering element, in this case the Ajax call, for the very first time (i.e., this is the first callback button clicked from the list) the following happens:

    1. It’ll check whether the form has being built before. For this context (the Ajax callback), it won’t find any built form, so we build the form.
      1. When building the form, all our logic will run, along with executing the query that uses the PagerSelectExtender (which will create a pager with element ID of 0).
    2. Then, it’ll process the form to return a response.
      1. In here, the $form_state->isProcessingInput() will be true because we’re rebuilding the form from a triggering element.
      2. Then, we’re gonna get to the point of the workflow where the form gets rebuilt again.
      3. Because we’re rebuilding the form, all the logic will be executed again.
      4. When the form gets rebuilt again, there’s gonna be a pager mismatch, and due to this mismatch, when we try to update the form render array, the row (from the values yielded by our query) won’t exist, so when we try to alter the row, this won't exist and whatever we add, won't be appended.

    The mismatch happens on the step 2.3, when we rebuild the form again. If you recall from the explanation, when using the PagerSelectExtender, when the query is executed, a dynamic pager is created used to paginate the results. But remember that this is the second time rebuilding the same form in the same render context, meaning that the PagerManager already holds a pager, the one created on step 1.1.

    Because this is the second time the form is built, and a pager was already built before (with element ID of 0), when PagerSelectExtender::ensureElement runs again, it’ll set its element ID to 1. This means, a new pager is going to be generated. The problem arises when we try to get the page we currently are.

    Assume we’re on the second page (?page=1), when PagerManager::createPager runs; the PagerManager::findPage will look for a value in the index 1 (the new pager element ID, the one from the second build). Because there won’t be any pager with the element ID 1, nothing will be found (the only index available is that of 0, the original pager, the one that was created first; that one does hold the actual page), and the value the PagerManager::findPage will return is 0 (first page of the pager; that’s the fallback). Because of this, when the second rebuild gets executed, and we try to update the render array, the table list values (the ones returned by the query) will be different, because the values that the query returned were from the first page, not the second page, and we’re trying to alter the value of a student from the second page. Because of that, the value will be empty.

    If we go ahead and press again another button from the student list to append the nickname, this issue won’t happen, since step 1 will be skipped, because the form is found to be already built, so just one rebuilt will be triggered, creating just one pager, making the pager creation and matching run smoothly. It means the subsequent callbacks will work because the form is getting built just once.

    The same problem occurs on the first page of the form, but in this case it’ll work (i.e., in this example case, it’ll append the nickname) on the first trigger because both pager will return the results of the first page (both pager will return 0 for the current page).

    A potential solution to this issue involves adding an element to the query being extended, and this element ID should be included in the pager render element as well. However, this approach might make the pager structure a little more stiff.

    The broader question raised by this issue is whether this behavior is intended or an unintended consequence of Drupal's form handling. If it is unintended, it may necessitate updates to the documentation or code adjustments to ensure consistent behavior in such scenarios.

  • 🇮🇳India sukr_s

    The issue is that the form_build_id is not cached when a new page is loaded or refreshed. This results in the buildForm being called twice. I believe due to this the pager increases the element count (assumption is that there are multiple pagers in the same page) and results in the page being reset to 0. I'm not sure why the cache is not set on a new form build.

    However if you are still facing the issue, you can resolve it with the following.

    In your showTokenValue function replace

    $form['table_container']['table'][$token_id]['token_value']['#value'] = $token->getToken();
    

    with the full definition of the token_value array i.e.

    $form['table_container']['table'][$token_id]['token_value'] =  [
            '#type' => 'html_tag',
            '#tag' => 'p',
            '#value' => $token->getToken(),
            '#attributes' => [
              'id' => ["protected-token-{$token_id}-value"],
            ],
          ];
    

    Hope this helps

Production build 0.71.5 2024