search

inspirehep / inspire-next

The INSPIRE repo.
https://inspirehep.net
GNU General Public License v3.0
59 stars 69 forks source link

Holding Pen: bulk-decision ignores check-box #2579

Closed ksachs closed 7 years ago

ksachs commented 7 years ago

Expected Behavior

bulk-decision (clicking the top 'Article Decisions') should act on records selected by the check-boxes, not on all in the current filter.

Current Behavior

The counter in the 'NN article workflows with decisions to be made' is correct, corresponding to the number of checked records. But the action is performed on all filtered records.

Steps to Reproduce (for bugs)

  1. open holdingpen listing (e.g. with 9 records)
  2. check several records, not all (e.g. 7 records) A box for 'Article Decisions' appears with the number of checked articles.
  3. click one action, it will act on all 9 records not just the selected 7.

Screenshots (if appropriate):

hp_bulk-decision_1 hp_bulk-decision_2

jacquerie commented 7 years ago

In local testing I'm finding that this is a display bug, not a more serious one: it's true that all records switch to "Waiting", but only those records whose checkbox was selected have the action performed on them. I will make a video to show what I'm seeing.

jacquerie commented 7 years ago

Ignore my previous comment: this is a serious bug.

jmartinm commented 7 years ago

Ignore my previous comment: this is the serious bug.

I can fix this tomorrow.

jacquerie commented 7 years ago

...in fact, it looks like the ordering of the records and the checkboxes presented to the user do not match, so selecting a subset via checkboxes results in applying the bulk operation to essentially a random subset of the records. That's really bad!

jmartinm commented 7 years ago

I have not yet been able to reproduce this locally. Tried with 4 different records in the HoldingPen selecting 2 of them and doing batch actions Core and Accept and only the selected ones got affected.

ksachs commented 7 years ago

I think it happens only when you first select all (the checkbox next to Details) and then de-select some records. And then it's random.

ksachs commented 7 years ago

screenshot from 2017-08-02 09 06 56 screenshot from 2017-08-02 09 07 57 screenshot from 2017-08-02 09 08 58 screenshot from 2017-08-02 09 09 25 screenshot from 2017-08-02 09 16 45

jmartinm commented 7 years ago

Thanks, will try that now.

jmartinm commented 7 years ago

Found the culprit, will be fixed in https://github.com/inspirehep/inspirehep-search-js/pull/37