Bug 36649 - Adding recently added items to processing from waiting list does not work if processing includes information from database columns
Summary: Adding recently added items to processing from waiting list does not work if ...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Preservation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-19 10:20 UTC by Anneli Österman
Modified: 2024-04-26 06:53 UTC (History)
3 users (show)

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


Attachments
Bug 36649: Correctly embed biblio when retrieving items when adding to a train (2.26 KB, patch)
2024-04-22 12:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 36649: Correctly embed biblio when retrieving items when adding to a train (2.41 KB, patch)
2024-04-23 10:46 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 36649: Correctly embed biblio when retrieving items when adding to a train (2.47 KB, patch)
2024-04-23 11:15 UTC, Pedro Amorim
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Anneli Österman 2024-04-19 10:20:55 UTC
After adding items to the waiting list in Preservation module you get a button "Add last x items to a train". If you click it and select a processing that contains columns from the database (for example biblio.title or biblio.author), the submit button does not work and items are not added to the selected processing. If the processing uses only free text or authorised values the adding of the items works.

I tested this in version 23.11 and on PTFS Europe's sandbox version 23.12.

How to reproduce:

1. Add a new processing that contains columns from the database, for example biblio.title and biblio.author.
2. Create a new train.
3. Add items to the waiting list.
4. Click on "Add last x items to the train" button.
5. Choose the train you created on number 2.
6. Choose the processing you added on number 1.
7. Try to click the Submit button.
8. Notice that the button does not work and items are not added to the selected processing.
Comment 1 Emmi Takkinen 2024-04-19 12:04:56 UTC
This problem is caused by line 185 in TrainsFormAddItems.vue:
item.biblio[api_attribute]. 

Apparently, if attribute uses column from biblio or biblioitems table, attribute should be fetched from biblio array which is part of item object. However there is no such an array.
Comment 2 Jonathan Druart 2024-04-22 12:38:51 UTC
Created attachment 165275 [details] [review]
Bug 36649: Correctly embed biblio when retrieving items when adding to a train

I didn't find when this regression has been introduced but the third
parameter here is the headers, not parameters, we should not repeat
"headers".

Test plan:
1. Add a new processing that contains columns from the database, for example biblio.title and biblio.author.
2. Create a new train.
3. Add items to the waiting list.
4. Click on "Add last x items to the train" button.
5. Choose the train you created on number 2.
6. Choose the processing you added on number 1.
7. Try to click the Submit button.
=> Without the patch nothing happens and there is a JS error in the
console: Uncaught TypeError: item.biblio is undefined
=> With this patch applied the item is correctly added to the train and
the attribute is properly populated.
Comment 3 ByWater Sandboxes 2024-04-23 10:46:33 UTC
Created attachment 165372 [details] [review]
Bug 36649: Correctly embed biblio when retrieving items when adding to a train

I didn't find when this regression has been introduced but the third
parameter here is the headers, not parameters, we should not repeat
"headers".

Test plan:
1. Add a new processing that contains columns from the database, for example biblio.title and biblio.author.
2. Create a new train.
3. Add items to the waiting list.
4. Click on "Add last x items to the train" button.
5. Choose the train you created on number 2.
6. Choose the processing you added on number 1.
7. Try to click the Submit button.
=> Without the patch nothing happens and there is a JS error in the
console: Uncaught TypeError: item.biblio is undefined
=> With this patch applied the item is correctly added to the train and
the attribute is properly populated.

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Comment 4 Pedro Amorim 2024-04-23 11:15:50 UTC
Created attachment 165376 [details] [review]
Bug 36649: Correctly embed biblio when retrieving items when adding to a train

I didn't find when this regression has been introduced but the third
parameter here is the headers, not parameters, we should not repeat
"headers".

Test plan:
1. Add a new processing that contains columns from the database, for example biblio.title and biblio.author.
2. Create a new train.
3. Add items to the waiting list.
4. Click on "Add last x items to the train" button.
5. Choose the train you created on number 2.
6. Choose the processing you added on number 1.
7. Try to click the Submit button.
=> Without the patch nothing happens and there is a JS error in the
console: Uncaught TypeError: item.biblio is undefined
=> With this patch applied the item is correctly added to the train and
the attribute is properly populated.

Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 5 Pedro Amorim 2024-04-23 11:16:05 UTC
Before, we added x-koha-embed headers at the http client level (see erm-api-client.js) not at the vue component level as you're doing here @Joubu.

Not a blocker, but may be worth mentioning as it's a deviation of the pattern?
Comment 6 Jonathan Druart 2024-04-23 11:47:55 UTC
(In reply to Pedro Amorim from comment #5)
> Before, we added x-koha-embed headers at the http client level (see
> erm-api-client.js) not at the vue component level as you're doing here
> @Joubu.
> 
> Not a blocker, but may be worth mentioning as it's a deviation of the
> pattern?

This is expected, we are supposed to embed when needed. Here we don't want to embed biblio's info from everywhere we are fetching items.

It's how getAll is designed:
     get items() {
         return {
            getAll: (query, params, headers) =>
Comment 7 Katrin Fischer 2024-04-26 06:53:58 UTC
Pushed for 24.05!

Well done everyone, thank you!