Make WordPress Core

Opened 2 months ago

Last modified 6 days ago

#62430 reopened defect (bug)

Bulk Action: Cannot submit bulk action if no items are selected

Reported by: jcutler's profile jcutler Owned by: joedolson's profile joedolson
Milestone: 6.7.2 Priority: normal
Severity: normal Version: 6.7
Component: Administration Keywords: close
Focuses: Cc:

Description

There was a change made that does not allow bulk actions in wp_list_table to be processed if no items are selected. I have a plugin with many operations that do not require any items to be selected. This change breaks those operations.

This appears to be the commit:

https://github.com/WordPress/WordPress/commit/0a4679908b52e7274b942b2a0792b6e2ac28d99f

Repro steps:
Install plugin that allows bulk actions to be performed without selecting any items.
In the admin area that uses the plugin(via wp_list_table), view the table.
Select the bulk action that does not require item selection (i.e. "download-all").
Click Apply

Expected Results:
Action is executed.

Actual results:
Admin notice displayed: "Please select at least one item to perform this action on."

I have verified that WordPress 6.7 introduces this breaking change.

I cannot update to WP 6.7 on my client's live site, so I have marked severity as blocker.

Please advise as this change breaks my plugin.

Change History (14)

#1 @joedolson
2 months ago

  • Owner set to joedolson
  • Status changed from new to accepted

This ticket was mentioned in Slack in #core by desrosj. View the logs.


2 months ago

#3 @desrosj
2 months ago

  • Milestone changed from Awaiting Review to 6.7.1

Introduced in [59134]/#45006.

#4 @joedolson
2 months ago

Can you provide an example of a plugin that triggers this issue, or otherwise provide a code example to generate the scenario?

I don't think that it makes sense to revert this, because for the majority of use cases there should be a notification blocking events if there is nothing selected. However, it should be relatively easy to provide a path so that plugins can selectively override the situation.

#5 @jcutler
2 months ago

Here's a fairly straight-forward example, excluding most of the required WP_List_Table implementation.

<?php
class My_List_Table extends WP_List_Table {
    ...
        
    public function get_bulk_actions() {
        $actions = [
            'download-selected' => 'Download Selected',
            'download-all' => 'Download All',
        ];            
        return $actions;
    }
    ...
    public function get_ui() {
?>
<div class="wrap">
    <h1 class="wp-heading-inline">Registrations</h1>
    <hr class="wp-header-end">
    <form id="registrations" method="post">
        <?php echo $this->views(); ?>
        <?php wp_nonce_field('manage_registrations', 'registration_nonce'); ?>
        <div id="post-body-content">
            <div class="table">
                <?php
                $this->prepare_items();
                $this->display();
                 ?>
            </div>
        </div>
    </form>
</div>
}
?>
<script>
document.addEventListener('DOMContentLoaded', function () {
    const form = document.querySelector('form');
    const bulk_actions_input = form.querySelector('input[name="bulk-actions"]');

    form.addEventListener('submit', function (event) {
        bulk_actions_submit(event);
    });
});

function bulk_actions_submit(event) {
    var action = $('#bulk-action-selector-top').val();
    var selected_item_count = $('input[name="bulk-action[]"]:checked').length;
    if( "-1" === action ){
        alert('Please select an action.');
        event.preventDefault();
        return false;
    }
    if(selected_item_count <= 0 && action !== 'download-all'){
        alert('Please select one or more items.');
        event.preventDefault();
        return false;
    }
    //allow form submit
}
</script>

Much of the above code is ChatGPT generated rather than try to simplify my plug-in code.

I would expect there are other developers that have implemented functionality similar to this in their plug-ins. Mine is a custom client plug-in that I can't share properly, but this should give you the idea of the workflow.

As a note on the importance of this, if I had not been actively working on a new feature and simply upgraded to version 6.7 I would not have known 6.7 broke my implementation until the customer reported this; client-side functionality is generally not a test I would run for a WordPress update. I have six different pages with this behavior, and having to modify and test those areas with what I consider essentially a breaking API change is not ideal. I would hope you will take into consideration how this may impact other developers as well. The only reasonable solution in my opinion would be to make the new functionality configurable, with the default behavior being disabled, plus a mechanism to override. I probably would have been pleased if this were part of the list table functionality when I first began working with this.

Last edited 2 months ago by jcutler (previous) (diff)

This ticket was mentioned in Slack in #core by desrosj. View the logs.


8 weeks ago

#7 @joedolson
8 weeks ago

I'm seeing two reasonable paths forward on this:

1) We can add a flag such as data-allow-empty that can be added to an option and will prevent the error flag.

