[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Katrin Fischer  changed:

   What|Removed |Added

 Status|Pushed to Stable|RESOLVED
 Resolution|--- |FIXED
 CC||katrin.fisc...@bsz-bw.de

--- Comment #61 from Katrin Fischer  ---
Onboarding tool is a new feature of 17.05 - so this is not needed in 16.11.x

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Fridolin SOMERS  changed:

   What|Removed |Added

 Status|Pushed to Master|Pushed to Stable
 CC||fridolin.som...@biblibre.co
   ||m

--- Comment #60 from Fridolin SOMERS  ---
Pushed to 17.05.x, will be in 17.05.01

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

 Status|Passed QA   |Pushed to Master

--- Comment #59 from Jonathan Druart  
---
Pushed to master for 17.11, thanks to everybody involved!

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Marcel de Rooy  changed:

   What|Removed |Added

 QA Contact|testo...@bugs.koha-communit |m.de.r...@rijksmuseum.nl
   |y.org   |

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #63310|0   |1
is obsolete||

--- Comment #58 from Marcel de Rooy  ---
Created attachment 63925
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63925=edit
Bug 17944: QA follow-up

- Remove an unused use statement
- Fix pod
- Use snake_case
- Fix test "An itemtype cannot be deleted if and only if there is
biblioitem linked with it"

Signed-off-by: Jonathan Druart 

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #63309|0   |1
is obsolete||

--- Comment #57 from Marcel de Rooy  ---
Created attachment 63924
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63924=edit
Bug 17944 - Add Koha::ItemType->can_be_deleted and use it from
admin/itemtypes.pl

Removed the sql code from Itemtypes.pm and replaced  it with DBIx
database query in the itemtypes.pl administrative script

Test plan:
1. In the staff interface, stage and manage MARC records for import

2. Try to delete an itemtype. If there are items of that itemtype in the
   database then a message telling you the number of items of that
   itemtype there are will be displayed.

3. Record that number

4. View the admin/itemtpes.pl script and confirm that there is sql code
   written in this file.

5. Apply this patch

6. View the admin/itemtypes.pl script and observe that there is no sql
   in this file. There is however DBIx code, for example
   $schema->resultset('Item')->search({ 'itype' => $itemtype_code} );
   which is searching for items with the itype value matching
   $itemtype_code value.

7. In the staff interface try to delete the same itemtype

8. Record the number of items there are with that itemtype in the
   resulting message

9. The numbers recorded in steps 3 and 8 should match showing that the
   DBIx code is working as intended

Signed-off-by: Jonathan Druart 

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Marcel de Rooy  changed:

   What|Removed |Added

 Status|Signed Off  |Passed QA
   Patch complexity|--- |Small patch

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Marcel de Rooy  changed:

   What|Removed |Added

 CC||m.de.r...@rijksmuseum.nl

--- Comment #56 from Marcel de Rooy  ---
String changes. Return after freeze.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944
Bug 17944 depends on bug 17855, which changed state.

Bug 17855 Summary: New onboarding tool feature to guide users through setting 
up Koha, and minor web installer UI improvements
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17855

   What|Removed |Added

 Status|Pushed to Master|RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #55 from Jonathan Druart  
---
Created attachment 63310
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63310=edit
Bug 17944: QA follow-up

- Remove an unused use statement
- Fix pod
- Use snake_case
- Fix test "An itemtype cannot be deleted if and only if there is
biblioitem linked with it"

Signed-off-by: Jonathan Druart 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

  Attachment #63307|0   |1
is obsolete||
  Attachment #63308|0   |1
is obsolete||

--- Comment #54 from Jonathan Druart  
---
Created attachment 63309
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63309=edit
Bug 17944 - Add Koha::ItemType->can_be_deleted and use it from
admin/itemtypes.pl

Removed the sql code from Itemtypes.pm and replaced  it with DBIx
database query in the itemtypes.pl administrative script

Test plan:
1. In the staff interface, stage and manage MARC records for import

2. Try to delete an itemtype. If there are items of that itemtype in the
   database then a message telling you the number of items of that
   itemtype there are will be displayed.

3. Record that number

4. View the admin/itemtpes.pl script and confirm that there is sql code
   written in this file.

5. Apply this patch

6. View the admin/itemtypes.pl script and observe that there is no sql
   in this file. There is however DBIx code, for example
   $schema->resultset('Item')->search({ 'itype' => $itemtype_code} );
   which is searching for items with the itype value matching
   $itemtype_code value.

7. In the staff interface try to delete the same itemtype

8. Record the number of items there are with that itemtype in the
   resulting message

9. The numbers recorded in steps 3 and 8 should match showing that the
   DBIx code is working as intended

Signed-off-by: Jonathan Druart 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

  Attachment #63259|0   |1
is obsolete||

--- Comment #52 from Jonathan Druart  
---
Created attachment 63307
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63307=edit
Bug 17944 - Removed SQl query from itemtypes.pl and replaced it with
can_be_deleted method in Koha/ItemType.pm

In Koha/ItemTypes.t I have reworded unit test output and added test for item
deletion.

In addition to updating admin script by removing warn and changing if
statement check

Signed-off-by: Jonathan Druart 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #53 from Jonathan Druart  
---
Created attachment 63308
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63308=edit
Bug 17944: QA follow-up

- Remove an unused use statement
- Fix pod
- Use snake_case
- Fix test "An itemtype cannot be deleted if and only if there is
biblioitem linked with it"

Signed-off-by: Jonathan Druart 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

 Status|Needs Signoff   |Signed Off

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|Failed QA   |Needs Signoff

--- Comment #51 from Alex Buckley  ---
Hi Jonathan

In the patch I just attached I have addressed all of your points:

1) 
> You are not retrieving any item type here, ->can_be_deleted will only return 
> 0.
> You need to call Koha::ItemType->find($id); to retrieve the item type object 
> you want from the DB.

