I am personally dialing down my involvement with drupal during the next year but patches are always welcome!
grota β created an issue.
we are using the patch and it makes sense and is simple, incidentally this is also required for http://drupal.org/project/commerce_order_document moving to RTBC
we are using #5, applies cleanly to 2.12.0 and is IMHO a simple enough patch, moving to RTBC
@graber I think this issue can easily be reproduced with a vbo that does not have any exposed filter. In that case there are no dom elements in the page with class vbo-view-form
(not sure why that happens) so the js in js/frontUi.js
ends up doing nothing.
@paolomainardi I added a MR with some small changes
Hi @Graber,
I saw that you added those 2 lines in 22d6240fd295ec59bb758e8279046507fbbc528b. I tested them and they fixed the issue for me.
I created this MR to add a test.
As far as I know there are no other scenarios right now where the set of selected rows is incorrect, so for me this issue can be closed.
Hi @Graber, sorry for the delay in answering. My proposed fix above had to be reverted because it actually introduced other regressions (incorrect elements on which ultimately the action is executed on) while selecting items across pages.
As soon as we'll be able to have another go at fixing the problem I'll let you know.
I think this is the way to reproduce it:
- create a view using a table style plugin.
- add some fields: at least a vbo field, configured to use batch, and a date field, used for the next point.
- configure the table style plugin to use a date field as default sorting option.
This should be enough.
The problem is in ViewsBulkOperationsActionProcessor::getPageList
.
The workaround from
3020922 β
is nullified when it calls $this->view->build();
since it calls $this->style_plugin->buildSortPost()
which in our case is in Drupal\views\Plugin\views\style\Table::buildSortPost()
which in turn ends up calling Drupal\views\Plugin\views\field\EntityField->clickSort()
which sets the order by statement based on the table's default sorting field.
I don't know what a proper fix should be, but I hope the previous description can be useful to find one.
I have put the following workaround in place based on hook_views_query_alter
:
As a note, I also tried an approach based on hook_views_pre_execute
without success (probably because the query was already built).
use Drupal\views\Plugin\views\query\Sql;
/**
* Whether the view is configured in batch mode.
*/
function xxx_vbo_view_is_vbo_configured_in_batch_mode(ViewExecutable $view) : bool {
$fieldHandlers = $view->getHandlers('field');
foreach ($fieldHandlers as $fieldHandler) {
if (($fieldHandler['plugin_id'] ?? NULL) === 'views_bulk_operations_bulk_form') {
return $fieldHandler['batch'] ?? FALSE;
}
}
return FALSE;
}
/**
* Get base field alias for a view.
*
* Algorithm copied from
* \Drupal\views_bulk_operations\Service\ViewsBulkOperationsActionProcessor::populateQueue.
*/
function xxx_get_view_base_field_alias(ViewExecutable $view) : string {
$base_field = $view->storage->get('base_field');
if (isset($view->query->fields[$base_field])) {
if (!empty($view->query->fields[$base_field]['table'])) {
$base_field_alias = $view->query->fields[$base_field]['table'] . '.' . $view->query->fields[$base_field]['alias'];
}
else {
$base_field_alias = $view->query->fields[$base_field]['alias'];
}
}
else {
$base_field_alias = $base_field;
}
return $base_field_alias;
}
/**
* Implements hook_views_query_alter().
*/
function xxx_views_query_alter(ViewExecutable $view, QueryPluginBase $query) {
// Operate only on views currently run by VBO.
if (!isset($view->views_bulk_operations_processor_built)) {
return;
}
// Operate only on VBOs that are configured in batch mode.
if (!xxx_vbo_view_is_vbo_configured_in_batch_mode($view)) {
return;
}
if (!$query instanceof Sql) {
return;
}
$base_field_alias = xxx_get_view_base_field_alias($view);
$query->orderby = [
[
'field' => $base_field_alias,
'direction' => 'ASC',
],
];
}
Thank you @polynya and @seamus_lee! I rebased MR3 and adding also the bit about the offlines scopes and a contribution from me to fill the cookie secret in a hook_update_N and hook_install.
I kept the commits unsquashed credit where credit is due.
Thanks @iamweird, I applied your patch and gave you credit in the commit!
I will release a new rc release for 2.x.
Thank you @eugene-bocharov, it's taken me a while but I finally managed to include your patch: I went with the more conservative check agains UpdateBackend, but I gave you credits in the commit.