- 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 aDrupal\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 theDrupal\Core\Database\Query\SelectExtender
“execute” function, to paginate the original query.Among the things we care, inside the
PagerSelectExtender::execute
, it first ensures thePagerSelectExtender
(inPagerSelectExtender::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. ThePagerSelectExtender
gets the available pager element ID (which it's actually the max pager element ID) from theDrupal\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 thePagerManager::createPager
.This is where the important stuff happens. In here, we first get the current page from the current request in
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 inDrupal\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.Drupal\Core\Pager\PagerParameters::findPage
.This function takes the pager element, and calls the
PagerParameters::getPagerQuery
, which callsPagerParameters::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, thegetPagerQuery
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 whenfindPage
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 thePagerManager->maxPagerElementId
(so, when another pager gets built, return the current max element ID, so the extender knows to use themaxPagerElementId
+ 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
intemplate_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:
-
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.
- 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).
- When building the form, all our logic will run, along with executing the query that uses the
-
Then, it’ll process the form to return a response.
- In here, the
$form_state->isProcessingInput()
will be true because we’re rebuilding the form from a triggering element. - Then, we’re gonna get to the point of the workflow where the form gets rebuilt again.
- Because we’re rebuilding the form, all the logic will be executed again.
- 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.
- In here, the
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 thePagerManager
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
), whenPagerManager::createPager
runs; thePagerManager::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 thePagerManager::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