2) You can add a behavior that auto selects the available checkboxes when your option is selected.

In either case, you'd need to make a change to your plugin; I'm inclined to think it makes more sense for you to select items when your option is selected. Your action is presumably already bypassing the selection, so whether all items are selected or not is relatively moot.

And this is really not an API change - the API never performed any action when no posts were selected; this is simply making it apparent that no action is being performed.

#8 @jcutler
8 weeks ago

Adding an option will likely be an adequate solution, but not ideal.

What I'm about to say is something you're probably not going to like, and I'll probably sound like a jackass.

Not performing an action is the default behavior, and therefore is the API. You have no way to quantify the impact of this change on other plugins/developers; you may feel that this is a minor inconvenience, but you have absolutely no way to know.

My implementation downloads all items from the database in that functional area, not just the ones on the screen. There may be hundreds of items to download, and I am not expecting my client to change their list to show every item, or page through all items, select all on each page, and download each page separately. Since you don't know what my application does or why your inclination for a solution isn't adequate.

When I started development on this project five years ago I was actually surprised there was no check to see if any list items were selected. I doubt I'm the only person that noticed this and put custom functionality in place.

I'm not going to try to persuade you on this any further. It sounds like you're likely to do what you want here (a good feature, just years overdue with unknown consequences), and for me it will just be a minor waste of my time to fix. And I hope that I'm wrong and there isn't a bevy of complaints when sites upgrade to 6.7 or higher.

Sorry for the mini-rant. I don't know how else to have you understand the potential impact of a change like this.

#9 @joedolson
8 weeks ago

  • Milestone changed from 6.7.1 to 6.7.2

Totally understood.

To be clear, this isn't my sole call - we discussed this in a 6.7.1 bug scrub today, and agreed that the new behavior is the correct and expected behavior. The fact that you've been taking advantage of a bug to get the interaction you wanted is an unfortunate side effect; but isn't something that we're going to revert at this point.

6.7 has been out now for a week, and as of yet your report is the only one I'm aware of. So from our perspective, this is a minor issue with a low impact.

For now, I feel like there is insufficient evidence of a problem to try and get this change into 6.7.1; I'm going to change the milestone to 6.7.2 for continuing discussion.

#10 @joedolson
8 weeks ago

  • Severity changed from blocker to normal

#11 @jcutler
8 weeks ago

I've worked in software for decades and can't tell you the number of times developers (others and even myself) fix a bug only to find out users consider it a "feature" (or vice-versa). :-)

Not all sites auto-update major versions, and certainly every plugin that might exhibit this behavior won't necessarily be accessed and have this functionality executed, so a week isn't really a great benchmark. But hopefully as you expect the impact will be limited.

Thanks for taking the time to consider this.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


4 weeks ago

#13 @jorbin
6 days ago

  • Keywords close added
  • Milestone 6.7.2 deleted
  • Resolution set to invalid
  • Status changed from accepted to closed

Based on the fact that this seems to be an edge case with a work around (code that is relying on this just needs to check one box, (the 2nd of suggestion from this comment) I think this can be closed as wontfix

#14 @jorbin
6 days ago

  • Milestone set to 6.7.2
  • Resolution invalid deleted
  • Status changed from closed to reopened

Meant to leave it open and just suggest closing for the time being.

Note: See TracTickets for help on using tickets.