Bug 36130 - ILL batches table not showing all batches
Summary: ILL batches table not showing all batches
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Pedro Amorim
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords: rel_23_11_candidate
Depends on: 30719 36241
Blocks: 36291
  Show dependency treegraph
 
Reported: 2024-02-19 13:11 UTC by Pedro Amorim
Modified: 2024-12-23 08:05 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.04
Circulation function:


Attachments
Bug 36130: Fetch all batches for the table (1.56 KB, patch)
2024-02-19 13:19 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36130: Fetch all batches for the table (1.60 KB, patch)
2024-02-19 14:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 36130: Fetch all batches for the table (1.66 KB, patch)
2024-02-21 13:30 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36130: Fetch all batches for the table (1.77 KB, patch)
2024-03-06 10:01 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36130: Fetch all batches for the table (1.82 KB, patch)
2024-03-11 12:14 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro Amorim 2024-02-19 13:11:04 UTC

    
Comment 1 Pedro Amorim 2024-02-19 13:19:04 UTC
Created attachment 162270 [details] [review]
Bug 36130: Fetch all batches for the table

Test plan, ktd:
- Add 'FreeForm' backend and enable ILLModule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
- Add 21 batches, run the following command in kshell:
for x in {1..21}; do echo "INSERT INTO illbatches (name, backend, library_id, status_code, patron_id) VALUES ('$x', 'FreeForm', 'CPL', 'NEW', 52);"; done | koha-mysql kohadev
- Visit ILL batches page:
<INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=batch_list
- Notice only 20 batches show.
- Apply patch. Notice 20 batches show but now there is a page 2 link.
- Test page 2 works as expected, delete a batch, notice only 1 page with 20 batches shows again.
Comment 2 David Nind 2024-02-19 14:29:36 UTC
Created attachment 162273 [details] [review]
Bug 36130: Fetch all batches for the table

Test plan, ktd:
- Add 'FreeForm' backend and enable ILLModule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
- Add 21 batches, run the following command in kshell:
for x in {1..21}; do echo "INSERT INTO illbatches (name, backend, library_id, status_code, patron_id) VALUES ('$x', 'FreeForm', 'CPL', 'NEW', 52);"; done | koha-mysql kohadev
- Visit ILL batches page:
<INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=batch_list
- Notice only 20 batches show.
- Apply patch. Notice 20 batches show but now there is a page 2 link.
- Test page 2 works as expected, delete a batch, notice only 1 page with 20 batches shows again.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Pedro Amorim 2024-02-21 13:30:45 UTC
Created attachment 162319 [details] [review]
Bug 36130: Fetch all batches for the table

Test plan, ktd:
- Add 'FreeForm' backend and enable ILLModule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
- Add 21 batches, run the following command in kshell:
for x in {1..21}; do echo "INSERT INTO illbatches (name, backend, library_id, status_code, patron_id) VALUES ('$x', 'FreeForm', 'CPL', 'NEW', 52);"; done | koha-mysql kohadev
- Visit ILL batches page:
<INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=batch_list
- Notice only 20 batches show.
- Apply patch. Notice 20 batches show but now there is a page 2 link.
- Test page 2 works as expected, delete a batch, notice only 1 page with 20 batches shows again.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Comment 4 Victor Grousset/tuxayo 2024-03-05 06:52:03 UTC
> - Visit ILL batches page:
> <INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=batch_list
> - Notice only 20 batches show.

hmm, I got a mostly empty page with just "new ILL request" and "List requests"

Is that the right URL? ↓↓↓↓
http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl?method=batch_list

