https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25260
--- Comment #68 from Tomás Cohen Arazi <[email protected]> --- (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. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
