Bug 14828 - Move the item types related code to Koha::ItemTypes
Summary: Move the item types related code to Koha::ItemTypes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 14100
Blocks: 15449 15306 15730 16114 16449 16671 17657 18525 19646
  Show dependency treegraph
 
Reported: 2015-09-15 16:31 UTC by Jonathan Druart
Modified: 2018-12-03 20:05 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14828: Add Koha::ItemType[s] classes (3.85 KB, patch)
2015-09-15 16:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: use Koha::ItemType[s] in admin/itemtypes (40.70 KB, patch)
2015-09-15 16:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used (10.13 KB, patch)
2015-09-15 16:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: Remove old package and tests (7.80 KB, patch)
2015-09-15 16:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: Add Koha::ItemType[s] classes (3.85 KB, patch)
2015-12-17 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: use Koha::ItemType[s] in admin/itemtypes (44.77 KB, patch)
2015-12-17 10:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used (10.76 KB, patch)
2015-12-17 10:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: Remove old package and tests (8.14 KB, patch)
2015-12-17 10:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: use Koha::ItemType[s] in admin/itemtypes (44.81 KB, patch)
2015-12-17 12:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14828: Add Koha::ItemType[s] classes (3.98 KB, patch)
2015-12-17 14:13 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used (10.90 KB, patch)
2015-12-17 14:15 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14828: Remove old package and tests (8.28 KB, patch)
2015-12-17 14:21 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14828: use Koha::ItemType[s] in admin/itemtypes (44.95 KB, patch)
2015-12-17 14:23 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14828: Add Koha::ItemType[s] classes (4.03 KB, patch)
2016-01-15 19:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used (10.95 KB, patch)
2016-01-15 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828: Remove old package and tests (8.33 KB, patch)
2016-01-15 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828: use Koha::ItemType[s] in admin/itemtypes (45.00 KB, patch)
2016-01-15 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828 [QA Followup] - Hide in OPAC checkbox not checked in editor even if enabled (1.20 KB, patch)
2016-01-15 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828: Add Koha::ItemType[s] classes (4.03 KB, patch)
2016-01-27 13:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828: Use Koha::ItemType[s] everywhere C4::ItemType was used (10.99 KB, patch)
2016-01-27 13:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828: Remove old package and tests (8.33 KB, patch)
2016-01-27 13:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828: use Koha::ItemType[s] in admin/itemtypes (45.00 KB, patch)
2016-01-27 13:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14828 [QA Followup] - Hide in OPAC checkbox not checked in editor even if enabled (1.20 KB, patch)
2016-01-27 13:30 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2015-09-15 16:31:57 UTC

    
Comment 1 Jonathan Druart 2015-09-15 16:40:34 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-09-15 16:40:37 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-09-15 16:40:40 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-09-15 16:40:43 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2015-09-15 16:44:50 UTC
The next step will be to replace C4::ItemTypes calls with Koha::ItemTypes->search
Comment 6 Josef Moravec 2015-12-16 13:23:07 UTC
Could you please rebase?
Comment 7 Jonathan Druart 2015-12-17 10:08:59 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-12-17 10:09:03 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-12-17 10:09:07 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2015-12-17 10:09:11 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2015-12-17 10:10:40 UTC
Quite tricky conflicts (with bug 10937, bug 14100, etc.) fixed, patches rebase against master.
Comment 12 Josef Moravec 2015-12-17 11:48:51 UTC
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.
Comment 13 Jonathan Druart 2015-12-17 12:54:22 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2015-12-17 12:57:15 UTC
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.
Comment 15 Marc Véron 2015-12-17 14:13:51 UTC Comment hidden (obsolete)
Comment 16 Marc Véron 2015-12-17 14:15:18 UTC Comment hidden (obsolete)
Comment 17 Marc Véron 2015-12-17 14:21:36 UTC Comment hidden (obsolete)
Comment 18 Marc Véron 2015-12-17 14:23:34 UTC Comment hidden (obsolete)
Comment 19 Marc Véron 2015-12-17 14:25:21 UTC
Tested all patches together, followed test plan from commentt #16, worked as expected.
Comment 20 Jonathan Druart 2015-12-17 14:41:10 UTC
> 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
Comment 21 Josef Moravec 2015-12-18 12:12:07 UTC
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 ;)
Comment 22 Kyle M Hall 2016-01-15 19:15:54 UTC Comment hidden (obsolete)
Comment 23 Kyle M Hall 2016-01-15 19:16:02 UTC Comment hidden (obsolete)
Comment 24 Kyle M Hall 2016-01-15 19:16:05 UTC Comment hidden (obsolete)
Comment 25 Kyle M Hall 2016-01-15 19:16:08 UTC Comment hidden (obsolete)
Comment 26 Kyle M Hall 2016-01-15 19:16:11 UTC Comment hidden (obsolete)
Comment 27 Kyle M Hall 2016-01-27 13:30:27 UTC
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>
Comment 28 Kyle M Hall 2016-01-27 13:30:35 UTC
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>
Comment 29 Kyle M Hall 2016-01-27 13:30:38 UTC
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>
Comment 30 Kyle M Hall 2016-01-27 13:30:41 UTC
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>
Comment 31 Kyle M Hall 2016-01-27 13:30:43 UTC
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>
Comment 32 Brendan Gallagher 2016-01-27 20:51:29 UTC
Pushed to Master - Should be in the May 2016 Release.  Thanks!
Comment 33 Srdjan Jankovic 2016-06-06 03:59:15 UTC
I'm having problems with t/db_dependent/HoldsQueue.t, line 64. $itemtype used to be a string, now is a record.
Comment 34 Jonathan Druart 2016-06-06 13:04:39 UTC
(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.