https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25260 --- Comment #68 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #65)
(In reply to Tomás Cohen Arazi from comment #63)
- t/db_dependent/Reserves.t failure
There are 2 things: 1. 1107 item_id => $item->biblionumber, Key must be biblio_id
2. The $title_level_target_query query in _Findgroupreserve is now returning the "reserved" hold. On master it's not matching any rows and the third query ($query) is hit and returned the different holds.
I think it's coming from: - JOIN hold_fill_targets USING (reserve_id) vs + JOIN hold_fill_targets ON ( + holds.biblio_id=hold_fill_targets.biblionumber + AND holds.patron_id=hold_fill_targets.borrowernumber)
Thanks! That solved Reserves.t. Will ask Agustin about that change, because we merged many commits in our branch into this commit (a year ago).
- t/db_dependent/Circulation.t failure
# got: 'on_reserve' # expected: 'too_soon' Related to hold's status as well so may be fixed if the previous test is corrected.
This might be something else because the problem stands. Will submit the version with the fix for review.
- Add some warning in about.pl about wrong letters (maybe?)
After we moved the marcxml out of biblioitems we added a warning on the report list view. Maybe we should do the same for the notice templates?
commit f22d2e7200ee8b35aff66b26acc3e2daa49f9f0d Bug 17898: Automagically convert SQL reports
Good idea. If there are chances this dev gets pushed to master, this is something I will work on.
Questions: * Shouldn't *_date DB fields be *_on?
Probably, Some will be _on, some others _until. I didn't think about it and went into doing the API-way. I didn't find a written Coding guideline about this, though. No problem for me with changing this. It will require a bunch of new mappings only (Koha::Hold->to_api_mapping).
* item_level => item_level_request
Currently it is: - 'item_level_hold' in (old)reserves - 'item_level_request' in hold_fill_target and tmp_holdsqueue - 'item_level' on the API (voted)
I think we agreed on "item_level_request", why did you change it?
As with dates, I preferred to directly use the API version. Can be revisited in a follow-up patch along with the other changes if QA requires it.
* Shouldn't holds.id be holds.hold_id? I cannot remember when/where but I think we agreed we shouldn't use "id" (to prevent wrong id to be returned on JOIN).
That's correct, and it is in the guidelines. Follow-up coming. Thanks for the review! -- You are receiving this mail because: You are watching all bug changes.