I have changed the itemtypes.pl admin script (see below) so it now performs the
can_be_deleted() function on the result of the find() function, therefore it is
actually retrieving a item type here:
> 142 my $ItemType = Koha::ItemTypes->find($itemtype_code);
> 143 my $overalltotal = $ItemType->can_be_deleted();
> 144 if ($overalltotal == 0) {
> 145 push @messages, { type => 'error', code => 'cannot_be_delet
> ed'};

2)
>  my $itemtype = Koha::ItemTypes->find($itemtype_code);
> +my $deleted = eval{ $itemtype->delete };;

> Unnecessary change.

I have changed the above code removing the changes to the second line and
removing the second semi colon in the second line (see below):
> 153 my $itemtype = Koha::ItemTypes->find($itemtype_code);
> 154 my $deleted = eval { $itemtype->delete };


3)

> This second try does not test what you want, see my previous comment:
> """
> The last test says 'An item type that is used by a biblioitem cannot be
> deleted'. But actually you are testing that "an item that is used by AN item
> AND A biblioitem cannot be deleted'.
> You should delete the item to make it test what you want to test.
> """

I have changed the test output from 'an item that is used by AN item
 AND A biblioitem cannot be deleted' to 'An item type that is used by an item
and a biblioitem cannot be deleted'
because we are actually running the can_be_deleted() function on itemtype not
an item.


Additionally I have implemented a test for deleting the item and biblioitem
that use the itemtype in question. 

