(In reply to Marcel de Rooy from comment #0)
> Note that Koha still is a bit hybrid to determine if an order has been
> cancelled: we have orderstatus and we have datecancellationprinted..

And in production data I come across both flavors:
status=ordered and date cancellation filled
status=cancelled and date is not filled.

Hm, I would assume this should not happen with "today's" code, but could be from earlier versions.

 Can you maybe tell form the dates/ordernumbers if this is older or more recnt? 
Or could it be migration/imported data?

(In reply to Katrin Fischer from comment #2)
> Hm, I would assume this should not happen with "today's" code, but could be
> from earlier versions.
> 
>  Can you maybe tell form the dates/ordernumbers if this is older or more
> recnt? 
> Or could it be migration/imported data?

No migrated data.
Yes, it seems that by far most occurrences are from 5-10 years ago. But I think that I had at least a few more recent.
Many installs will probably have such historical cases and this script lets you cleanup.

Created attachment 162261 [details] [review]
Bug 36068: Add maint script acq_cancel_obsolete_orders.pl

Introducing $orders->filter_by_obsolete and $orders->cancel.

Test plan:
Run t/db_dependent/Koha/Acquisition/Orders.t
Create basket with few orders. Remove some biblio records.
Run acq_cancel_obsolete_orders.pl -c

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Created attachment 162262 [details] [review]
Bug 36068: Do not overwrite cancellation date in ->cancel

Test plan:
Run t/db_dependent/Koha/Acquisition/Order.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Created attachment 163721 [details] [review]
Bug 36068: Add maint script acq_cancel_obsolete_orders.pl

Introducing $orders->filter_by_obsolete and $orders->cancel.

Test plan:
Run t/db_dependent/Koha/Acquisition/Orders.t
Create basket with few orders. Remove some biblio records.
Run acq_cancel_obsolete_orders.pl -c

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 163722 [details] [review]
Bug 36068: Do not overwrite cancellation date in ->cancel

Test plan:
Run t/db_dependent/Koha/Acquisition/Order.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 165000 [details] [review]
Bug 36068: Add maint script acq_cancel_obsolete_orders.pl

Introducing $orders->filter_by_obsolete and $orders->cancel.

Test plan:
Run t/db_dependent/Koha/Acquisition/Orders.t
Create basket with few orders. Remove some biblio records.
Run acq_cancel_obsolete_orders.pl -c

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 165001 [details] [review]
Bug 36068: Do not overwrite cancellation date in ->cancel

Test plan:
Run t/db_dependent/Koha/Acquisition/Order.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

All works as described.. QA scripts are happy and new methods are tested.

Passing QA

Hi Marcel, trying to understand the logic here I stumbled on this line:

+    [1] Order lines that have no biblio anymore but are still considered open
+        (received < ordered, not cancelled).

Can you please explain especially the second part?

Similar here:

+    [1] Biblionumber is null but received < ordered and not cancelled.

Thanks for writing a lot of documentation on these scripts.

A note: -c is logical, but often reserves for "confirm". Should we maybe switch it?

./misc/maintenance/acq_cancel_obsolete_orders.pl -h

Gives me the documentation, but it doesn't contain the typical list of parameter the script accepts. I think it would be nice to be consistent here with the other scripts.

(In reply to Katrin Fischer from comment #13)
> Hi Marcel, trying to understand the logic here I stumbled on this line:
> 
> +    [1] Order lines that have no biblio anymore but are still considered
> open
> +        (received < ordered, not cancelled).
> 
> Can you please explain especially the second part?

The biblio has been already deleted. But somehow we had order lines that were still 'open'. They were not cancelled. (Note that Koha is confused about that status because we have two fields: datecancellationprinted and status.)
What also was possible (perhaps by older bugs) is quantity undefined or received < quantity.

(In reply to Katrin Fischer from comment #13)
> Similar here:
> 
> +    [1] Biblionumber is null but received < ordered and not cancelled.

Thats referring to the same. The script calls the module.

(In reply to Katrin Fischer from comment #13)
> A note: -c is logical, but often reserves for "confirm". Should we maybe
> switch it?

+GetOptions(
+    'confirm' => \$params->{confirm}, 'help' => \$params->{help}, 'age:i' => \$params->{age},
+);

The c stands for confirm but it is confirming a cancellation. Script name is helpful too: acq_cancel_obsolete_orders.pl.

(In reply to Katrin Fischer from comment #13)
> ./misc/maintenance/acq_cancel_obsolete_orders.pl -h
> 
> Gives me the documentation, but it doesn't contain the typical list of
> parameter the script accepts. I think it would be nice to be consistent here
> with the other scripts.

Will have a look.

Created attachment 165113 [details] [review]
Bug 36068: (QA follow-up) Add OPTIONS section in POD

Test plan:
Run script with --help.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

I think my issue is, I don't understand your syntax here:

(received < ordered, not cancelled).

How do you read the < ?

(In reply to Katrin Fischer from comment #19)
> I think my issue is, I don't understand your syntax here:
> 
> (received < ordered, not cancelled).
> 
> How do you read the < ?

Simply as lower than. The number of received items is lower than the number ordered :)

Pushed for 24.05!

Well done everyone, thank you!

Not backported to 23.11.x