- no server log error
- no JS error
- illbatches table is populated with 21 rows
- services where restarted after setting up ILL
- ILL home looks about right (http://localhost:8081/cgi-bin/koha/ill/ill-requests.pl)

Any hint of some implicit step that I would have missed?
Comment 5 Pedro Amorim 2024-03-06 10:01:04 UTC
Created attachment 162810 [details] [review]
Bug 36130: Fetch all batches for the table

Test plan, ktd:
- Add 'FreeForm' backend and enable ILLModule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
- Install a metadata enrichment plugin, e.g.:
https://github.com/PTFS-Europe/koha-plugin-api-pubmed/releases
- Add 21 batches, run the following command in kshell:
for x in {1..21}; do echo "INSERT INTO illbatches (name, backend, library_id, status_code, patron_id) VALUES ('$x', 'FreeForm', 'CPL', 'NEW', 52);"; done | koha-mysql kohadev
- Visit ILL batches page:
<INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=batch_list
- Notice only 20 batches show.
- Apply patch. Notice 20 batches show but now there is a page 2 link.
- Test page 2 works as expected, delete a batch, notice only 1 page with 20 batches shows again.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Comment 6 Pedro Amorim 2024-03-06 10:02:44 UTC
(In reply to Victor Grousset/tuxayo from comment #4)
> Any hint of some implicit step that I would have missed?

Thank you Victor, couple things were happening here:
1) I was missing a step in the test plan (the metadata enrichment plugin step is required)
2) The base ILL batches functionality broke after bug 34478. This current bug now depends on bug 36241 in order to work properly.

Thank you.
Comment 7 Tomás Cohen Arazi (tcohen) 2024-03-09 14:04:26 UTC
Pagination works are advertised with this patch.

I had to use your data faker script [1] in order to test.

But when I scroll down on the page, some weird behavior is trying to scroll up again. Please check.

[1] https://github.com/ammopt/koha-ill-dev/blob/master/fake_data.pl
Comment 8 Tomás Cohen Arazi (tcohen) 2024-03-11 12:14:23 UTC
Created attachment 163025 [details] [review]
Bug 36130: Fetch all batches for the table

Test plan, ktd:
- Add 'FreeForm' backend and enable ILLModule, run:
bash <(curl -s https://raw.githubusercontent.com/ammopt/koha-ill-dev/master/start-ill-dev.sh)
- Install a metadata enrichment plugin, e.g.:
https://github.com/PTFS-Europe/koha-plugin-api-pubmed/releases
- Add 21 batches, run the following command in kshell:
for x in {1..21}; do echo "INSERT INTO illbatches (name, backend, library_id, status_code, patron_id) VALUES ('$x', 'FreeForm', 'CPL', 'NEW', 52);"; done | koha-mysql kohadev
- Visit ILL batches page:
<INTRA_URL>/cgi-bin/koha/ill/ill-requests.pl?method=batch_list
- Notice only 20 batches show.
- Apply patch. Notice 20 batches show but now there is a page 2 link.
- Test page 2 works as expected, delete a batch, notice only 1 page with 20 batches shows again.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Edith Speller <edith.speller@ukhsa.gov.uk>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi (tcohen) 2024-03-11 12:20:11 UTC
(In reply to Tomás Cohen Arazi from comment #7)
> But when I scroll down on the page, some weird behavior is trying to scroll
> up again. Please check.

I decided to file a separate bug for that, as it is not introduced by this.
Comment 10 Katrin Fischer 2024-03-12 16:49:26 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 11 Fridolin Somers 2024-03-13 08:09:43 UTC
Depends on Bug 36241 not in 23.11.x
Comment 12 Pedro Amorim 2024-03-13 16:51:13 UTC
(In reply to Fridolin Somers from comment #11)
> Depends on Bug 36241 not in 23.11.x

Hey, sorry Frido I added the dependency because at the time I submitted the bug bug 36241 was required to follow the test plan.
This actually depends on bug 30719 which is in 23.11.x so this should ideally be backported.
Comment 13 Fridolin Somers 2024-03-19 07:45:54 UTC
Pushed to 23.11.x for 23.11.04
Comment 14 Lucas Gass (lukeg) 2024-03-19 21:53:50 UTC
Missing dependencies for 23.05.x, no backport.