Then I have implemented another further test to show that now the item and
biblioitem have been removed the can_be_deleted() function correctly shows that
the itemtype can be deleted:
> 145 is ( $item->delete, 1, 'An item has been deleted' );
> 146 
> 147 is ( $biblio->delete, 1, 'A biblioitem has been deleted' );
> 148 
> 149 is ( $item_type->can_be_deleted, 1, 'The item type that was being u
> sed > by the removed item and biblioitem can now be deleted' );

I have run the ItemTypes.t unit test and all tests are successful:
> kohadev-koha@kohadevbox:/home/vagrant/kohaclone/t/db_dependent/Koha$ prove -v 
> > ItemTypes.t
> ItemTypes.t .. 
> 1..26
> ok 1 - use Koha::ItemType;
> ok 2 - use Koha::ItemTypes;
> ok 3 - first result
> ok 4 - itemtype/code
> ok 5 - description
> ok 6 - rentalcharge
> ok 7 - imageurl
> ok 8 - summary
> ok 9 - checkinmsg
> ok 10 - checkinmsgtype
> ok 11 - second result
> ok 12 - itemtype/code
> ok 13 - description
> ok 14 - rentalcharge
> ok 15 - imageurl
> ok 16 - summary
> ok 17 - checkinmsg
> ok 18 - checkinmsgtype
> ok 19 - There are 3 item types
> ok 20 - item types should be sorted by translated description
> ok 21 - An item type that is not used can be deleted
> ok 22 - An item type that is used by an item cannot be deleted
> ok 23 - An item that is used by an item and a biblioitem cannot be deleted
> ok 24 - An item has been deleted
> ok 25 - A biblioitem has been deleted
> ok 26 - The item type that was being used by the removed item and biblioitem  
> > can be deleted
> ok
> All tests successful.

Cheers
Alex

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #63220|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #50 from Alex Buckley  ---
Created attachment 63259
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63259=edit
Bug 17944 - Removed SQl query from itemtypes.pl and replaced it with
can_be_deleted method in Koha/ItemType.pm

In Koha/ItemTypes.t I have reworded unit test output and added test for item
deletion.

In addition to updating admin script by removing warn and changing if
statement check

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #49 from Jonathan Druart  
---
Comment on attachment 63220
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63220
Bug 17944 - Removed SQl query from itemtypes.pl and replaced it with
can_be_deleted method in Koha/ItemType.pm

Review of attachment 63220:
 --> 
(https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html=17944=63220)
-

::: admin/itemtypes.pl
@@ +139,5 @@
>  $op  = 'list';
> +
> + } elsif ( $op eq 'delete_confirm' ) {
> +my $ItemType = Koha::ItemType->new();
> +my $overalltotal = $ItemType->can_be_deleted();

You are not retrieving any item type here, ->can_be_deleted will only return 0.
You need to call Koha::ItemType->find($id); to retrieve the item type object
you want from the DB.

@@ +154,2 @@
>  my $itemtype = Koha::ItemTypes->find($itemtype_code);
> +my $deleted = eval{ $itemtype->delete };;

Unnecessary change.

::: t/db_dependent/Koha/ItemTypes.t
@@ +139,5 @@
> +is( $item_type->can_be_deleted, 0, 'An item type that is used by an item 
> cannot be deleted' );
> +
> +my $biblio = $builder->build_object({ class => 'Koha::Biblioitems', value => 
> { itemtype => $item_type->itemtype }});
> +
> +is ( $item_type->can_be_deleted, 0, 'An item that is used by an item and a 
> biblioitem cannot be deleted' );

This second try does not test what you want, see my previous comment:
"""
The last test says 'An item type that is used by a biblioitem cannot be
deleted'. But actually you are testing that "an item that is used by AN item
AND A biblioitem cannot be deleted'.
You should delete the item to make it test what you want to test.
"""

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

 Status|Needs Signoff   |Failed QA

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|In Discussion   |Needs Signoff

--- Comment #48 from Alex Buckley  ---
Hi Jonathan

I have implemented:
> return $nb_items + $nb_biblioitems == 0 ? 1 : 0;

I have changed the wording of the last test and have implemented a test to
remove the item. All tests run successfully. 

As for updating the admin script I have removed the warn and changed the if
statement check so it is now:
> if ($overalltotal == 0){

Was there any other updates you wanted for the itemtypes.pl admin script?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #47 from Alex Buckley  ---
Created attachment 63220
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63220=edit
Bug 17944 - Removed SQl query from itemtypes.pl and replaced it with
can_be_deleted method in Koha/ItemType.pm

In Koha/ItemTypes.t I have reworded unit test output and added test for item
deletion.

In addition to updating admin script by removing warn and changing if
statement check

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #63152|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

 Status|Needs Signoff   |In Discussion

--- Comment #46 from Jonathan Druart  
---
I am switching the status to 'in discussion' until it's ready to be tested.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #45 from Jonathan Druart  
---
Alex, great! The patch is much better :)

Indeed the return value of can_be_deleted should be 1 or 0. You can achieve
that with:
  return $nb_items + $nb_biblioitems == 0 ? 1 : 0;
You will avoid the empty string return value.

In the tests:
The last test says 'An item type that is used by a biblioitem cannot be
deleted'. But actually you are testing that "an item that is used by AN item
AND A biblioitem cannot be deleted'.
You should delete the item to make it test what you want to test.

Finally you will need to update the admin script.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #44 from Alex Buckley  ---
Hi Jonathan

I have implemented the changes you suggested in a single patch for bug 17944.

Because I have implemented the following return line (which you suggested) from
can_be_deleted() subroutine:

return $nb_items + $nb_biblioitems == 0;

I had to slightly change the unit test from how you suggested it, this is
because whenever there were items and/or biblioitems with the itemtype it meant
that ' ' was returned. So the unit test callers are now:
is( $item_type->can_be_deleted, '', 'An item type that is used by an item
cannot be deleted' );

is ( $item_type->can_be_deleted, '', 'An item type that is used by a biblioitem
cannot be deleted' );

Additionally as '' is returned if the itemtype is in use, rather than the
number of items and biblioitems using the itemtype I have changed itemtypes.tt
to saying:

Cannot delete this item type.

This record is in use. Deletion is not possible.

If you would still like the number of items and biblioitems using the itemtype
to be displayed to the user in itemtypes.tt then I will need to change the
return for can_be_deleted to a longer if else statement.

Could you please have a look at this patch?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #43 from Alex Buckley  ---
Created attachment 63152
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63152=edit
Bug 17944 - Removed SQl query from itemtypes.pl and replaced it with
can_be_deleted method in Koha/ItemType.pm

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|Failed QA   |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #63045|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

 Status|Needs Signoff   |Failed QA

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #42 from Jonathan Druart  
---
Alex,
Some additional remarks (we are aiming the perfect patch, right? :))

1. The POD of the new method is incorrect, please adapt it.
Note that a line should not be longer than 80 chars

2. The method can_be_deleted should be something like:

sub can_be_deleted {
my ($self) = @_; 
my $nb_items = Koha::Items->search( { itype => $self->itemtype } )->count;
my $nb_biblioitems = Koha::Biblioitems->search( { itemtype =>
$self->itemtype } )->count;
return $nb_items + $nb_biblioitems == 0;
}

$self is the current object, from this object you know the itemtype (code), so
you so not need to pass it as an argument.

3. The tests are wrong:
I have a biblio with biblionumber=1, so they fail (Duplicate entry '1' for key
'PRIMARY'). You need to use our TestBuilder module to generate random data.
What you want is:
a) Generate the $builder object:
  my $builder = t::lib::TestBuilder->new;

b) create an item type:
  my $item_type = $builder->build_object({ class => 'Koha::ItemTypes' });

c) you want to test if an item type, not used, can be removed. It's time!
  is( $item_type->can_be_deleted, 1, 'An item type that is not used can be
deleted' );

d) create an item using this item type:
  my $item = $builder->build_object({ class => 'Koha::Items', value => { itype
=> $item_type->itemtype });

e) Then you want to know if the item type can be removed
  is( $item_type->can_be_deleted, 0, 'An item type that is used by an item
cannot be deleted );

Then you can continue with a biblioitem object.

Please resubmit a patch without the delete method, just can_be_deleted
step-by-step :)

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|BLOCKED |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #62504|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #41 from Alex Buckley  ---
Created attachment 63045
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63045=edit
Bug 17944 - Overridden Koha::Object->delete with Koha::ItemType->delete and
implemented exception to be shown if the itemtype cannot be deleted.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #62505|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #62503|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #62500|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #62502|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #40 from Alex Buckley  ---
(In reply to Jonathan Druart in comment 39)

