Sometimes an order is just a mistake (double entry or similar) and it should be possible to delete those entries entirely from the database so they won't show up in any reports.
Our Acquisitions Assistant confirms that this would be a welcome improvement / bugfix. Let us know if we can help with sign-off. -Sebastian
One of our partners just told me that she would love to have the ability to delete an order entry. She says,"What KOHA needs is a delete order line feature. That is quite different from cancelling an order. I guess because we are small in comparison to many libraries, keeping records of cancelled orders is not important to us and gets in the way actually."
*** Bug 14091 has been marked as a duplicate of this bug. ***
See bug 36066 too. We only allow API to delete cancelled lines there. This bug is for the interface.
Created attachment 162294 [details] [review] Bug 18360: Allow to delete a cancelled order from open basket Doing this under flag of order_manage and not on the higher level of baskets. Starting here with a single order delete for cancelled lines. So deleting an open order line requires cancelling it first. Can be extended later if needed. Test plan: Pick an open basket with cancelled lines. Try to delete a few lines. Test that you cannot do this on an closed basket. (Bonus: manipulate URL with basketno, ordernumber and op=cud-delete-order.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Katrin Fischer from comment #0) > Sometimes an order is just a mistake (double entry or similar) and it should > be possible to delete those entries entirely from the database so they won't > show up in any reports. The current proposal does build on this use case and just starts with only allowing deletion of cancelled order lines without a biblio record. Can be extended later on a follow-up if needed. First try to get this moving.
Hi Marcel, thanks for the patch. I tested it in Sandbox http://staff-b18360.sandboxes.ptfs-europe.co.uk/ I'm not sure if I missed something but the Link to delete the cancelled lines only appears if the basket is open. was that the intention? With Bug 33664 the ability to cancel order lines in closed baskets was added. I think it would make sense to be able to delete the line even if the basket is closed.
(In reply to Michaela Sieber from comment #7) > Hi Marcel, > thanks for the patch. > I tested it in Sandbox > http://staff-b18360.sandboxes.ptfs-europe.co.uk/ > > I'm not sure if I missed something but the Link to delete the cancelled > lines only appears if the basket is open. > > was that the intention? > > With Bug 33664 the ability to cancel order lines in closed baskets was added. > I think it would make sense to be able to delete the line even if the basket > is closed. Hi Michaela, Yes that was the intention. Normally we can only edit a basket and its lines when we reopen. Was not aware of 33664. Making things inconsistent imo.. Prefer to keep this as is now here.
Created attachment 162484 [details] [review] Bug 18360: Allow to delete a cancelled order from open basket Doing this under flag of order_manage and not on the higher level of baskets. Starting here with a single order delete for cancelled lines. So deleting an open order line requires cancelling it first. Can be extended later if needed. Test plan: Pick an open basket with cancelled lines. Try to delete a few lines. Test that you cannot do this on an closed basket. (Bonus: manipulate URL with basketno, ordernumber and op=cud-delete-order.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
(In reply to PTFS Europe Sandboxes from comment #9) > Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> > Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Thanks Michaela
Created attachment 165273 [details] [review] Bug 18360: Allow to delete a cancelled order from open basket Doing this under flag of order_manage and not on the higher level of baskets. Starting here with a single order delete for cancelled lines. So deleting an open order line requires cancelling it first. Can be extended later if needed. Test plan: Pick an open basket with cancelled lines. Try to delete a few lines. Test that you cannot do this on an closed basket. (Bonus: manipulate URL with basketno, ordernumber and op=cud-delete-order.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Tiny rebase (CSRF)
Created attachment 165525 [details] [review] Bug 18360: Allow to delete a cancelled order from open basket Doing this under flag of order_manage and not on the higher level of baskets. Starting here with a single order delete for cancelled lines. So deleting an open order line requires cancelling it first. Can be extended later if needed. Test plan: Pick an open basket with cancelled lines. Try to delete a few lines. Test that you cannot do this on an closed basket. (Bonus: manipulate URL with basketno, ordernumber and op=cud-delete-order.) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Clear simple code, QA scripts happy and all working. Whilst it regresses bug 33664 I think that's OK here and brings more consistency. Passing QA
(In reply to Martin Renvoize from comment #14) > Whilst it regresses bug 33664 I think that's OK here and brings more > consistency. Could you explain? The other bug is about cancelling. This one about delete? > Passing QA Thanks
(In reply to Marcel de Rooy from comment #15) > (In reply to Martin Renvoize from comment #14) > > > Whilst it regresses bug 33664 I think that's OK here and brings more > > consistency. > > Could you explain? The other bug is about cancelling. This one about delete? > > > Passing QA > > Thanks From Mattermost: ashimema: About having to re-open the basket you already noted it with Micheala and she signed off so I felt it was OK marcelr Ah ok not really a regression. But perhaps inconsistent? Though I like the idea of reopening before deleting personally.
I think we might want to discuss the open/closed basket again, but this is progress and I love it :)
Taking a closer look - maybe also to be handled on a separate bug: Why is it necessary to delete the record first? I could imagine libraries still having the record with older items or other linked orders. + alert( _("Please delete the linked bibliographic record first.") );
Sorry, this needs a follow-up for CSRF: Programming error - No CSRF token passed for POST http://localhost:8081/intranet/acqui/basket.pl?basketno=1 (referer: http://localhost:8081/cgi-bin/koha/acqui/basket.pl?basketno=1)
(In reply to Katrin Fischer from comment #18) > Taking a closer look - maybe also to be handled on a separate bug: > > Why is it necessary to delete the record first? I could imagine libraries > still having the record with older items or other linked orders. > > + alert( _("Please delete the linked bibliographic > record first.") ); Probably had in mind to prevent 'ghost records' in the catalog. Created by Acquisition and now having no links anymore. Could imagine that there will be different opinions here. Maybe still move that to a new report?
(In reply to Marcel de Rooy from comment #20) > (In reply to Katrin Fischer from comment #18) > > Taking a closer look - maybe also to be handled on a separate bug: > > > > Why is it necessary to delete the record first? I could imagine libraries > > still having the record with older items or other linked orders. > > > > + alert( _("Please delete the linked bibliographic > > record first.") ); > > Probably had in mind to prevent 'ghost records' in the catalog. Created by > Acquisition and now having no links anymore. > Could imagine that there will be different opinions here. > Maybe still move that to a new report? Yes, it was not meant as a blocker here. Just wondering about reasoning. I'll add additional_work_needed as a reminder for this loose end.
Created attachment 165869 [details] [review] Bug 18360: (follow-up) Fix for CSRF Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Katrin Fischer from comment #21) > (In reply to Marcel de Rooy from comment #20) > > (In reply to Katrin Fischer from comment #18) > > > Taking a closer look - maybe also to be handled on a separate bug: > > > > > > Why is it necessary to delete the record first? I could imagine libraries > > > still having the record with older items or other linked orders. > > > > > > + alert( _("Please delete the linked bibliographic > > > record first.") ); > > > > Probably had in mind to prevent 'ghost records' in the catalog. Created by > > Acquisition and now having no links anymore. > > Could imagine that there will be different opinions here. > > Maybe still move that to a new report? > > Yes, it was not meant as a blocker here. Just wondering about reasoning. > I'll add additional_work_needed as a reminder for this loose end. Bug 36731
Pushed for 24.05! Well done everyone, thank you!
Not backported to 23.11.x