Problem/Motivation

Warning: Undefined array key "exclude_mode" in
Drupal\views_bulk_operations\Plugin\views\field\ViewsBulkOperationsBulkForm->viewsForm()
(line 721 of modules/contrib/views_bulk_operations/src/Plugin/views/field/ViewsBulkOperationsBulkForm.php).

When rendering a View that includes the Views Bulk Operations (VBO) bulk form field, PHP throws a warning about an undefined array key exclude_mode.

Steps to reproduce

  • Create a new View:
  • Add the Views Bulk Operations: Bulk form field to the View.
  • Save and open View page.
  • Select All, then unselect all items.
  • Reload the View page.
CommentFileSizeAuthor
#8 fix-exclude_mode_toggle_in_views_bulk_operations.patch1.45 KBdkmishra
#7 3566601-undefined-array-key-exclude_mode.patch778 bytesalbert.garcia

Issue fork views_bulk_operations-3566601

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

devendra mishra created an issue. See original summary.

dkmishra’s picture

dkmishra commented
Title: Warning: Undefined array key "exclude_mode" in Drupal\views_bulk_operations\Plugin\views\field\ViewsBulkOperationsBulkForm->viewsForm() (line 721 of modules/contrib/views_bulk_operations/src/Plugin/views/field/ViewsBulkOperationsBulkForm.php). » Warning: Undefined array key "exclude_mode" in views_bulk_operations
dkmishra’s picture

dkmishra commented
Issue summary: View changes
dkmishra’s picture

dkmishra commented
Issue summary: View changes

albert.garcia made their first commit to this issue’s fork.

albert.garcia opened merge request !136

albert.garcia’s picture

albert.garcia commented
StatusFileSize
new 3566601-undefined-array-key-exclude_mode.patch778 bytes

Assuming the array key should be set somewhere or is unset somewhere where it should not... Checking for the array key should get rid of the warning while this is investigated. Attaching a patch which matches the MR I submitted.

dkmishra’s picture

dkmishra commented
StatusFileSize
new fix-exclude_mode_toggle_in_views_bulk_operations.patch1.45 KB

This patch fixes a bug with exclude_mode in the Views Bulk Operations bulk form.
The issue occurred in two places: line 721 and line 803 of
ViewsBulkOperationsBulkForm.php. Both locations are corrected in this patch.

Tested locally:
- Created a VBO view with multiple content items
- Enabled exclude mode and selected/deselected items
- Checked that the selection state is correctly inverted
- Bulk operations worked as expected

thulenb’s picture

thulenb commented

I am getting the same error.
I show 20 items on the view and get the error on line 721 20 times plus error on line 803 one time.
I double-checked if the patch is correctly applied.
I also have version 4.4.4

Does somebody have a hint what else I could do?

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Version: 4.4.4 » 4.4.x-dev
Status: Active » Needs work

@devendra mishra the patch looks pretty good, could you roll those into a new MR or on top of !136? Or @albert.garcia, maybe you could?

@grabber might knows why we are unset()ing the 'exclude_mode' key here which might be part of the problem:
https://git.drupal.org/project/views_bulk_operations/-/blob/f301718ef562...

Introduces a (maybe on purpose) a tri-state for this key.

Setting this version to the dev branch, thanks for reporting this issue. I am able to confirm this patch fixes the issue and it's quite reproducible.

@graber happy to add tests if you want, though this might not warrant any.

devendra mishra changed the visibility of the branch 3566601-exclude-mode-toggle-vbo-fix to hidden.

devendra mishra opened merge request !137

dkmishra’s picture

dkmishra commented

Thanks for the feedback, @joelpittet!
I’ve created a new MR with the changes. Please have a look when you get a chance.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Assigned: Unassigned » graber
Status: Needs work » Needs review

Great, thank you @devendra mishra!

I will assign this to @graber to have a peek at my question in #10

graber’s picture

graber commented
Status: Needs review » Needs work

I think we should add default values to the tempstore array in the set tempstore so everything ever required is set (and currently exclude_mode is added there on initialization). exclude_mode key should be a boolean in all cases so we should drop all unset() and set it to FALSE in those places. We could then drop all NULL checks.

I'll handle this.

graber’s picture

graber commented
Related issues: +#3562184: Warning: Undefined array keys

graber changed the visibility of the branch 3566601-warning-undefined-array to hidden.

graber’s picture

graber commented
Assigned: graber » Unassigned
Status: Needs work » Needs review

Please test someone, hope I haven't missed anything. This makes sure exclude_mode tempstore key is always a boolean and explicitly set to FALSE if there's nothing to exclude.

graber’s picture

graber commented

@joel, Re tests.. now we should get warnings if exclude_mode is not set. We could add a watchdog test as in Drupal LMS tests (TBH, such a method should be in Core as that's not checked in functional and JS tests by default and should be).

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

That looks more elaborate but does the change I was hoping for, so +1 from me. I'll let @devendra mishra test and RTBC to ensure it's doing what deven needs.

That LMS test is interesting, but I wonder if each test method running it's own instance of drupal built from scratch, the logs might not be super accurate at finding things. Though it would be nice if those PHP notices errored all the time in PHPunit if they don't already...

graber’s picture

graber commented

Re the tests: the problem with functional js ones is that the notices / warnings happen on a different level - in the app and not in the test runner. If any of those happen in the runner as in the case of kernel tests - it’s reported. Not 100% sure about functional (no js) tests though. Worth checking.

erutan’s picture

erutan commented

I was getting this error itermittently, after applying the patch I haven't seen it.

It also fixes an OOM error when trying to load a view that I was passing args to via the url. :)

I'd mark it as RBTC aside from the issues testing it.

erutan’s picture

erutan commented

This has been committed. Should it still be Needs Review?

graber’s picture

graber commented
Status: Needs review » Fixed

Must've missed it, thanks.

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.