Hi Jonathan

I have squashed all the bugs together locally and I have implemented a delete
method in ItemType.pm to override Koha::Object->delete.

I have implemented a try catch so if the delete doesn't work then catch is run
and Koha::Exceptions::CannotDeleteDefault->throw is run (as shown below)

>116 sub delete{
>117 my ( $self ) = @_;
>118 try {
>119 my $deleted = $self->SUPER::delete($self);
>120 logaction( 'ITEMTYPE', 'DELETE', $self->reserve_id, Dumper(
>>$self->unblessed) )
>121 if C4::Context->preference('ItemTypeLog');
>122 return $deleted;
>123 }  catch {
>124 Koha::Exceptions::CannotDeleteDefault->throw('itemtype 
>>could not be deleted');
>125 }
>126 }

However I am a bit confused where you would like the Koha::ItemType->delete
call to be in itemtypes.pl.

Correct me if I am wrong but I think it cant go in elsif ( $op eq
'delete_confirm') block  because putting a caller to delete into this block
means if the delete runs successfully the user has no choice to confirm (or
not) the deletion.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #39 from Jonathan Druart  
---
Alex,

Please squash all the patches and resubmit, for readability ;)

For the check, I think the best would be to override Koha::Object->delete from
Koha::ItemType->delete (`git grep 'sub delete' Koha` for examples), and raise
an exception if the item type cannot be deleted.
You will need to surround the ->delete call with an eval in the pl script.

Let me known if you have any questions about this suggestion.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Owen Leonard  changed:

   What|Removed |Added

 Status|Needs Signoff   |BLOCKED

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|Patch doesn't apply |Needs Signoff

--- Comment #38 from Alex Buckley  ---
I have merged bug 17855 and the patches of this bug and reattached so now the
patches on this bug report apply successfully after you have applied bug 17855

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #37 from Alex Buckley  ---
Created attachment 62505
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62505=edit
Bug 17944 -Integrated the removal of SQL from itemtypes.pl admin script with
the onboarding tool following tester feedback of merge conflict

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #35 from Alex Buckley  ---
Created attachment 62503
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62503=edit
Bug 17944 - Moved the DBIx code into the ItemType.pm file

Created get_items_and_biblioitems in Koha::ItemType using
DBIx coding style.

Changed itemtypes.pl to use that method

This addresses comment #15.

Signed-off-by: Mark Tompsett 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #36 from Alex Buckley  ---
Created attachment 62504
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62504=edit
Bug 17944 - Updates following second tester feedback

Changed function name from get_items_and_biblioitems to is_used

Added POD for is_used function

Initial unit test for is_used implemented (not yet working)

Implemented use of Koha::Items and Koha::Biblioitems instead of using Koha
resultsets

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #34 from Alex Buckley  ---
Created attachment 62502
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62502=edit
Bug 17944 - Tidied the DBIx code in itemtypes.pl

Signed-off-by: Owen Leonard 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #62501|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #33 from Alex Buckley  ---
Created attachment 62501
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62501=edit
Bug 17944 - Tidied the DBIx code in itemtypes.pl

Signed-off-by: Owen Leonard 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #32 from Alex Buckley  ---
Created attachment 62500
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62500=edit
Bug 17944 - Removed the sql code from Itemtypes.pm

Removed the sql code from Itemtypes.pm and replaced  it with DBIx
database query in the itemtypes.pl administrative script

Test plan:
1. In the staff interface, stage and manage MARC records for import

