Bug 18433 - Allow to select results to export in item search
Summary: Allow to select results to export in item search
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on: 9573 18394 18459
Blocks: 27565
  Show dependency treegraph
 
Reported: 2017-04-14 14:02 UTC by Julian Maurice
Modified: 2021-04-20 04:06 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement to the item search in the staff interface (Home > Search > Item search) adds the ability to export selected items. Before this enhancement the only option available was to export all the search results.
Version(s) released in:
20.05.00


Attachments
Bug 18433: Allow to select results to export in item search (19.49 KB, patch)
2017-04-14 14:03 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: Allow to select results to export in item search (19.62 KB, patch)
2017-04-14 19:52 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18433: QA fixes (5.30 KB, patch)
2017-04-19 09:57 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: Allow to select results to export in item search (18.91 KB, patch)
2017-04-19 15:06 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: QA fixes (5.30 KB, patch)
2017-04-19 15:06 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: Remove indentation from CSV output (942 bytes, patch)
2017-04-19 15:14 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: Use existing pattern for find_by_koha_field (3.70 KB, patch)
2017-04-19 19:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18433: Add new subroutine GetDescriptionByKohaField to AuthorisedValues TT plugin (1.49 KB, patch)
2017-04-19 19:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18433: Use the TT plugin for itemsearch_item.json.inc (2.28 KB, patch)
2017-04-19 19:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18433: Remove Koha::AuthorisedValueCategories::find_by_koha_field (9.17 KB, patch)
2017-04-20 09:07 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: Allow to select results to export in item search (18.87 KB, patch)
2017-04-20 13:44 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: Allow to select results to export in item search (19.09 KB, patch)
2019-11-13 13:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: Remember item search results selected rows in session storage (4.52 KB, patch)
2020-01-17 09:29 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 18433: Allow to select results to export in item search (19.17 KB, patch)
2020-02-17 19:01 UTC, David Nind
Details | Diff | Splinter Review
Bug 18433: Remember item search results selected rows in session storage (4.58 KB, patch)
2020-02-17 19:01 UTC, David Nind
Details | Diff | Splinter Review
Bug 18433: Allow to select results to export in item search (19.24 KB, patch)
2020-02-18 08:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18433: Remember item search results selected rows in session storage (4.65 KB, patch)
2020-02-18 08:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18433: Add missing filter (1.36 KB, patch)
2020-02-18 08:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18433: Display the number of selected items (1.17 KB, patch)
2020-02-18 08:58 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2017-04-14 14:02:47 UTC

    
Comment 1 Julian Maurice 2017-04-14 14:03:29 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2017-04-14 19:52:38 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2017-04-17 13:47:35 UTC
Something sounds wrong here, I'd suggest that GetCategoryByKohaField should be replaced with Koha::AuthorisedValues->search_by_koha_field->categories, do not you think?

Failing QA because of CamelCase for GetCategoryByKohaField and lack of tests.

Note: I did not test but I'd expect
my @items = Koha::Items->search({ itemnumber => { -in => \@itemnumbers } });

to be faster and easier to read than:

my @items;
foreach my $itemnumber (@itemnumbers) {
    my $item = Koha::Items->find($itemnumber);
    if ($item) {
        push @items, $item;
    }
}
Comment 4 Jonathan Druart 2017-04-17 13:48:02 UTC
You should also use Text::CSV to build the CSV content.
Comment 5 Julian Maurice 2017-04-18 07:19:15 UTC
(In reply to Jonathan Druart from comment #3)
> Something sounds wrong here, I'd suggest that GetCategoryByKohaField should
> be replaced with Koha::AuthorisedValues->search_by_koha_field->categories,
> do not you think?

I'm not sure, what would be the return value of Koha::AuthorisedValues->search_by_koha_field('items.barcode') for example ? A list of the authorised values of the category linked to items.barcode ?

What I want is a single AV category, so for me it makes sense to have a Koha::AVCategories->findBy... method.

> 
> Failing QA because of CamelCase for GetCategoryByKohaField and lack of tests.
> 

All Koha::Template::Plugin::* methods are in CamelCase. I think you meant findByKohaField, right ?

> Note: I did not test but I'd expect
> my @items = Koha::Items->search({ itemnumber => { -in => \@itemnumbers } });
> 
> to be faster and easier to read than:
> 
> my @items;
> foreach my $itemnumber (@itemnumbers) {
>     my $item = Koha::Items->find($itemnumber);
>     if ($item) {
>         push @items, $item;
>     }
> }

Probably, I'm not familiar with the search() parameters yet, that's why I dit it that way.

(In reply to Jonathan Druart from comment #4)
> You should also use Text::CSV to build the CSV content.

It's not really CSV, just a simple text file with one barcode per line, so what's the benefit of using Text::CSV ?
Comment 6 Julian Maurice 2017-04-18 07:43:54 UTC
> (In reply to Jonathan Druart from comment #4)
> > You should also use Text::CSV to build the CSV content.
> 
> It's not really CSV, just a simple text file with one barcode per line, so
> what's the benefit of using Text::CSV ?

Sorry, I confused with bug 18394, but still, the CSV is generated by Template::Toolkit (for translatability), so, same question: what's the benefit of Text::CSV ? :)
Comment 7 Julian Maurice 2017-04-19 09:57:38 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2017-04-19 12:48:41 UTC
Koha::Item->biblioitem should be moved to its own bug report and covered by tests.
Comment 9 Jonathan Druart 2017-04-19 12:55:05 UTC
When I export in CSV there are 2 spaces at the beginning of each line.
Comment 10 Julian Maurice 2017-04-19 15:06:19 UTC Comment hidden (obsolete)
Comment 11 Julian Maurice 2017-04-19 15:06:23 UTC Comment hidden (obsolete)
Comment 12 Julian Maurice 2017-04-19 15:06:43 UTC
Rebased patches on top of bug 18459
Comment 13 Julian Maurice 2017-04-19 15:14:30 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2017-04-19 19:17:36 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2017-04-19 19:17:43 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2017-04-19 19:17:48 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2017-04-19 19:20:53 UTC
I think I finally found what was wrong.
Take a look at the 2 last patches. What you actually want is Koha::AuthorisedValues->get_description_by_koha_field.
But you missed a TT plugin for an easy access. This method is cached and so certainly much powerful than what your were trying to do.
Let me know if you have any questions.

If you agree we will need to use this sub from the 2 other files, remove the tests and the new method you added.
Comment 18 Julian Maurice 2017-04-20 07:04:50 UTC
(In reply to Jonathan Druart from comment #17)
> What you actually want is
> Koha::AuthorisedValues->get_description_by_koha_field.

Yes, exactly! :-)
Thanks
Comment 19 Julian Maurice 2017-04-20 09:07:08 UTC Comment hidden (obsolete)
Comment 20 Julian Maurice 2017-04-20 09:08:25 UTC Comment hidden (obsolete)
Comment 21 Julian Maurice 2017-04-20 09:09:03 UTC
Back to Signed off
Comment 22 Julian Maurice 2017-04-20 13:44:35 UTC
Created attachment 62472 [details] [review]
Bug 18433: Allow to select results to export in item search

This adds a column of checkboxes in the results table to be able to select the
items to be exported

Test plan:
1. Go to item search and click 'Search'
2. Without checking any checkbox, verify that the export still works (it should
   export all results)
3. Tick some checkboxes and re-export, verify that only selected items are
   exported

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 23 Julian Maurice 2017-04-20 13:45:15 UTC
Squashed all patches for readability
Comment 24 Jonathan Druart 2017-04-24 20:01:55 UTC
Hum, I get "Template process failed: undef error - The method biblioitem is not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121." when I export to CSV.

Something else I completely missed before: if you select rows then switch to another page the behaviour will not work as expected.
Comment 25 Julian Maurice 2017-04-25 07:09:01 UTC
(In reply to Jonathan Druart from comment #24)
> Hum, I get "Template process failed: undef error - The method biblioitem is
> not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121."
> when I export to CSV.

If I apply the patch on a fresh master, I don't get this error, as Koha::Item::biblioitem is defined. You said on bug 18459 that the patch isn't in master, but it is (http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=6527b3b67506db0289997eb9bb98647fa1967a55) so a git fetch + git rebase should fix the problem

> 
> Something else I completely missed before: if you select rows then switch to
> another page the behaviour will not work as expected.

What are you expecting exactly ? That the hidden rows keep their 'selected' state ? Or that the "Export selected results to" label changes back to "Export all results to" because there is no visible selected rows anymore ?
Comment 26 Jonathan Druart 2017-04-25 12:31:15 UTC
(In reply to Julian Maurice from comment #25)
> (In reply to Jonathan Druart from comment #24)
> > Hum, I get "Template process failed: undef error - The method biblioitem is
> > not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121."
> > when I export to CSV.
> 
> If I apply the patch on a fresh master, I don't get this error, as
> Koha::Item::biblioitem is defined. You said on bug 18459 that the patch
> isn't in master, but it is
> (http://git.koha-community.org/gitweb/?p=koha.git;a=commit;
> h=6527b3b67506db0289997eb9bb98647fa1967a55) so a git fetch + git rebase
> should fix the problem

Hum, not sure what happened here, certainly a lack of caffeine...

> > Something else I completely missed before: if you select rows then switch to
> > another page the behaviour will not work as expected.
> 
> What are you expecting exactly ? That the hidden rows keep their 'selected'
> state ? Or that the "Export selected results to" label changes back to
> "Export all results to" because there is no visible selected rows anymore ?

I do not really know but the current behaviour is wrong.
I think the better would be to keep the selected state.
Ccing Owen to get his point of view.
Comment 27 Owen Leonard 2017-08-29 16:40:25 UTC
I agree with Jonathan that the best behavior would be to allow the user to select items on multiple pages and have it remember across pages. I say this not knowing how difficult it might be to implement it.

At the very least the "Export selected results" button should return to its default state if the first page of selections is getting lost, but I don't think it's enough.
Comment 28 Fridolin Somers 2018-10-09 11:18:08 UTC
Maybe this should be splitted into one bug changing Koha::Template::Plugin::AuthorisedValues and this bug focusing on changes in item search.
Comment 29 Jonathan Druart 2018-10-09 15:27:03 UTC
(In reply to Fridolin SOMERS from comment #28)
> Maybe this should be splitted into one bug changing
> Koha::Template::Plugin::AuthorisedValues and this bug focusing on changes in
> item search.

The 2 methods added to the TT plugin are part of master now (since bug 9573).
Comment 30 Fridolin Somers 2018-10-10 06:36:45 UTC
(In reply to Jonathan Druart from comment #29)
> (In reply to Fridolin SOMERS from comment #28)
> > Maybe this should be splitted into one bug changing
> > Koha::Template::Plugin::AuthorisedValues and this bug focusing on changes in
> > item search.
> 
> The 2 methods added to the TT plugin are part of master now (since bug 9573).

Ah super. Only needs rebase then.
Comment 31 Julian Maurice 2019-11-13 13:32:51 UTC
Created attachment 95380 [details] [review]
Bug 18433: Allow to select results to export in item search

This adds a column of checkboxes in the results table to be able to select the
items to be exported

Test plan:
1. Go to item search and click 'Search'
2. Without checking any checkbox, verify that the export still works (it should
   export all results)
3. Tick some checkboxes and re-export, verify that only selected items are
   exported

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 32 Julian Maurice 2019-11-13 13:33:53 UTC
Patch rebased on master
Comment 33 Jonathan Druart 2020-01-02 13:30:42 UTC
(In reply to Owen Leonard from comment #27)
> I agree with Jonathan that the best behavior would be to allow the user to
> select items on multiple pages and have it remember across pages. I say this
> not knowing how difficult it might be to implement it.
> 
> At the very least the "Export selected results" button should return to its
> default state if the first page of selections is getting lost, but I don't
> think it's enough.

This is still valid.
Comment 34 Julian Maurice 2020-01-17 09:29:31 UTC
Created attachment 97479 [details] [review]
Bug 18433: Remember item search results selected rows in session storage

Session storage is not shared between browser tabs, and is cleared when
a tab is closed, so it seems like a good fit for storing selected rows

Test plan:
1. Go to item search (intranet) and submit the form
2. Select some rows on the first page by clicking on checkboxes
3. Go to another page of results
4. Select more rows
5. Go back to the first page
6. Selected rows should still be checked
7. Play with sorting and filtering, and see that selected rows are
   always checked
8. Export as CSV and as a barcodes file, verify that the export contains
   all selected rows
9. Click on 'Clear selection', it should deselect all rows from all
   pages
10. Select some rows again
11. Click on 'Edit search' and submit the same search
12. No row should be selected
13. Select some rows again
14. Refresh the page and submit again the same search
15. No row should be selected
Comment 35 Julian Maurice 2020-01-17 09:31:51 UTC
(In reply to Jonathan Druart from comment #33)
> This is still valid.

Not anymore ;)
Comment 36 David Nind 2020-02-17 19:01:24 UTC
Created attachment 99123 [details] [review]
Bug 18433: Allow to select results to export in item search

This adds a column of checkboxes in the results table to be able to select the
items to be exported

Test plan:
1. Go to item search and click 'Search' (Home > Search > Item search)
2. Without checking any checkbox, verify that the export still works (it should
   export all results)
3. Tick some checkboxes and re-export, verify that only selected items are
   exported

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: David Nind <david@davidnind.com>
Comment 37 David Nind 2020-02-17 19:01:27 UTC
Created attachment 99124 [details] [review]
Bug 18433: Remember item search results selected rows in session storage

Session storage is not shared between browser tabs, and is cleared when
a tab is closed, so it seems like a good fit for storing selected rows

Test plan:
1. Go to item search (intranet) and submit the form
2. Select some rows on the first page by clicking on checkboxes
3. Go to another page of results
4. Select more rows
5. Go back to the first page
6. Selected rows should still be checked
7. Play with sorting and filtering, and see that selected rows are
   always checked
8. Export as CSV and as a barcodes file, verify that the export contains
   all selected rows
9. Click on 'Clear selection', it should deselect all rows from all
   pages
10. Select some rows again
11. Click on 'Edit search' and submit the same search
12. No row should be selected
13. Select some rows again
14. Refresh the page and submit again the same search
15. No row should be selected

Signed-off-by: David Nind <david@davidnind.com>
Comment 38 David Nind 2020-02-17 19:06:18 UTC
Note for others:

- This is on the 'Item search' page rather than the normal 'Search the catalog'
- Go to Staff interface homepage > Search dropdown list in the header > Item search
Comment 39 Jonathan Druart 2020-02-18 08:58:16 UTC
Created attachment 99159 [details] [review]
Bug 18433: Allow to select results to export in item search

This adds a column of checkboxes in the results table to be able to select the
items to be exported

Test plan:
1. Go to item search and click 'Search'
2. Without checking any checkbox, verify that the export still works (it should
   export all results)
3. Tick some checkboxes and re-export, verify that only selected items are
   exported

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 40 Jonathan Druart 2020-02-18 08:58:20 UTC
Created attachment 99160 [details] [review]
Bug 18433: Remember item search results selected rows in session storage

Session storage is not shared between browser tabs, and is cleared when
a tab is closed, so it seems like a good fit for storing selected rows

Test plan:
1. Go to item search (intranet) and submit the form
2. Select some rows on the first page by clicking on checkboxes
3. Go to another page of results
4. Select more rows
5. Go back to the first page
6. Selected rows should still be checked
7. Play with sorting and filtering, and see that selected rows are
   always checked
8. Export as CSV and as a barcodes file, verify that the export contains
   all selected rows
9. Click on 'Clear selection', it should deselect all rows from all
   pages
10. Select some rows again
11. Click on 'Edit search' and submit the same search
12. No row should be selected
13. Select some rows again
14. Refresh the page and submit again the same search
15. No row should be selected

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 41 Jonathan Druart 2020-02-18 08:58:23 UTC
Created attachment 99161 [details] [review]
Bug 18433: Add missing filter

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 42 Jonathan Druart 2020-02-18 08:58:27 UTC
Created attachment 99162 [details] [review]
Bug 18433: Display the number of selected items

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 43 Jonathan Druart 2020-02-18 08:58:57 UTC
(In reply to Julian Maurice from comment #35)
> (In reply to Jonathan Druart from comment #33)
> > This is still valid.
> 
> Not anymore ;)

Awesome Julian, thanks for that follow-up!

(In reply to Jonathan Druart from comment #42)
> Created attachment 99162 [details] [review] [review]
> Bug 18433: Display the number of selected items
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Do you agree with that patch?
Comment 44 Julian Maurice 2020-02-18 09:04:29 UTC
(In reply to Jonathan Druart from comment #43)
> Do you agree with that patch?

Yep. I haven't tested it, but patch looks good.
Comment 45 Martin Renvoize 2020-02-19 16:12:58 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 46 Joy Nelson 2020-03-13 00:42:37 UTC
enhancement not backported to 19.11.x
Comment 47 David Cook 2021-04-20 04:06:07 UTC
This change causes the regression reported in Bug 27565.