| Time |
S |
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.or[…]_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 |
| 05:03 |
|
|
chriss joined #koha |
| 05:58 |
|
|
magnuse joined #koha |
| 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:14 |
|
|
cait joined #koha |
| 06:15 |
|
|
cait1 joined #koha |
| 06:28 |
|
cait1 |
kidclamp: khall: bywater solutions demo is down |
| 06:28 |
|
cait1 |
https://catalog.bywatersolutions.com/ |
| 06:29 |
|
magnuse |
\o/ |
| 06:35 |
|
|
reiveune joined #koha |
| 06:35 |
|
reiveune |
hello |
| 06:40 |
|
|
lds joined #koha |
| 06:40 |
|
* dcook |
waves |
| 06:44 |
|
|
fridolin joined #koha |
| 06:45 |
|
fridolin |
hi |
| 06:52 |
|
|
cait joined #koha |
| 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/view[…]?pageId=223543412 |
| 06:53 |
|
|
alex_a joined #koha |
| 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 |
|
|
sophie_m joined #koha |
| 06:56 |
|
cait1 |
we have one 21.05 already frm Admin_Kuhn++ |
| 06:57 |
|
|
fridolin1 joined #koha |
| 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:07 |
|
|
sophie_m joined #koha |
| 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:21 |
|
|
Joubu joined #koha |
| 07:22 |
|
Joubu |
thx dcook for bug 28519 |
| 07:22 |
|
huginn |
Bug https://bugs.koha-community.or[…]_bug.cgi?id=28519 enhancement, P5 - low, ---, dcook, Needs Signoff , Add a 2nd directory for Perl modules |
| 07:27 |
|
|
paul_p joined #koha |
| 07:29 |
|
|
nlegrand joined #koha |
| 07:29 |
|
nlegrand |
Bonjour #koha ! |
| 08:19 |
|
magnuse |
kia ora nlegrand |
| 08:24 |
|
nlegrand |
o/ magnuse |
| 08:27 |
|
|
cait joined #koha |
| 08:28 |
|
|
cait2 joined #koha |
| 08:30 |
|
ashimema |
morning #koha |
| 08:33 |
|
ashimema |
looks like I have lots of catching up to do.. |
| 08:38 |
|
|
alex_a joined #koha |
| 08:48 |
|
|
kohaputti joined #koha |
| 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.or[…]_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:04 |
|
|
davidnind joined #koha |
| 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:41 |
|
|
petrova joined #koha |
| 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.or[…]_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:39 |
|
|
khall joined #koha |
| 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:49 |
|
|
khall_ joined #koha |
| 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:13 |
|
|
davidnind left #koha |
| 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.or[…]_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:00 |
|
|
sophie_m joined #koha |
| 12:01 |
|
|
paul_p joined #koha |
| 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:31 |
|
|
davidnind joined #koha |
| 12:34 |
|
|
Dyrcona joined #koha |
| 12:38 |
|
kohaputti |
ashimema, reverse transfer is ? |
| 12:41 |
|
kohaputti |
https://bugs.koha-community.or[…].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 |
|
|
collum joined #koha |
| 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:14 |
|
|
khall joined #koha |
| 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.or[…]_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:32 |
|
|
theodoros joined #koha |
| 13:48 |
|
|
khall_ joined #koha |
| 13:57 |
|
kohaputti |
ashimema, https://bugs.koha-community.or[…]21674&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:05 |
|
|
henryb joined #koha |
| 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:31 |
|
|
lukeG1 joined #koha |
| 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.or[…]_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:39 |
|
|
lds_ joined #koha |
| 14:39 |
|
|
fridolin1 left #koha |
| 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/191[…]vacy-in-libraries perhaps interesting for people here |
| 15:17 |
|
kohaputti |
ok, trying to understand this now :) |
| 15:19 |
|
|
cait joined #koha |
| 15:20 |
|
|
cait1 joined #koha |
| 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.or[…]_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.or[…]_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.or[…]_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:35 |
|
|
reiveune left #koha |
| 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.or[…]_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:52 |
|
|
davidnind joined #koha |
| 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:07 |
|
|
khall joined #koha |
| 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.or[…]_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:34 |
|
|
collum joined #koha |
| 16:49 |
|
|
khall_ joined #koha |
| 16:51 |
|
nugged |
> gotta cook tea and take kids to clubs |
| 16:51 |
|
nugged |
ashimema: 🤗 |
| 16:57 |
|
oleonard |
ashimema++ |
| 16:57 |
|
|
collum joined #koha |
| 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.or[…]_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.or[…]_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 |
|
|
reiveune joined #koha |
| 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:22 |
|
|
reiveune left #koha |
| 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.or[…]_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 |
| 17:59 |
|
|
collum joined #koha |
| 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.or[…]_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 |
| 18:18 |
|
|
cait joined #koha |
| 18:19 |
|
|
cait1 joined #koha |
| 18:24 |
|
|
marie-luce joined #koha |
| 18:28 |
|
|
davidnind left #koha |
| 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 :) |
| 19:05 |
|
|
cait1 left #koha |
| 19:05 |
|
|
cait joined #koha |
| 19:19 |
|
|
lukeG joined #koha |
| 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.or[…]_bug.cgi?id=28519 enhancement, P5 - low, ---, dcook, Needs Signoff , Add a 2nd directory for Perl modules |