2. Try to delete an itemtype. If there are items of that itemtype in the
   database then a message telling you the number of items of that
   itemtype there are will be displayed.

3. Record that number

4. View the admin/itemtpes.pl script and confirm that there is sql code
   written in this file.

5. Apply this patch

6. View the admin/itemtypes.pl script and observe that there is no sql
   in this file. There is however DBIx code, for example
   $schema->resultset('Item')->search({ 'itype' => $itemtype_code} );
   which is searching for items with the itype value matching
   $itemtype_code value.

7. In the staff interface try to delete the same itemtype

8. Record the number of items there are with that itemtype in the
   resulting message

9. The numbers recorded in steps 3 and 8 should match showing that the
   DBIx code is working as intended

Signed-off-by: Owen Leonard 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #31 from Alex Buckley  ---
Created attachment 62499
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62499=edit
Bug 17944 - Moved the sql code from itemtypes.pl into Koha/ItemTypes.pm

Signed-off-by: Owen Leonard 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #59656|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #59655|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #59657|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #59718|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #59953|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

  Attachment #59954|0   |1
is obsolete||

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

 Status|Needs Signoff   |Patch doesn't apply

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Marc Véron  changed:

   What|Removed |Added

 CC||ve...@veron.ch

--- Comment #30 from Marc Véron  ---
Wanted to test, but can not apply:

Wende an: Bug 17944 - Moved the sql code from itemtypes.pl into
Koha/ItemTypes.pm
Wende an: Bug 17944 - Removed the sql code from Itemtypes.pm
Wende an: Bug 17944 - Tidied the DBIx code in itemtypes.pl
Wende an: Bug 17944 - Moved the DBIx code into the ItemType.pm file
Wende an: Bug 17944 - Updates following second tester feedback
/home/marc/koha/.git/rebase-apply/patch:16: trailing whitespace.
=head3 is_used 
/home/marc/koha/.git/rebase-apply/patch:91: trailing whitespace.
itemlost => 0, 
fatal: sha1 information is lacking or useless (Koha/ItemType.pm).
Dem Repository fehlen notwendige Blobs um auf einen 3-Wege-Merge
zurückzufallen.
Kann nicht zu 3-Wege-Merge zurückfallen.

(Can not fall back to 3-way-merge because of missing blobs)

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #29 from Alex Buckley  ---
(In reply to Baptiste in comment 28)

Ah sorry I see what the issue is I accidentally missed some steps in the test
plan for this patch.

The ItemTypes.t unit test needs to be run after running the web installer
(installing libraries, categories and item types) but before adding/importing
any bibliographic records.

You were getting a duplicate entry error (which I have just replicated) by
having biblio record(s) in your Koha database before running this unit test and
so when a record with the primary key of 1 is inserted into the biblio table by
this unit test (a change I added to the ItemTypes.t unit test as part of this
patch) it throws an error because a biblio record with the primary key of 1
already exists.

Therefore a corrected test plan for attachment 59954 (Bug 17944 - Implemented a
subtest in the ItemTypes.t file for is_used function in ItemType.pm) is:

1. Apply patches of this bug
2. Restart memcached: sudo service memcached restart
3. Drop and recreate your Koha database 
4. Run through the Koha web installer making sure to select all data values for
installation
5. When the web installer has finished, in your terminal navigate to
t/db_dependent/Koha/
6. Start koha-shell: sudo koha-shell 
7. Run the ItemTypes.t unit test: prove -v ItemTypes.t
8. Notice all tests are successful, and notice that the last two outputs
show the is_used subtest gave successful results

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Baptiste  changed:

   What|Removed |Added

 CC||baptiste.wojtkowski@biblibr
   ||e.com

--- Comment #28 from Baptiste  ---
Looks like it doesn't work on my koha :

On the master :

All tests successful.
Files=1, Tests=18,  1 wallclock secs ( 0.02 usr  0.01 sys +  0.84 cusr  0.04
csys =  0.91 CPU)
Result: PASS

On branch bug/17944 :

DBD::mysql::st execute failed: Duplicate entry '1' for key 'PRIMARY' [for
Statement "INSERT INTO `biblio` ( `biblionumber`) VALUES ( ? )" with
ParamValues: 0=1] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.
DBIx::Class::Storage::DBI::_dbh_execute(): Duplicate entry '1' for key
'PRIMARY' at /home/koha/src/Koha/Object.pm line 120
# Looks like you planned 20 tests but ran 2.
# Looks like your test exited with 255 just after 2.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|In Discussion   |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #26 from Alex Buckley  ---
Created attachment 59953
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59953=edit
Bug 17944 - Updates following second tester feedback

Changed function name from get_items_and_biblioitems to is_used

Added POD for is_used function

Initial unit test for is_used implemented (not yet working)

Implemented use of Koha::Items and Koha::Biblioitems instead of using Koha
resultsets

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #27 from Alex Buckley  ---
Created attachment 59954
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59954=edit
Bug 17944 - Implemented a subtest in the ItemTypes.t file for is_used function
in ItemType.pm

