https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 154979 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154979&action=edit Bug 34666: Allow item_group to be null and still match The current logic requires that the grop ids match, but this eliminates null matches from the group. The fallout essentially is that the queue won't be checked to fill holds in cases of title level matches where holds don't have an item group id. The queue checks the transport cost matrix while the check reserves check does not, so this may have an impact on holds costs and delivery times To test: 0 - Apply unit test patch 1 - prove -v t/db_dependent/Reserves.t 2 - It fails 3 - Apply this patch 4 - prove -v t/db_dependent/Reserves.t 5 - It passes! -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 154981 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154981&action=edit Bug 34666: Combine queries in _Findgroupreserve The queries here are the same except for 2 differences: 1 - They check if the hold was on a particular item 2 - The latter confirms that the reserve item group matches the item's item group For 1, it doesn't matter - only 1 item can be mapped ot a reserve, itemnumber is the primary key for hold_fill_targets - so we are either matching it in the first query or the second, either way we get the same reserve - the returns are the same so we don't care which query it came from For 2, this has already been checked when the queue was built. We don't need to verify the match because it wouldn't be in the targets if they didn't match To test: 1 - Apply second unit test patch 2 - prove t/db_dependent/Reserves.t 3 - It should pass 4 - Apply this patch 5 - prove t/db_dependent/Reserves.t 6 - It continues to pass -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154979|0 |1 is obsolete| | --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 154984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154984&action=edit Bug 34666: Allow item_group to be null and still match The current logic requires that the grop ids match, but this eliminates null matches from the group. The fallout essentially is that the queue won't be checked to fill holds in cases of title level matches where holds don't have an item group id. The queue checks the transport cost matrix while the check reserves check does not, so this may have an impact on holds costs and delivery times To test: 0 - Apply unit test patch 1 - prove -v t/db_dependent/Reserves.t 2 - It fails 3 - Apply this patch 4 - prove -v t/db_dependent/Reserves.t 5 - It passes! Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154981|0 |1 is obsolete| | --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 154986 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=154986&action=edit Bug 34666: Combine queries in _Findgroupreserve The queries here are the same except for 2 differences: 1 - They check if the hold was on a particular item 2 - The latter confirms that the reserve item group matches the item's item group For 1, it doesn't matter - only 1 item can be mapped ot a reserve, itemnumber is the primary key for hold_fill_targets - so we are either matching it in the first query or the second, either way we get the same reserve - the returns are the same so we don't care which query it came from For 2, this has already been checked when the queue was built. We don't need to verify the match because it wouldn't be in the targets if they didn't match To test: 1 - Apply second unit test patch 2 - prove t/db_dependent/Reserves.t 3 - It should pass 4 - Apply this patch 5 - prove t/db_dependent/Reserves.t 6 - It continues to pass Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Please fetch latest qa tools: WARN t/db_dependent/Reserves.t OK critic OK forbidden patterns OK git manipulation OK pod SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 510, now: 515) OK valid -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #12)

(In reply to Marcel de Rooy from comment #11)

...

Please fetch latest qa tools:

WARN t/db_dependent/Reserves.t OK critic OK forbidden patterns OK git manipulation OK pod SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 510, now: 515) OK valid

Fresh KTD, pulled - qa-tools pulled - I get tidiness warnings (often) but not for these patches. Do you have other patches applied?

Interesting. I remember a discussion earlier if there is a difference between the Koha git repo perltidyrc and the one used in KTD. Could you compare them? I am just using the git repo perltidyrc. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #14)

(In reply to Marcel de Rooy from comment #13)

...

(In reply to Nick Clemens from comment #12)

...

(In reply to Marcel de Rooy from comment #11)

...

Please fetch latest qa tools:

WARN t/db_dependent/Reserves.t OK critic OK forbidden patterns OK git manipulation OK pod SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 510, now: 515) OK valid

Fresh KTD, pulled - qa-tools pulled - I get tidiness warnings (often) but not for these patches. Do you have other patches applied?

Interesting. I remember a discussion earlier if there is a difference between the Koha git repo perltidyrc and the one used in KTD. Could you compare them? I am just using the git repo perltidyrc.

I only find the one file

Tomas: In view of above, could you confirm that KTD uses the perltidyrc from the root directory of the Koha git repo ? cat .perltidyrc # delete backup if no errors --backup-file-extension='/' --character-encoding=utf8 --output-line-ending=unix --iterations=2 # } else { --cuddled-else # Use the full indent size for continuations --continuation-indentation=4 --no-outdent-long-lines --maximum-line-length=120 --break-at-old-comma-breakpoints And which version, Nick? root@master:/usr/share/koha# perltidy -v This is perltidy, v20230309 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #15)

And which version, Nick? root@master:/usr/share/koha# perltidy -v This is perltidy, v20230309

kohadev-koha@kohadevbox:koha(master)$ perltidy -v This is perltidy, v20200110 Updated perltidy, got the error, patch attached -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154983|0 |1 is obsolete| | --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 155266 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155266&action=edit Bug 34666: Unit test Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #154985|0 |1 is obsolete| | --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 155268 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155268&action=edit Bug 34666: Add a second unit test for item level Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155221|0 |1 is obsolete| | --- Comment #22 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 155270 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155270&action=edit Bug 34666: Tidy Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 --- Comment #24 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #23)

You must use the version of perltidy shipped in ktd.

And, if it's too old and need to be upgraded, see bug 30002. Looks like we have 20230309-1~koha1 but it's not the version installed in ktd. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |24860 CC| |fridolin.somers@biblibre.co | |m --- Comment #27 from Fridolin Somers <fridolin.somers@biblibre.com> --- Looks like it depends on Bug 24860 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24860 [Bug 24860] Add ability to place item group level holds -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|23.11.00,23.05.04 |23.11.00,23.05.04,22.11.10 released in| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34666 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28205 -- You are receiving this mail because: You are watching all bug changes.