Bug 16055

Summary:	Deleting a basket group containing baskets fails silently
Product:	Koha	Reporter:	Owen Leonard <oleonard>
Component:	Acquisitions	Assignee:	Jonathan Druart <jonathan.druart>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	brendan, jonathan.druart, julian.maurice
Version:	Main
Hardware:	All
OS:	All
URL:	/cgi-bin/koha/acqui/basketgroup.pl
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16038 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11708
Change sponsored?:	---	Patch complexity:	Trivial patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Bug 16055: Do not allow basketgroup deletion unless empty [SIGNED-OFF] Bug 16055: Do not allow basketgroup deletion unless empty [PASSED QA] Bug 16055: Do not allow basketgroup deletion unless empty

Description Owen Leonard 2016-03-11 17:51:08 UTC

In the list of basket groups for a vendor there is a "delete" option for each basket group whether or not the basket group contains baskets. If you choose "delete" for a basket group containing a basket it will fail silently and an error will be logged:

DBD::mysql::st execute failed: Cannot delete or update a parent row: a foreign key constraint fails (`aqbasket`, CONSTRAINT `aqbasket_ibfk_3` FOREIGN KEY (`basketgroupid`) REFERENCES `aqbasketgroups` (`id`) ON UPDATE CASCADE) [for Statement "DELETE FROM aqbasketgroups WHERE id=?" with ParamValues: 0="3"]

Since the template knows the number of baskets in a basket group it should be easy to hide or disable the delete button in those cases.

It also would be nice to add a JavaScript confirmation before deleting a basket. Right now deletions are completed without any warning.

Keep in mind the work being done in Bug 16038 and Bug 11708 when considering this one.

Comment 1 Jonathan Druart 2016-04-05 08:16:05 UTC Comment hidden (obsolete)

Created attachment 49897 [details] [review]
Bug 16055: Do not allow basketgroup deletion unless empty

If a basketgroup has basket attached, it should not be deletable.
This patch just removes the Delete button from the interface if it
cannot be deleted.
When editing a basketgroup, the "Delete basket group" button is no longer
displayed.

Test plan:
1/ Create a basketgroup
2/ Attach 1+ baskets to this basketgroup
3/ Confirm you are not able to delete it
4/ Remove the baskets from this basketgroup
5/ Confirm you are able to delete it

Comment 2 Jonathan Druart 2016-04-05 08:17:21 UTC

(In reply to Owen Leonard from comment #0)
> Keep in mind the work being done in Bug 16038 and Bug 11708 when considering
> this one.

One is blocked and the other one does not apply. Moreover this is a bug fix and it could be great to backport it.
I think this patch should be pushed before the 2 others.

Comment 3 Owen Leonard 2016-04-05 18:35:09 UTC Comment hidden (obsolete)

Created attachment 49925 [details] [review]
[SIGNED-OFF] Bug 16055: Do not allow basketgroup deletion unless empty

If a basketgroup has basket attached, it should not be deletable.
This patch just removes the Delete button from the interface if it
cannot be deleted.
When editing a basketgroup, the "Delete basket group" button is no
longer displayed.

Test plan:
1/ Create a basketgroup
2/ Attach 1+ baskets to this basketgroup
3/ Confirm you are not able to delete it
4/ Remove the baskets from this basketgroup
5/ Confirm you are able to delete it

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Comment 4 Katrin Fischer 2016-04-05 21:44:32 UTC

Should we have icons on those buttons? If yes... please add on another bug :)

Comment 5 Katrin Fischer 2016-04-05 21:45:45 UTC

Created attachment 49932 [details] [review]
[PASSED QA] Bug 16055: Do not allow basketgroup deletion unless empty

If a basketgroup has basket attached, it should not be deletable.
This patch just removes the Delete button from the interface if it
cannot be deleted.
When editing a basketgroup, the "Delete basket group" button is no
longer displayed.

Test plan:
1/ Create a basketgroup
2/ Attach 1+ baskets to this basketgroup
3/ Confirm you are not able to delete it
4/ Remove the baskets from this basketgroup
5/ Confirm you are able to delete it

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Comment 6 Brendan Gallagher 2016-04-20 20:30:04 UTC

Pushed to Master - Should be in the May 2016 Release.  Thanks!

Comment 7 Julian Maurice 2016-04-21 08:06:38 UTC

Patch pushed to 3.22.x, will be in 3.22.6