Bug 28028

Looking through the history of this script.. I'm not sure it's actually done anything for a while?

It appears that it used to update the MARC record from the items table onloan field.. I don't think store does that.. or am I missing something?

Created attachment 119365 [details] [review]
Bug 28028: (bug 23463 follow-up) Fix fix_onloan.pl

There is an obvious error in the script, the name of the method is
missing
Undefined subroutine &Koha::Items called at misc/migration_tools/fix_onloan.pl line 13.

Test plan:
Run the script and confirm that it does not explode

Created attachment 119432 [details] [review]
Bug 28028: (bug 23463 follow-up) Fix fix_onloan.pl

There is an obvious error in the script, the name of the method is
missing
Undefined subroutine &Koha::Items called at misc/migration_tools/fix_onloan.pl line 13.

Test plan:
Run the script and confirm that it does not explode

Signed-off-by: David Nind <david@davidnind.com>

As Martin stated earlier also, this script doesn't do anything. If I'm not mistaken the whole script can be now removed because bug 5579 is merged in and we don't store items anymore in MARC record format.

(In reply to Joonas Kylmälä from comment #4)
> As Martin stated earlier also, this script doesn't do anything. If I'm not
> mistaken the whole script can be now removed because bug 5579 is merged in
> and we don't store items anymore in MARC record format.

I believe this script might be for updating items.onloan from issues.date_due, so it would still makes total sense for migrations. You migrate the items... you migrate the issues and then update the items.onloan for the issued items.

Maybe just the script description doc is a bit off?

Yes, we may well want it to update items.on_loan.. but I don't think the store call does that any more.. so although this patch lets the script run.. I don't know that it actually makes it work as it used to.

I need to dig a little further unless someone has the time to test.

Katrins' correct though.. she's identified the exact use case where I think we were using it in the past... at migration time.

Hmm, strange:

If I look ModItem() subroutine on revision 0c8d8e867b18b61f39381d2061cd2124fe1c5ed3 (which is just before it was deleted from codebase it has not fetched issues.date_due value as far as I can see, I checked all the code paths in ModItem() to my best ability.

So this would conflict with what both Martin and Katrin are remembering.

(In reply to Joonas Kylmälä from comment #7)
> Hmm, strange:
> 
> If I look ModItem() subroutine on revision
> 0c8d8e867b18b61f39381d2061cd2124fe1c5ed3 (which is just before it was
> deleted from codebase it has not fetched issues.date_due value as far as I
> can see, I checked all the code paths in ModItem() to my best ability.
> 
> So this would conflict with what both Martin and Katrin are remembering.

Yup, I now checked out Koha version just before Bug 23463 was merged:

$ git reset --hard f04e77c7e5411ca7dcf761e7e1177d7362a10654

and the fix_onloan.pl script didn't update the items.onloan value to be same as in issues table.

Here is the steps I used to test this:

> $ git reset --hard f04e77c7e5411ca7dcf761e7e1177d7362a10654
> $ reset_all
> # Checkout an item to patron and then:
> $ koha-mysql kohadev
> > update items SET onloan = "2020-01-01" WHERE itemnumber=549
> $ koha-shell kohadev
> $ (inside koha-shell) perl fix_onloan.pl
> $ koha-mysql kohadev
> > select * from items WHERE itemnumber=549 # and notice it didn't update

So I strongly think this script has been doing nothing after bug 5579 has been merged and we stopped storing items in MARC format.

Created attachment 119607 [details] [review]
Bug 28028: Remove misc/migration_tools/fix_onloan.pl

There is an obvious error in the script since bug 23463 and it's known
to be broken for 10 years now (since bug 5579). As nobody complains we
can safely remove this script.

Thanks Joonas for the investigation. Let remove it then!

Created attachment 119608 [details] [review]
Bug 28028: Remove misc/migration_tools/fix_onloan.pl

There is an obvious error in the script since bug 23463 and it's known
to be broken for 10 years now (since bug 5579). As nobody complains we
can safely remove this script.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Created attachment 119766 [details] [review]
Bug 28028: Remove misc/migration_tools/fix_onloan.pl

There is an obvious error in the script since bug 23463 and it's known
to be broken for 10 years now (since bug 5579). As nobody complains we
can safely remove this script.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Totally ok with removing the script, but we need to remember to update bug descriptions when changing direction :)

Pushed to master for 21.05, thanks to everybody involved!

Pushed to 20.11.x for 20.11.06

Pushed to 20.05.x for 20.05.12

Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.