Time Nick Message
02:31 dcook @later tell nugged take a look at bug 28519 as well
02:31 huginn dcook: The operation succeeded.
02:31 dcook bug 28519
02:31 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28519 enhancement, P5 - low, ---, dcook, Needs Signoff , Add a 2nd directory for Perl modules
02:31 dcook I've been wanting to do something like that for ages
06:13 paxed $ grep remove_guarantors misc/cronjobs/update_patrons_category.pl
06:13 paxed my $remove_guarantors = 0;
06:13 paxed hmmm yes, useful.
06:28 cait1 kidclamp: khall: bywater solutions demo is down
06:28 cait1 https://catalog.bywatersolutions.com/
06:29 magnuse \o/
06:35 reiveune hello
06:40 * dcook waves
06:45 fridolin hi
06:53 cait1 ... and hi #koha :)
06:53 cait1 fridolin: biblibre demo is the oldest version right now - any change in sight?
06:53 Nemo_bis Oh, saw some Koha promotion going strong on social media :) https://www.kiwi.fi/pages/viewpage.action?pageId=223543412
06:54 alex_a Bonjour
06:54 cait1 Nemo_bis: hope Google translate can cope :)
06:55 fridolin cait1: ohhh we will soon update to 20.11.06, thanks for the alert
06:56 cait1 just let me know or update the demo page yourself - i usually do it once a month (hence the comments today)
06:56 cait1 we have one 21.05 already frm Admin_Kuhn++
06:57 cait1 fridolin are you imitating me? :)
06:57 dcook I'll probably be doing a 21.05 soon, although planning to wait until 21.05.01...
06:58 cait1 dcook: if you have a public demo, I'll be happy to add it to the website
06:58 dcook Ah, this is for a particular library, so no demo
06:59 dcook We'll be doing the seemingly typical thing of just doing XX.11 releases
06:59 dcook seemingly typical vendor thing*
07:01 paxed just in case it isn't clear from that wiki.fi page, in finland all marc data in koha libraries is centrally catalogued
07:03 paxed data comes from vendors to koha Täti database, from there to local koha installations. only specific local data is locally catalogued (eg. objects)
07:06 cait1 paxed: it's really interesting as I think our models might be similar
07:06 cait1 does centrally mean one institution does all the cataloguing or is it a cooperative model?
07:08 paxed cait1: co-op
07:11 cait1 paxed: nice :)
07:12 cait1 are you doing changes in the ILS and uploading or updating in the central database and downloading? (I think the latter)
07:14 cait1 700 more libraies listed on hea since last month too :) up to 13822
07:19 paxed cait1: the latter, yes
07:22 Joubu thx dcook for bug 28519
07:22 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28519 enhancement, P5 - low, ---, dcook, Needs Signoff , Add a 2nd directory for Perl modules
07:29 nlegrand Bonjour #koha !
08:19 magnuse kia ora nlegrand
08:24 nlegrand o/ magnuse
08:30 ashimema morning #koha
08:33 ashimema looks like I have lots of catching up to do..
08:49 kohaputti ashimema, hi! Seems like bug 12362 caused a little regression. When you cancel a hold in transit it doesn't show anymore as in transit on the item page. In the DB we have then http://paste.debian.net/plain/1200245
08:49 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12362 normal, P5 - low, ---, martin.renvoize, Pushed to master , Branch transfer records orphaned on cancelled holds
08:50 kohaputti Seems like after that bug the logic was changed so that we create a new transfer *request* which doesn't have a item.transfertwhen / datesend yet registered and in the detail.pl page we only show those
08:50 kohaputti ashimema, I noticed you have quite a bunch of related bugs you're working on so maybe one of them already has a fix for this?
08:53 ashimema good catch.. I'm just on catch up mode at the minute having been out for a week..
08:53 ashimema I'll stick investigating that at the top of the pile..
08:54 ashimema the cancel stuff turned into a bit of a pain in the end.. I think i jumped a bit too quick initially :(.
08:54 ashimema as you say.. it may well be fixed by another bug I've been working on.. but I couldn't point out which one yet ;)
08:55 kohaputti I'd say it is quite bad, maybe even critical regression for 20.05, the transfers state is quite hidden now and might lead to librarians giving misleading info about an item's availability
08:55 kohaputti 21.05
08:55 ashimema I agree
08:55 kohaputti I'm also trying work on a fix for this right now, do we have a bug report already?
08:57 ashimema ah, I see what you mean now..
08:57 ashimema don't think there's a bug yet reported for this exact bit
08:57 kohaputti I will create one right away then
08:57 ashimema pretty sure none of my other work fixes this
08:57 kohaputti I'm putting this as critical, sounds good?
08:57 ashimema I think detail.pl needs adapting to understand 'TransferCancellation'
08:58 ashimema yeah, sounds good
08:58 ashimema you say you're looking into it right now.. are you happy continuing or would you like to pass the baton and get me to look at it?
08:58 kohaputti ashimema, I'm happy if we both look into this
08:58 * ashimema could spend some time today... once my morning meeting is out of the way.
08:58 ashimema brill
08:58 kohaputti no guarantees I will understand this new system
08:58 ashimema lets both attack it then :)
09:11 kohaputti ashimema, hmm, why have frombranch populated btw for TransferCancellation
09:12 kohaputti ashimema, anyway, I'm unsure how to accept this transfer request so ... maybe it gets changed at some point
09:12 ashimema so.. you should only get this type of transfer if you cancel a transfer whilst it's 'in transit'
09:13 ashimema so.. checking it in to the middle library should trigger the 'transfer back to' prompt
09:13 ashimema I'll have a dig.. juggling a meeting at the minute I'm afraid.
09:14 ashimema the frombranch being populated.. that feels a little odd.. I'm less sure of that
09:20 kohaputti yes it does bring that pop-up on check-in, though seems like the transit datesent doesn't get updated
09:51 ashimema hmm
09:52 ashimema confirming (either with 'Yes' or 'Yes, Print slip') should set datesent :(
09:52 ashimema I just got out of my call
09:52 ashimema have to fix the sandboxes for a demo, then will jump on this
10:05 ashimema got a bug number for your newly created bug kohaputti?
10:05 kohaputti bug 28520
10:05 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28520 critical, P5 - low, ---, koha-bugs, NEW , Cancelling a hold that is in transit hides item's transit status
10:05 ashimema ta
10:24 cait2 paxed: sorry if i missed your reply - technical issues with the laptop earlier
10:34 * cait2 read back now
10:49 kohaputti ashimema, I now notice that also the check after this change in Koha::Item::Transfer->in_transit is not right. The cancelled transfer has been in transit and just by merely cancelling the hold shouldn't make it non-transit state but now because there is no datesent it is not in transit
10:56 kohaputti ashimema, I might have explained in a difficult way: I mean cancelling hold shouldn't empty the datesent because if datesent is null then the is_transit check returns false.
10:58 kohaputti ashimema, hmm, maybe is_transit should then also check whether reason == TransferCancellation
11:03 ashimema I know exactly what you mean
11:03 ashimema I'm digging through now
11:03 ashimema in effect it's the cancelled trnasfer that's still 'in_transit'
11:04 ashimema the new one, 'TransferCallellation' isn't actually in transit yet.. transit will only get triggered upon checkin of the prior transfer
11:04 ashimema ack
11:08 ashimema crap, 24434 didn't get it
11:08 ashimema * crap, 24434 didn't get in
11:12 ashimema right..
11:12 ashimema 24434 is the big one here kohaputti..
11:12 ashimema I think it resolves most of it.
11:12 ashimema I'm just trying to get my head back into it..
11:24 oleonard Hi all
11:40 ashimema kohaputti, do you think you would have a moment to look at bug 27064
11:40 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27064 normal, P5 - low, ---, nick, Signed Off , Transferring an item with a hold allows the user to set a hold waiting without transferring to the correct branch
11:40 ashimema it's mixed up in all this and I think is also fairly critical
11:40 ashimema problem I'm having is keeping track of the set of bugs as a whole now..
11:41 ashimema getting one off the pile would help my brain ;)
11:58 kohaputti ashimema, I'm checking
12:08 kohaputti ashimema, for availability showing purposes on opac the new 'TransferCallellation' should be somehow listed as in transit, otherwise the patron thinks it is available at the library
12:08 kohaputti ashimema, but continuing now on 27064 ...
12:21 ashimema Yes, I'm working on that part now
12:22 ashimema in short, I believe we need to switch to 'get_transfers' and deprecate 'get_transfer'
12:22 kohaputti I think you mean C4::Circulation::GetTransfers ?
12:22 kohaputti or no?
12:22 ashimema but also update get_transfers to list cancelled_in_transit up high too
12:22 ashimema nope..
12:22 ashimema right..
12:22 kohaputti ah, there is so many ways to get the transfers
12:23 kohaputti this one you mean is Koha::Item::get_transfer
12:23 ashimema C4::Circulation::GetTransfers never actually did 's'.. i.e it only ever returns one transfer
12:23 ashimema that's what Koha::Item->get_transfer does
12:23 ashimema i.e get_transfer was a direct clone of GetTransfers
12:23 ashimema I introduced 'get_transfers' later.. and it deals with queues.. so returns an ordered list
12:24 ashimema in this case we want that list
12:24 ashimema so we can get both transfers.. the one that's 'in_transit and cancelled' and the replacement one too
12:38 kohaputti ashimema, reverse transfer is ?
12:41 kohaputti https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27064#c32 victor also seems to have reported a bug
12:41 huginn Bug 27064: normal, P5 - low, ---, nick, Signed Off , Transferring an item with a hold allows the user to set a hold waiting without transferring to the correct branch
12:41 kohaputti "There is a second box"
12:48 ashimema OK.. looking into the second box
12:48 ashimema thanks
12:48 ashimema reverse transfer..
12:48 ashimema the cancellation stuff..
12:49 ashimema basically I was trying to fix the orphaned transfers.. so if an item was actually in transit when it's cancelled we added a transfer back to it's originating branch
12:49 ashimema that's the 'reverse transfer'
12:50 ashimema but in a bunch of cases we don't want it.. hence adding a 'replace' param in the above
12:50 kohaputti ok
12:51 ashimema I have a tweak to get_transfer/get_transfers I'm working on now..
12:51 ashimema but I need to work through side effects.. which is going to be fun
12:52 pastebot "ashimema" at 127.0.0.1 pasted "get_transfer query update" (22 lines) at http://paste.koha-community.org/1011
12:52 kohaputti ashimema, it is really bad transferbook does more than just transferring the book... like returns info about holds.
12:52 ashimema yup
12:52 kohaputti really hard to modify this code
12:52 ashimema I'm trying hard to get rid of it
12:53 ashimema but I've worked through it at least three times now and found myself tied in knots
12:53 ashimema it's a horrible rabbit hole
12:53 ashimema at least it's only called in one place now..
12:53 caroline_timelady good <time of day> everyone! How's monday so far?
12:53 ashimema I think allot of the logic should likely go in the controller now instead
12:54 ashimema hi caroline_timelady
12:54 kohaputti ashimema, thanks for the info, I will accommodate my critics accordingly then :)
12:55 ashimema hehe
12:55 ashimema I am still working towards killing it ;)
12:55 ashimema just not in this bug.. ;)
13:01 ashimema if it makes you feel any better.. but I think it's unlikely it will.. your 'critical' about the details page not showing the item is in transit.. that actually pre-existed my bug.. you're just drawn to it more now..
13:02 ashimema i.e. prior to my bug, the transfer was just completely nuked so you had no idea where the item was at all
13:02 kohaputti oh, I see
13:03 ashimema audit trail in branchtransfers is cool.. but it'shighlighting all sorts of edge cases that were hidden before..
13:03 kohaputti We could lower the priority then
13:03 ashimema I still reckon it's important to fix ;)
13:06 kohaputti ashimema, could we maybe queue those transfers, instead of just cancelling the first one keep it and then make a new one back to the from_library?
13:07 ashimema yeah, in effect that's what I'm thinking..
13:07 kohaputti or maybe you have something already in mind
13:07 kohaputti ok
13:07 ashimema well.. ish
13:07 ashimema right now, if you queue a transfer the first checkin, assuming it's at the right branch, will trigger a 'transfer arrived' dialogue and then you can lend the item etc (to fulfil the hold)..
13:08 ashimema it's only on the next checkin you then get notified of the queued transfer
13:08 ashimema that works perfectly for successful transfers..
13:08 ashimema in this case as we've cancelled the transfer in-transit.. we want to prompt the immediate return on that first checkin..
13:08 ashimema I think
13:09 kohaputti ashimema, hmm, are you sure the details page didn't show the item in transit before this bug. I think it might have kept the transfer to the original hold destination and shown it
13:09 kohaputti I can check but how sure are you? :D
13:09 ashimema I'd just keep the bug at critical and not worry too much about the history ;)
13:10 ashimema ModItemTransfer just nuked things..
13:10 ashimema but it may have used a different code path to cancel/delete.. one that just set datearrived
13:10 ashimema there's a regression in there somewhere in either case I reckon
13:12 kohaputti hmm, ok. I might check later tonight though so we will keep the functionality similar to how it has been before and don't break any previous workflows due to any new changes we might do
13:12 ashimema good plan
13:12 tcohen morning
13:19 oleonard Hi tcohen
13:32 * ashimema has now removed the superflous error message box on bug 27064
13:32 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27064 normal, P5 - low, ---, nick, Signed Off , Transferring an item with a hold allows the user to set a hold waiting without transferring to the correct branch
13:57 kohaputti ashimema, https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=121674&action=diff not sure it is right, shouldn't we just put the line above also in if else so the errmsgloop doesn't get populated
13:57 kohaputti no need to do anything specific to holds
13:58 ashimema humm
13:58 kohaputti maybe I can send ALT patch :D
13:58 ashimema hehe.. that might help my poor brain ;)
14:12 kohaputti ashimema, attached it now, can you sign-off if it looks good
14:13 ashimema :)
14:13 kohaputti ashimema, I'm still doing qa on this patch set btw, I think I saw something dodgy earlier
14:13 ashimema :)
14:13 ashimema thanks,
14:38 kohaputti ashimema, how comes bug 27064 is in SO state and I don't see any SO lines?
14:38 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27064 normal, P5 - low, ---, nick, Signed Off , Transferring an item with a hold allows the user to set a hold waiting without transferring to the correct branch
14:39 kohaputti ah, because it is from Nick originally
14:44 kohaputti ashimema, maybe you can think now ;) PQA.
14:45 ashimema aha, I see.. yeah, I like your alt
14:45 ashimema nice one
14:49 kohaputti ah, sorry looks like you submitted the patches before and I had those versions with out your SO.. but good you added those back :)
14:49 ashimema haha, no worries
14:49 kohaputti I mean we had a race condition
14:49 ashimema all looks good to me now :)
14:50 ashimema I'm attacking your critical now.. have a few various idea's and trying them out now
14:50 kohaputti ashimema, I assume you only added the follow-up earlier? And just submitted again those other bugs with your SO?
14:50 kohaputti if so, then we are good
14:51 ashimema correct
15:06 kohaputti ashimema, hmm, maybe the logic should be cancel transfer upon arrival?
15:07 ashimema possibly.. but there are lots of ways to cancel a transfer.. not just via holds
15:07 ashimema I think
15:08 kohaputti or do we need a new branch location for unknown in-transit location
15:08 kohaputti then it would be in-transit from unknown location to it's from_library
15:08 ashimema I think I have a fix.. just checking and then will upload
15:08 ashimema just so you can see where I'm going..
15:09 kohaputti sounds great
15:09 ashimema I still need to write some unit tests for it to get through QA etc though.
15:12 ashimema patch attached
15:13 ashimema i went round and round in circles and that the simplest approach in the end..
15:13 ashimema basically.. fix the 'in_transit' method back to ignoring cancellation status..
15:14 ashimema and update the details controller to use get_transfers (note the 's')..
15:14 ashimema the query in that method has been updated to return cancelled but in transit first
15:17 eythian https://www.metafilter.com/191698/patron-records-and-circulation-privacy-in-libraries perhaps interesting for people here
15:17 kohaputti ok, trying to understand this now :)
15:21 kohaputti ashimema, maybe this could work, then the same fix needs to be put to OPAC too
15:21 kohaputti and rest?
15:21 kohaputti where else
15:21 ashimema ah yes.. it does need applying to OPAC too
15:22 ashimema I have a bug that starts working through all cases of GetTransfers calls and replaces them with calls to get_transfer
15:22 ashimema I think I need to dig that back out and work through each case of that converting to get_transfers and handle it appropriately
15:22 ashimema that's bug 24295
15:22 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24295 enhancement, P5 - low, ---, martin.renvoize, Needs Signoff , C4::Circulation::GetTransfers should be removed, use Koha::Item->get_transfer instead
15:22 kohaputti ashimema, should we remove the from_branch from these cancellation transfers?
15:23 ashimema which I'm about to switch back to 'assigned' now we've uncovered this issue.. I don't think it's ready
15:23 kohaputti ashimema, our librarians got confused because the transfer was from the same branch to same branch
15:23 ashimema I think for audit trail it's helpful to have.. isn't it?
15:23 kohaputti ashimema, like if the from_branch is empty we could show some message that the origin is unknown
15:23 ashimema hum.. but we don't update the 'tobranch'
15:24 kohaputti from_branch
15:24 ashimema so shouldn't it come out as 'from A to B', checkin triggers 'from B to A'
15:24 ashimema so I'm not sure where 'from A to A' comes from?
15:24 kohaputti the from_branch is automatically the holdingbranch of item and tobranch is automatically the original transfers from_branch
15:25 kohaputti ashimema, if the hold is cancelled whilst it still has not been accepted for transfer
15:25 ashimema ah.. for the second transfer..
15:25 ashimema yeah..
15:25 ashimema I see
15:25 kohaputti I mean someone checks the item in in the from_branch and so the holdingbranch is still that
15:25 kohaputti I guess it is through the whole transfer actually
15:26 ashimema calling 'transit' should update the 'from_branch' to 'holdingbranch'.. and 'holdingbranch' should have just been updated by the checkin
15:26 ashimema I think
15:27 ashimema so yeah..
15:27 ashimema at 'request time' your right.. from == to..
15:27 ashimema but at 'transit time' from will get updated
15:27 ashimema I think
15:27 kohaputti wait it is in my example
15:29 kohaputti ashimema, in the example both frombranch: MPL and tobranch: MPL are same
15:29 kohaputti https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28520
15:29 huginn Bug 28520: critical, P5 - low, ---, koha-bugs, NEW , Cancelling a hold that is in transit hides item's transit status
15:29 ashimema indeed..
15:29 ashimema but have you actually set the tranfer to transit ;)
15:30 ashimema there's no 'datesent' in the second transfer yet..
15:30 kohaputti let me see, I think I did, by checking in the item
15:30 ashimema ah...
15:30 kohaputti ashimema, the second transfer is the one that was automatically generated by cancelling the hold
15:30 ashimema I bet the 'updatewrongtransfer' stuff isn't kicking in here yet
15:30 ashimema indeed
15:30 kohaputti what is updatewrongtransfer stuff? :D
15:31 ashimema bug 24434
15:31 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24434 major, P5 - low, ---, martin.renvoize, Failed QA , C4::Circulation::updateWrongTransfer is never called but should be
15:31 ashimema basically.. right now if you check an item in at an end location that doesn't match the final tobranch of a transfer.. we don't trigger the onward transfer to the correct location.
15:32 ashimema which is a bugger.
15:32 ashimema I'm just trying to wrap my head round the QA failure on that one now too
15:32 ashimema fun times aye
15:33 kohaputti ashimema, this is hard question. Which branch should the in-transit item be located in
15:34 kohaputti the destination or from one
15:34 ashimema the current holding.. i.e the 'frombranch'
15:35 kohaputti hmm, let me see what happens when we check-in the item with this cancelled transfer in the original destination branch, whether it updates the frombranch
15:35 ashimema really.. from should only get set at the point of transit and not at the point of request I reckon
15:35 ashimema but that idea broke lots of things too early in the refactor
15:35 reiveune bye
15:36 ashimema good question..
15:36 ashimema I get the feeling that without 24434 it won't do anything to the transfer at all..
15:36 ashimema I might be wrong..
15:37 kohaputti ashimema, I mean I think in our case if a patron has cancelled the hold it probably is still being transferred to it's original pickup location and then people will check it in there and if it updated the frombranch at that point then when the item returns back home it would have a proper message where it came from
15:37 kohaputti but if it doesn't update the frombranch to the transfer the message will be weird "transfer to branch A from branch A"
15:38 ashimema when will you see that message?
15:38 ashimema remember it's queued..
15:38 ashimema hum...
15:38 ashimema get_transfer and get_transfers will return a different transfer at the top
15:38 kohaputti not sure, just this is what the librarian reported
15:39 ashimema get_transfer will return the un-cancelled return transfer
15:39 ashimema let me run through and test it..
15:39 ashimema I've not actually had a moment go test that code I uploaded yet .. haha
15:40 kohaputti ashimema, the code actually doesn't work right because in "+ if ( defined($transfer) && $transfer->in_transit ) {"
15:40 kohaputti it requires the thing to be in-transit
15:41 kohaputti but these transfers generated by hold cancellation are not in_transit
15:41 ashimema that's what the change to get_transfers does ;)
15:41 ashimema get_transfers returns a list in modified fifo order..
15:41 kohaputti I know, but because of that && $transfer->in_transit part it doesn't get displayed
15:41 ashimema it'll return 'in_transit' first always (including cancelled whilst in transit)
15:42 ashimema I also modified 'in_transit' in that patch ;)
15:42 ashimema to ignore cancelleddate
15:42 ashimema so in_transit should be true
15:42 kohaputti datesent is null
15:42 ashimema wha...
15:42 ashimema using your example
15:43 kohaputti the hold has been just cancelled, nobody has yet checked the item in, it is still in the transportation van
15:43 ashimema get_transfers should return transfer 1 at the top
15:43 ashimema which has a transfer
15:43 kohaputti so the datesent is null
15:44 kohaputti ooh... hmm
15:44 kohaputti does it show the old transfer?
15:44 kohaputti wait..
15:44 kohaputti maybe it does
15:44 kohaputti sorry
15:44 ashimema yes
15:44 ashimema :)
15:44 kohaputti so confusing :D
15:44 ashimema yeah..
15:45 ashimema I'm not sure about it..
15:45 ashimema though I think the logic is mostly sound now.. it feels a bit dirty and confusing
15:46 ashimema aha..
15:46 ashimema I think I have a nicer solution.. that ditches the immediate creation of the return transfer again
15:46 ashimema that's cleaner
15:46 kohaputti when will it do it?
15:46 ashimema at the checkin
15:46 ashimema like you suggested
15:47 kohaputti ok! I actually worked on another bug relating to this and wanted to do it on check-in back then as well
15:47 ashimema the core thing is.. we need to check get_transfers instead of get_transfer
15:47 ashimema haha
15:47 ashimema cool
15:47 ashimema I just couldn't work out how to before..
15:47 ashimema light bulb moment
15:48 kohaputti I didn't work on it more because I thought bug 12362 solved it
15:48 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12362 normal, P5 - low, ---, martin.renvoize, Pushed to master , Branch transfer records orphaned on cancelled holds
15:48 kohaputti hopefully this check-in idea will work
15:49 ashimema I think Petro confused me a while back.. I can't have understood something he said
15:49 ashimema it's all coming back now :)
15:50 kohaputti ashimema, maybe it is actually best we require the transfer to go to it's destination and then come back, because you know, you don't have to do that physically, the librarian can just juggle the branches in intranet to fake the book has traveled the world :)
15:50 kohaputti and, that was the old behaviour before bug 12362
15:50 ashimema agreed
15:50 Dyrcona eythian: I found that metafilter link and the surrounding discussion to be of interest. Personally Identifiable information was covered in the keynote and a lightning talk at this year's Evergreen conference.
15:50 kohaputti though before the triggering to back home didn't work well
15:51 kohaputti you had to check-in twice at the cancelled hold's pickup branch to make it go back home
15:57 kohaputti ashimema, I'm actually bit unsure what is your idea and how it would work. My current idea would be to keep the transfer and not cancel it and on destination check-in check whether there is still an T type reserve for the item if the transfer reason = "Reserve"
15:58 kohaputti and if there is no reserve then create a new transfer back home
15:58 ashimema My idea is to 'cancel' the transfer by setting the cancellation_reason and date.. but not create drop the creation of the return transfer at that point..
15:59 kohaputti but then we have no transfer?
15:59 ashimema instead.. call get_transfers (with the updated logic that returns 'in_transit' at the top, regardless of whether it was cancelled or not
15:59 kohaputti ah, ok
15:59 ashimema then check for cancellation_reason (or cancellationdate)
15:59 ashimema and if found, add the return transfer at that point
15:59 ashimema or rather.. ask the librarian if they want to add it.
16:00 kohaputti hmm.. and ok, then the in_transit check would be back to the previous
16:00 kohaputti return ( defined( $self->datesent )
16:00 kohaputti && !defined( $self->datearrived )
16:00 ashimema that way we catch your case for hold cancellations.. but also catch other cancellation reasons.
16:00 ashimema exactly
16:00 kohaputti we would drop the && !defined( $self->datecancelled ) );
16:00 kohaputti from in_transit
16:00 ashimema I think that's the addition that really screwed the pooch as it were
16:00 ashimema that was added at Patro's request.. well.. I thought it made sense and it flew through QA :P
16:01 ashimema clearly I didn't fully anticipate the circular dependancy it left us iwth
16:09 kohaputti ashimema, hmm, I think I found the culprit for the original problem actually. Do you see how the $validTransfer variable is set in the Circulation.pm code, it disabled the return to home branch on check-in
16:09 kohaputti let me see if git blame tells us more
16:12 kohaputti Bug 26078: (follow-up) Use a boolean rather than changing returnbranch
16:12 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26078 major, P5 - low, ---, nick, Pushed to oldstable , "Item returns to issuing library" creates infinite transfer loop
16:12 kohaputti this seems to have changed the behaviour
16:12 kohaputti it also added the comment:
16:12 kohaputti + # At this point we will either fill the transfer or it is a wrong transfer
16:12 kohaputti + # either way we should not now generate a new transfer
16:13 kohaputti but seems like it broke this workflow
16:22 kohaputti ashimema, maybe just set $validTransfer = 1 if we come across canceled transfer which is not yet arrived?
16:22 kohaputti and also revert the is_transit logic
16:23 ashimema something along those lines
16:23 ashimema gotta cook tea and take kids to clubs
16:23 ashimema will continue on this, this evening
16:23 kohaputti ok, I will also go now, see you tomorrow
16:25 ashimema Have a good evening
16:51 nugged > gotta cook tea and take kids to clubs
16:51 nugged ashimema: 🤗
16:57 oleonard ashimema++
16:57 oleonard kohaputti++
17:14 kohaputti ashimema, I just pushed some patches and it fixes the triggering of transfer back home, the patches depend on the bug 27064 we just worked earlier today on.
17:14 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27064 normal, P5 - low, ---, nick, Passed QA , Transferring an item with a hold allows the user to set a hold waiting without transferring to the correct branch
17:14 kohaputti ashimema, however, I don't know how the stockrotation feature works with this...
17:16 ashimema are those patches you pushed public somewhere?
17:18 kohaputti bug 28520
17:18 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28520 critical, P5 - low, ---, koha-bugs, NEW , Cancelling a hold that is in transit hides item's transit status
17:18 ashimema yup.. found them
17:18 kohaputti I think it requires probably the adaptation to the stockrotation thing but maybe you could have a look?
17:18 ashimema I find the 'validTransfer' stuff confusing.. it's been a while since I wrapped my head around that
17:19 ashimema I like your patches though.. they progress where I was going nicely
17:19 kohaputti ashimema, can you tell me why stockrotation cancels transfers?
17:20 kohaputti where are the items supposed to be after they are cancelled, like can we just finish the transfer instead?
17:21 ashimema erm
17:21 kohaputti oh, we need a patch to rename that "replace" parameter btw, it is not very descriptive
17:21 ashimema I don't see where stockrotation cancels transfers..
17:21 ashimema yeah.
17:21 kohaputti let me find it, I think it did that
17:21 ashimema ack.. i have kids screaming at me now
17:22 ashimema or do you mean rotating collections
17:22 ashimema they're pretty different features
17:23 kohaputti ashimema, It is this rotating colleciton
17:23 kohaputti ah ok, I thought they were the same
17:23 kohaputti both have collection in the name :DD
17:24 kohaputti in RotatingCollections.pm there is:
17:24 kohaputti my $transfer = $item_object->request_transfer(
17:24 kohaputti which passes replace => 1 which means we try cancel the transfer
17:29 kohaputti There is also one call with replace => 1 in StockRotationItem.pm and ModItemTransfer.pm
17:29 kohaputti so total we have 3 places we need to check whether it is okay or need to be re-written some other way
17:31 kohaputti aha, in ModItemTransfer we can revert to the previous behaviour, which is to set the transfer arrived
17:32 kohaputti (that was done in bug Bug 24446)
17:32 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24446 enhancement, P5 - low, ---, martin.renvoize, Pushed to master , Stockrotation: Update to use daterequested in branchtransfers
17:38 ashimema ok
17:38 ashimema will take another look this evening
18:08 kohaputti ashimema, we could also just revert the "Bug 12362: Cancel transfer with hold cancelation" where the transfer is cancelled, to restore the previous behaviour which a lot better than having incorrect availability info
18:08 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12362 normal, P5 - low, ---, martin.renvoize, Pushed to master , Branch transfer records orphaned on cancelled holds
18:11 kohaputti let me do that actually and send patches..
18:11 kohaputti I can also do the fix by checking if there is any hold to full-fill
18:12 kohaputti trying to change the meaning of that cancellationdate is a slippery slope and could probably cause more regressions
18:12 kohaputti so it needs to be done with a good amount of time and in small pieces
18:14 paxed /win 14
18:14 paxed oops
19:01 kohaputti ashimema, found the perfect patch, attached it in the report :)
19:04 cait1 "the perfect patch"
19:05 cait1 :)
19:05 cait1 kohaputti++ ashimema++
19:05 cait1 bye :)
23:23 dcook Thanks, Joubu. I've been wanting to do bug 28519 for years, so I'm glad I finally had the motivation.
23:23 huginn Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28519 enhancement, P5 - low, ---, dcook, Needs Signoff , Add a 2nd directory for Perl modules