Bug 16015 - Cannot delete a group of libraries
Summary: Cannot delete a group of libraries
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 16091 (view as bug list)
Depends on: 15294
Blocks:
  Show dependency treegraph
 
Reported: 2016-03-09 12:22 UTC by Jonathan Druart
Modified: 2019-06-27 09:24 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16015: Restore the ability to delete a group of libraries (1.26 KB, patch)
2016-03-09 12:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16015: Restore the ability to delete a group of libraries (1.25 KB, patch)
2016-03-11 07:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16015: Restore the ability to delete a group of libraries (1.31 KB, patch)
2016-03-13 23:44 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
[PASSED QA] Bug 16015: Restore the ability to delete a group of libraries (1.37 KB, patch)
2016-03-20 15:55 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-03-09 12:22:19 UTC

    
Comment 1 Jonathan Druart 2016-03-09 12:24:38 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2016-03-11 03:06:20 UTC
This is totally weird. It must be that $libraries_count gets interpreted as "0". Perl, I did not expect that from you.
Anyway, although this patch does solve the issue, pedantic devil in me says - wait.
The way it is written says
my $libraries_count = (scalar( $category->libraries ) > 0)
which does yield correct boolean, but evaluates to 1 no matter how many libraries are there.
Correct way could be
my $libraries_count = scalar( $category->libraries ) + 0
or
(my $libraries_count = scalar( $category->libraries )) > 0
Comment 3 Jonathan Druart 2016-03-11 07:58:49 UTC
(In reply to Srdjan Jankovic from comment #2)
> This is totally weird. It must be that $libraries_count gets interpreted as
> "0". Perl, I did not expect that from you.
> Anyway, although this patch does solve the issue, pedantic devil in me says
> - wait.
> The way it is written says
> my $libraries_count = (scalar( $category->libraries ) > 0)
> which does yield correct boolean, but evaluates to 1 no matter how many
> libraries are there.
> Correct way could be
> my $libraries_count = scalar( $category->libraries ) + 0
> or
> (my $libraries_count = scalar( $category->libraries )) > 0

Do not worry, what you read is confusing :)
$category->libraries returns, in an array context, an array of DBIx::Class objects.
But, in a scalar context, an ... iterator.
So it's always defined.
I will submit a patch to clarify the changes and avoid any confusions later.
Comment 4 Jonathan Druart 2016-03-11 07:59:48 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-03-11 08:00:12 UTC
+    if ( my $libraries_count = $category->libraries->count ) {

is definitely the way to do :)
Comment 6 Srdjan Jankovic 2016-03-13 23:44:05 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-03-18 16:46:39 UTC
*** Bug 16091 has been marked as a duplicate of this bug. ***
Comment 8 Jonathan Druart 2016-03-18 16:47:43 UTC
Raising the severity as it's a regression.
Comment 9 Katrin Fischer 2016-03-20 15:53:15 UTC
The patch fixes the problem, but the error/message dialogs are not looking quite right (maybe something for Owen/Aleisha :) )

Are you sure you want to delete the group '' (TEST)?
Comment 10 Katrin Fischer 2016-03-20 15:55:12 UTC
Created attachment 49331 [details] [review]
[PASSED QA] Bug 16015: Restore the ability to delete a group of libraries

If a group of libraries is linked to at least 1 library, the group
cannot be delete and the librarian should get a warning message.
But if no libraries are linked, the deletion should be allowed.
Since bug 15294 this behavior is broken: The deletion is always blocked:
"This library category cannot be deleted. 0 libraries are still using
it", hum...

Test plan:
Create a group of libraries, delete it

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Brendan Gallagher 2016-03-21 18:27:46 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks!
Comment 12 Julian Maurice 2016-03-23 07:12:12 UTC
Does not concern stable versions, status changed to RESOLVED