Test plan:
1. Go to t/db_dependent/Koha/
2. Start koha-shell: sudo koha-shell 
3. Run the ItemTypes.t unit test: prove -v ItemTypes.t
4. Notice all tests are successful, and notice that the last two outputs
show the is_used subtest gave successful results

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

Version|17.05   |unspecified

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #25 from Jonathan Druart  
---
(In reply to Alex Buckley from comment #22)
> (in reply to Jonathan Druart in comment 21)
> 
> 1/ What tests specifically do you want?

When a subroutine or a method is added or updated, we need to provide tests
("unit tests") in the t directory.
For a method like "Koha::ItemType->is_used" you will want to add tests to
t/db_dependent/Koha/ItemTypes.t.
Create a new subtest per method is a good idea (`git grep subtest t` to see
examples)
In your case you want to test that this new method returns true if at least 1
item and/or 1 biblioitem is using it. Otherwise it returns false.

(In reply to Alex Buckley from comment #24)
> (in reply to Jonathan Druart in comment 21)
> Could you please explain what you mean in your point 5? Are you asking why I
> havent put the is_used subroutine in the Koha::Items and Koha::Biblioitems
> files?

Instead of using
  $schema->resultset('Item')->search
you can use
  Koha::Items->search

We have Koha objects to represent our data model, so when they exist
(Koha::Items and Koha::Biblioitems do exist) it's better to use them.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #24 from Alex Buckley  ---
(in reply to Jonathan Druart in comment 21)

Please disregard comment 22, I have fixed the strict refs error issue. 

In the commit I am currently working on I have addressed your following points
2, and 3.

Could you please explain what you mean in your point 5? Are you asking why I
havent put the is_used subroutine in the Koha::Items and Koha::Biblioitems
files?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #23 from Alex Buckley  ---
(in reply to Jonathan Druart in comment 21)

1/ Presumably you mean test plan rather than the output of a qa test? If so the
test plan is slightly modified from comment 5)

Test plan:
1. In the staff interface, stage and manage MARC records for import

2. Try to delete an itemtype. If there are items of that itemtype in the
   database then a message telling you the number of items of that
   itemtype there are will be displayed.

3. Record that number

4. View the admin/itemtpes.pl script and confirm that there is sql code
   written in this file.

5. Apply this patch

6. View the admin/itemtypes.pl script and observe that there is no sql
   in this file. Confirm there is a module call 
my $overalltotal = Koha::ItemType->get_items_and_biblioitems($itemtype_code)

7. Go to Koha/ItemType.pm and confirm that there is a function named
get_items_and_biblioitems

8. In the staff interface try to delete the same itemtype

9. Record the number of items there are with that itemtype in the
   resulting message

10. The numbers recorded in steps 3 and 9 should match showing that the
   DBIx code in the get_items_and_biblioitems subroutine is working as intended

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|Failed QA   |In Discussion

--- Comment #22 from Alex Buckley  ---
(in reply to Jonathan Druart in comment 21)

1/ What tests specifically do you want?

3/ I tried '->is_used' but it threw a strict refs error (as this is my first
time writing perl subroutines I tried a different subroutine name
(get_items_and_biblioitems) which worked successfully.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

 Status|Signed Off  |Failed QA

--- Comment #21 from Jonathan Druart  
---
Alex,
There are some wrong things here:
1/ No tests are provided
2/ You do not instanciate a Koha::ItemType object but using
get_items_and_biblioitems as a class method.
3/ get_items_and_biblioitems is badly named, it does not return the items and
the biblioitems, but the number of item and biblioitems.
That's why I suggested ->is_used, which would return true or false.
4/ There is no POD for this new method
5/ Why do not you want to use Koha::Items and Koha::Biblioitems?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

M. Tompsett  changed:

   What|Removed |Added

  Attachment #59715|0   |1
is obsolete||
  Attachment #59716|0   |1
is obsolete||
  Attachment #59717|0   |1
is obsolete||

--- Comment #20 from M. Tompsett  ---
Created attachment 59718
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59718=edit
Bug 17944 - Moved the DBIx code into the ItemType.pm file

Created get_items_and_biblioitems in Koha::ItemType using
DBIx coding style.

Changed itemtypes.pl to use that method

This addresses comment #15.

Signed-off-by: Mark Tompsett 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

M. Tompsett  changed:

   What|Removed |Added

 Status|Needs Signoff   |Signed Off

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #19 from Alex Buckley  ---
Created attachment 59717
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59717=edit
Bug 17944 - Added additional USE pragma

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #18 from Alex Buckley  ---
Created attachment 59716
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59716=edit
Bug 17955 - removed incorrect function call

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|ASSIGNED|Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

M. Tompsett  changed:

   What|Removed |Added

 Status|Needs Signoff   |ASSIGNED

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #17 from M. Tompsett  ---
Comment on attachment 59715
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59715
Bug 17944 - Moved the DBIx code into the ItemType.pm file

Review of attachment 59715:
 --> 
(https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html=17944=59715)
-

::: admin/itemtypes.pl
@@ +140,4 @@
>  
>   } elsif ( $op eq 'delete_confirm' ) {
>  
> +my $overalltotal = 
> Koha::ItemType->get_items_and_biblioitems($itemtype_code);

This is what was asked for in comment #15.

@@ +151,4 @@
>  
>  } elsif ( $op eq 'delete_confirmed' ) {
>  
> +my $itemtype = Koha::ItemType->get_items_and_biblioitems($itemtype_code);

This change confuses me.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|Signed Off  |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #16 from Alex Buckley  ---
Created attachment 59715
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59715=edit
Bug 17944 - Moved the DBIx code into the ItemType.pm file

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Jonathan Druart  changed:

   What|Removed |Added

 CC||jonathan.dru...@bugs.koha-c
   ||ommunity.org

--- Comment #15 from Jonathan Druart  
---
Can I suggest to create a method in Koha::ItemType (something like ->is_used),
which would return Koha::Items->search({ itype => $itemtype_code })->count +
Koha::Biblioitems->search({ itemtype => $itemtype_code })->count

That way we could provide tests for this change.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Owen Leonard  changed:

   What|Removed |Added

 Status|Needs Signoff   |Signed Off

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Owen Leonard  changed:

   What|Removed |Added

  Attachment #59564|0   |1
is obsolete||

--- Comment #13 from Owen Leonard  ---
Created attachment 59656
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59656=edit
[SIGNED-OFF] Bug 17944 - Removed the sql code from Itemtypes.pm

Removed the sql code from Itemtypes.pm and replaced  it with DBIx
database query in the itemtypes.pl administrative script

Test plan:
1. In the staff interface, stage and manage MARC records for import

2. Try to delete an itemtype. If there are items of that itemtype in the
   database then a message telling you the number of items of that
   itemtype there are will be displayed.

3. Record that number

4. View the admin/itemtpes.pl script and confirm that there is sql code
   written in this file.

5. Apply this patch

6. View the admin/itemtypes.pl script and observe that there is no sql
   in this file. There is however DBIx code, for example
   $schema->resultset('Item')->search({ 'itype' => $itemtype_code} );
   which is searching for items with the itype value matching
   $itemtype_code value.

7. In the staff interface try to delete the same itemtype

8. Record the number of items there are with that itemtype in the
   resulting message

9. The numbers recorded in steps 3 and 8 should match showing that the
   DBIx code is working as intended

Signed-off-by: Owen Leonard 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Owen Leonard  changed:

   What|Removed |Added

  Attachment #59391|0   |1
is obsolete||

--- Comment #12 from Owen Leonard  ---
Created attachment 59655
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59655=edit
[SIGNED-OFF] Bug 17944 - Moved the sql code from itemtypes.pl into
Koha/ItemTypes.pm

Signed-off-by: Owen Leonard 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Owen Leonard  changed:

   What|Removed |Added

  Attachment #59565|0   |1
is obsolete||

--- Comment #14 from Owen Leonard  ---
Created attachment 59657
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59657=edit
[SIGNED-OFF] Bug 17944 - Tidied the DBIx code in itemtypes.pl

Signed-off-by: Owen Leonard 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|In Discussion   |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #11 from Alex Buckley  ---
Created attachment 59565
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59565=edit
Bug 17944 - Tidied the DBIx code in itemtypes.pl

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #10 from M. Tompsett  ---
(In reply to Alex Buckley from comment #9)
> So before I attach the new commit just to check would you be happy with this?

Yes, and it is prettier. Thank you. :)

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #9 from Alex Buckley  ---
So before I attach the new commit just to check would you be happy with this?

 my $schema = Koha::Database->new()->schema();
 my $itemtotal = $schema->resultset('Item')->search({ 'itype' =>
$itemtype_code})->count;
 my $bibliototal = $schema->resultset('Biblioitem')->search({ 'itemtype' =>
$itemtype_code})->count;

 my $overalltotal = $itemtotal + $bibliototal;

 if ($overalltotal) {
  push @messages, { type => 'error', code => 'cannot_be_deleted', total =>
$overalltotal };
  $op = 'list';
 } else {
  my $itemtype = Koha::ItemTypes->find($itemtype_code);
  $template->param( itemtype => $itemtype, );
}   

I have tested it and it outputs the correct value

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #8 from Alex Buckley  ---
I'll try implementing that now.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

M. Tompsett  changed:

   What|Removed |Added

 Status|Needs Signoff   |In Discussion

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #7 from M. Tompsett  ---
Comment on attachment 59564
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59564
Bug 17944 - Removed the sql code from Itemtypes.pm and replaced it with DBIx
database query in the itemtypes.pl administrative script

Review of attachment 59564:
 --> 
(https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html=17944=59564)
-

::: admin/itemtypes.pl
@@ +145,5 @@
> +my $itemtotal = $schema->resultset('Item')->search({ 'itype' => 
> $itemtype_code} );
> +my $bibliototal = $schema->resultset('Biblioitem')->search({ 'itemtype' 
> => $itemtype_code} );
> +
> +$itemtotal->count;
> +$bibliototal->count;

Why?

@@ +146,5 @@
> +my $bibliototal = $schema->resultset('Biblioitem')->search({ 'itemtype' 
> => $itemtype_code} );
> +
> +$itemtotal->count;
> +$bibliototal->count;
> +my $overalltotal = $itemtotal + $bibliototal;

I like the idea, but this segment of code is not quite right.

Why the context line?
why not my $foo = $schema->resulset('Bar')->search({ 'foobar' => $itemtypecode
})->count // 0; with the appropriate variables and tables changed?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #6 from Alex Buckley  ---
(in reply to comment 4 by M.tompsett)

I have tried to implement DBIx in the itemtypes.pl script.

To get it working successfully I had to implement two DBIx queries, one to
search the items and another to search the biblioitems for the itemtype being
attempted to be deleted.

Then I add the number of retrieved items together. 

I tried to implement the entire original sql query into the equivalent singular
DBIx query however I could not get a successful search of both items and
biblioitems.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #5 from Alex Buckley  ---
Created attachment 59564
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59564=edit
Bug 17944 - Removed the sql code from Itemtypes.pm and replaced it with DBIx
database query in the itemtypes.pl administrative script

Test plan:
1. In the staff interface, stage and manage MARC records for import

2. Try to delete an itemtype. If there are items of that itemtype in the
database then a message telling you the number of items of that itemtype
there are will be displayed.

3. Record that number

4. View the admin/itemtpes.pl script and confirm that there is sql code
written in this file.

5. Apply this patch

6. View the admin/itemtypes.pl script and observe that there is no sql in
this file. There is however DBIx code, for example
$schema->resultset('Item')->search({ 'itype' => $itemtype_code} ); which
is searching for items with the itype value matching $itemtype_code
value.

7. In the staff interface try to delete the same itemtype

8. Record the number of items there are with that itemtype in the
resulting message

9. The numbers recorded in steps 3 and 8 should match showing that the
DBIx code is working as intended

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #4 from M. Tompsett  ---
Comment on attachment 59391
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59391
Bug 17944 - Moved the sql code from itemtypes.pl into Koha/ItemTypes.pm

Review of attachment 59391:
 --> 
(https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html=17944=59391)
-

::: Koha/ItemTypes.pm
@@ +53,5 @@
> +) AS tmp
> +WHERE tmp.t=?
> +', {}, $itemtype_code );
> +return $total;
> +}

