Created attachment 42566 [details] [review] Bug 14828: Add Koha::ItemType[s] classes
Created attachment 42567 [details] [review] Bug 14828: use Koha::ItemType[s] in admin/itemtypes Test plan: Add/edit/remove item types from the administration module (admin/itemtypes.pl). You should get message feedback after each action. Don't forget to play with the image and try to remove an item type linked to records (you should not be able to remove it).
Created attachment 42568 [details] [review] Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used This patch mainly replaces C4::ItemType->all with Koha::ItemTypes->search. Test plan: At the places where the C4::ItemType module was used, confirm there is no regression: - acqui/neworderempty.pl - catalogue/itemsearch.pl - admin/item_circulation_alerts.pl and the 2 cataloguing plugins: - marc21_linking_section.pl - unimarc_field_4XX.pl QA step: prove t/db_dependent/HoldsQueue.t should return green Note that the tests were buggy.
Created attachment 42569 [details] [review] Bug 14828: Remove old package and tests C4::ItemType can be now removed safely, there is no use of this module.
The next step will be to replace C4::ItemTypes calls with Koha::ItemTypes->search
Could you please rebase?
Created attachment 45748 [details] [review] Bug 14828: Add Koha::ItemType[s] classes
Created attachment 45749 [details] [review] Bug 14828: use Koha::ItemType[s] in admin/itemtypes Test plan: Add/edit/remove item types from the administration module (admin/itemtypes.pl). You should get message feedback after each action. Don't forget to play with the image and try to remove an item type linked to records (you should not be able to remove it).
Created attachment 45750 [details] [review] Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used This patch mainly replaces C4::ItemType->all with Koha::ItemTypes->search. Test plan: At the places where the C4::ItemType module was used, confirm there is no regression: - acqui/neworderempty.pl - catalogue/itemsearch.pl - admin/item_circulation_alerts.pl and the 2 cataloguing plugins: - marc21_linking_section.pl - unimarc_field_4XX.pl QA step: prove t/db_dependent/HoldsQueue.t should return green Note that the tests were buggy.
Created attachment 45751 [details] [review] Bug 14828: Remove old package and tests C4::ItemType can be now removed safely, there is no use of this module.
Quite tricky conflicts (with bug 10937, bug 14100, etc.) fixed, patches rebase against master.
I just tried and found some issues: In translating itemtypes there is string "Koha::ItemType=HASH(0x8b7c6e0)" (for example) instead of the itemtype code. On itemtype list (table on itemtype main page), there are missing images, the charge is showed only if it's zero. But editing works as expected. Adding new itemtype does not work because of this error: Can't call method "searchcategory" on an undefined value at /usr/share/koha/intranet/cgi-bin/admin/itemtypes.pl line 62. I didn't find any reggression in these scripts: - acqui/neworderempty.pl - catalogue/itemsearch.pl - admin/item_circulation_alerts.pl Trying to use marc21_linking_section.pl cataloguing plugin i get these errors in log: Use of uninitialized value $self in pattern match (m//) at /usr/share/perl5/MARC/File/XML.pm line 441. TransformMarcToKoha called with undefined record at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 212. Use of uninitialized value $pos in index at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 215. Use of uninitialized value in index at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 215. Use of uninitialized value in split at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 220. Use of uninitialized value in split at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 221. Use of uninitialized value $CN in substitution (s///) at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 226. Use of uninitialized value $startfrom in multiplication (*) at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 257. Use of uninitialized value $startfrom in addition (+) at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 279. Use of uninitialized value $startfrom in subtraction (-) at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 279. CGI::param called in list context from package CGI::Compile::ROOT::usr_share_koha_intranet_cgi_2dbin_cataloguing_plugin_launcher_2epl line 26, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436. CGI::param called in list context from package C4::Auth line 399, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436. Can't call method "field" on an undefined value at /usr/share/koha/intranet/cgi-bin/cataloguing/value_builder/marc21_linking_section.pl line 96. I had Plack active during testing.
Created attachment 45769 [details] [review] Bug 14828: use Koha::ItemType[s] in admin/itemtypes Test plan: Add/edit/remove item types from the administration module (admin/itemtypes.pl). You should get message feedback after each action. Don't forget to play with the image and try to remove an item type linked to records (you should not be able to remove it).
Quite tricky conflicts (with bug 10937, bug 14100, etc.) fixed, patches rebase against master.(In reply to Josef Moravec from comment #12) > I just tried and found some issues: > > In translating itemtypes there is string "Koha::ItemType=HASH(0x8b7c6e0)" > (for example) instead of the itemtype code. Oops, I mess up in fixing the conflicts! Should be fixed now. > On itemtype list (table on itemtype main page), there are missing images, > the charge is showed only if it's zero. But editing works as expected. Charge is shown without plack, with plack the issue is on master. Please open a new bug report. For the image, I cannot reproduce, which images are you using? > Adding new itemtype does not work because of this error: > Can't call method "searchcategory" on an undefined value at > /usr/share/koha/intranet/cgi-bin/admin/itemtypes.pl line 62. Before of the first problem. > Trying to use marc21_linking_section.pl cataloguing plugin i get these > errors in log: Could you please confirm this is not a master issue please? I get a lot of warning in the logs, but not the error. Not sure I am using it correctly.
Created attachment 45771 [details] [review] Bug 14828: Add Koha::ItemType[s] classes Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45772 [details] [review] Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used This patch mainly replaces C4::ItemType->all with Koha::ItemTypes->search. Test plan: At the places where the C4::ItemType module was used, confirm there is no regression: - acqui/neworderempty.pl - catalogue/itemsearch.pl - admin/item_circulation_alerts.pl and the 2 cataloguing plugins: - marc21_linking_section.pl - unimarc_field_4XX.pl QA step: prove t/db_dependent/HoldsQueue.t should return green Note that the tests were buggy. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45773 [details] [review] Bug 14828: Remove old package and tests C4::ItemType can be now removed safely, there is no use of this module. Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 45774 [details] [review] Bug 14828: use Koha::ItemType[s] in admin/itemtypes Test plan: Add/edit/remove item types from the administration module (admin/itemtypes.pl). You should get message feedback after each action. Don't forget to play with the image and try to remove an item type linked to records (you should not be able to remove it). Signed-off-by: Marc Véron <veron@veron.ch>
Tested all patches together, followed test plan from commentt #16, worked as expected.
> QA step: > prove t/db_dependent/HoldsQueue.t should return green > Note that the tests were buggy. Just note that they are still buggy, I have opened a but report, see bug 15391
Regarding to marc21_linking plugin, it's the same for me on master too, so it isn't problem of this patch. Probably I am badly using this plugin ;)
Created attachment 46731 [details] [review] Bug 14828: Add Koha::ItemType[s] classes Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 46732 [details] [review] Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used This patch mainly replaces C4::ItemType->all with Koha::ItemTypes->search. Test plan: At the places where the C4::ItemType module was used, confirm there is no regression: - acqui/neworderempty.pl - catalogue/itemsearch.pl - admin/item_circulation_alerts.pl and the 2 cataloguing plugins: - marc21_linking_section.pl - unimarc_field_4XX.pl QA step: prove t/db_dependent/HoldsQueue.t should return green Note that the tests were buggy. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 46733 [details] [review] Bug 14828: Remove old package and tests C4::ItemType can be now removed safely, there is no use of this module. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 46734 [details] [review] Bug 14828: use Koha::ItemType[s] in admin/itemtypes Test plan: Add/edit/remove item types from the administration module (admin/itemtypes.pl). You should get message feedback after each action. Don't forget to play with the image and try to remove an item type linked to records (you should not be able to remove it). Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 46735 [details] [review] Bug 14828 [QA Followup] - Hide in OPAC checkbox not checked in editor even if enabled Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 47341 [details] [review] Bug 14828: Add Koha::ItemType[s] classes Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 47342 [details] [review] Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used This patch mainly replaces C4::ItemType->all with Koha::ItemTypes->search. Test plan: At the places where the C4::ItemType module was used, confirm there is no regression: - acqui/neworderempty.pl - catalogue/itemsearch.pl - admin/item_circulation_alerts.pl and the 2 cataloguing plugins: - marc21_linking_section.pl - unimarc_field_4XX.pl QA step: prove t/db_dependent/HoldsQueue.t should return green Note that the tests were buggy. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 47343 [details] [review] Bug 14828: Remove old package and tests C4::ItemType can be now removed safely, there is no use of this module. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 47344 [details] [review] Bug 14828: use Koha::ItemType[s] in admin/itemtypes Test plan: Add/edit/remove item types from the administration module (admin/itemtypes.pl). You should get message feedback after each action. Don't forget to play with the image and try to remove an item type linked to records (you should not be able to remove it). Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 47345 [details] [review] Bug 14828 [QA Followup] - Hide in OPAC checkbox not checked in editor even if enabled Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to Master - Should be in the May 2016 Release. Thanks!
I'm having problems with t/db_dependent/HoldsQueue.t, line 64. $itemtype used to be a string, now is a record.
(In reply to Srdjan Jankovic from comment #33) > I'm having problems with t/db_dependent/HoldsQueue.t, line 64. $itemtype > used to be a string, now is a record. Right, fixed on bug 16671.