While not an easy thing, I don't even know how to do it off the top of my head,
consider reworking in DBIx.

Also, wouldn't this partially double count? Don't worry, this is a pre-existing
issue.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

   See Also||https://bugs.koha-community
   ||.org/bugzilla3/show_bug.cgi
   ||?id=17955

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

   See Also||https://bugs.koha-community
   ||.org/bugzilla3/show_bug.cgi
   ||?id=17954

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

 Status|ASSIGNED|Needs Signoff

--- Comment #3 from Alex Buckley  ---
Test plan:
1. View the admin/itemtypes.pl script and confirm that sql code is written in
this file

2. Apply this patch

3. View admin/itemtypes.pl script notice the sql code has been removed and
replaced with the perl module call (on line 142):
 my $total = Koha::ItemTypes->check_items_and_biblioitems($itemtype_code);


4. View Koha/ItemTypes.pm and observe the subroutine
check_items_and_biblioitems
(This subroutine contains the sql code that was in itemtypes.pl)

5. Now to check that the deletion of an item type still works with the sql code
moved out of the itemtypes.pl script, log into the Koha staff interface

6. Visit Administration->Item type. Select the 'Delete' button next to an item
type

7. Select the 'Yes, delete this item type' button

8. Observe the message informing you the item type has been deleted. Thus the
patch works as intended. 

Note: as I understand it placing the sql code into a perl module in the Koha
directory is acceptable, because sql code exists in the perl modules in the C4
directory. Please advise me if this isn't the case and I will move this code
again.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

--- Comment #2 from Alex Buckley  ---
Created attachment 59391
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=59391=edit
Bug 17944 - Moved the sql code from itemtypes.pl into Koha/ItemTypes.pm

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17944] Remove the sql code from itemtypes.pl administrative perl script

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17944

Alex Buckley  changed:

   What|Removed |Added

Summary|Remove the sql code from|Remove the sql code from
   |onboarding and  |itemtypes.pl administrative
   |administrative perl |perl script
   |scripts, and remove |
   |unnecessary use pragmas |
   |from onboarding.pl  |

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/