Bug 17843 - Move C4::Koha::getitemtypeinfo to Koha::ItemTypes
Summary: Move C4::Koha::getitemtypeinfo to Koha::ItemTypes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 17835
Blocks: 15779 18971 19194 19978 21842
  Show dependency treegraph
 
Reported: 2017-01-04 09:30 UTC by Jonathan Druart
Modified: 2019-10-14 19:58 UTC (History)
3 users (show)

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


Attachments
Bug 17843: Repace C4::Koha::getitemtypeinfo with Koha::ItemTypes (15.14 KB, patch)
2017-01-04 09:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17843: Remove C4::Koha::getitemtypeinfo (3.25 KB, patch)
2017-01-04 09:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes (15.21 KB, patch)
2017-04-07 10:53 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17843: Remove C4::Koha::getitemtypeinfo (3.30 KB, patch)
2017-04-07 10:54 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes (15.22 KB, patch)
2017-04-18 08:41 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17843: Remove C4::Koha::getitemtypeinfo (3.30 KB, patch)
2017-04-18 08:41 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes (16.23 KB, patch)
2017-06-16 15:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17843: Remove C4::Koha::getitemtypeinfo (2.28 KB, patch)
2017-06-16 15:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes (16.24 KB, patch)
2017-06-23 09:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17843: Remove C4::Koha::getitemtypeinfo (2.37 KB, patch)
2017-06-23 09:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17843: [QA Follow-up] Some polishing (5.15 KB, patch)
2017-06-23 09:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-01-04 09:30:01 UTC

    
Comment 1 Jonathan Druart 2017-01-04 09:43:58 UTC
Created attachment 58583 [details] [review]
Bug 17843: Repace C4::Koha::getitemtypeinfo with Koha::ItemTypes

The C4::Koha::getitemtypeinfo subroutine did the almost same job as
GetItemTypes. On top of that it returned the imageurl value processed by
C4::Koha::getitemtypeimagelocation.
This value is only used from the 2 [opac-]shelves.pl scripts. Then it's
better not retrieve it only when we need it.

Test plan:
Play with the different scripts touched by this patch and focus on item
types. The same description as prior to this patch must be displayed.
Note that sometimes it is not the translated description which is
displayed, but that should be fixed on another bug report. Indeed we do
not expect this patch to change any behaviors.
Comment 2 Jonathan Druart 2017-01-04 09:44:04 UTC
Created attachment 58584 [details] [review]
Bug 17843: Remove C4::Koha::getitemtypeinfo

At this point there should not be any calls to this subroutine.
Comment 3 Lari Taskula 2017-04-07 10:53:58 UTC
Created attachment 61972 [details] [review]
Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes

The C4::Koha::getitemtypeinfo subroutine did the almost same job as
GetItemTypes. On top of that it returned the imageurl value processed by
C4::Koha::getitemtypeimagelocation.
This value is only used from the 2 [opac-]shelves.pl scripts. Then it's
better not retrieve it only when we need it.

Test plan:
Play with the different scripts touched by this patch and focus on item
types. The same description as prior to this patch must be displayed.
Note that sometimes it is not the translated description which is
displayed, but that should be fixed on another bug report. Indeed we do
not expect this patch to change any behaviors.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 4 Lari Taskula 2017-04-07 10:54:12 UTC
Created attachment 61973 [details] [review]
Bug 17843: Remove C4::Koha::getitemtypeinfo

At this point there should not be any calls to this subroutine.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 5 Lari Taskula 2017-04-07 10:56:41 UTC
Rebased on top of master, fixed a minor conflict and typo in patch title. Tested scripts touched by first patch. Switching to Signed Off.
Comment 6 Lari Taskula 2017-04-18 08:41:46 UTC
Created attachment 62266 [details] [review]
Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes

The C4::Koha::getitemtypeinfo subroutine did the almost same job as
GetItemTypes. On top of that it returned the imageurl value processed by
C4::Koha::getitemtypeimagelocation.
This value is only used from the 2 [opac-]shelves.pl scripts. Then it's
better not retrieve it only when we need it.

Test plan:
Play with the different scripts touched by this patch and focus on item
types. The same description as prior to this patch must be displayed.
Note that sometimes it is not the translated description which is
displayed, but that should be fixed on another bug report. Indeed we do
not expect this patch to change any behaviors.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 7 Lari Taskula 2017-04-18 08:41:57 UTC
Created attachment 62267 [details] [review]
Bug 17843: Remove C4::Koha::getitemtypeinfo

At this point there should not be any calls to this subroutine.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 8 Jonathan Druart 2017-06-16 15:35:51 UTC
Created attachment 64400 [details] [review]
Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes

The C4::Koha::getitemtypeinfo subroutine did the almost same job as
GetItemTypes. On top of that it returned the imageurl value processed by
C4::Koha::getitemtypeimagelocation.
This value is only used from the 2 [opac-]shelves.pl scripts. Then it's
better not retrieve it only when we need it.

Test plan:
Play with the different scripts touched by this patch and focus on item
types. The same description as prior to this patch must be displayed.
Note that sometimes it is not the translated description which is
displayed, but that should be fixed on another bug report. Indeed we do
not expect this patch to change any behaviors.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 9 Jonathan Druart 2017-06-16 15:36:00 UTC
Created attachment 64401 [details] [review]
Bug 17843: Remove C4::Koha::getitemtypeinfo

At this point there should not be any calls to this subroutine.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 10 Marcel de Rooy 2017-06-23 08:40:09 UTC
QA: Looking here now
Comment 11 Marcel de Rooy 2017-06-23 09:51:12 UTC
Created attachment 64582 [details] [review]
Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes

The C4::Koha::getitemtypeinfo subroutine did the almost same job as
GetItemTypes. On top of that it returned the imageurl value processed by
C4::Koha::getitemtypeimagelocation.
This value is only used from the 2 [opac-]shelves.pl scripts. Then it's
better not retrieve it only when we need it.

Test plan:
Play with the different scripts touched by this patch and focus on item
types. The same description as prior to this patch must be displayed.
Note that sometimes it is not the translated description which is
displayed, but that should be fixed on another bug report. Indeed we do
not expect this patch to change any behaviors.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2017-06-23 09:51:16 UTC
Created attachment 64583 [details] [review]
Bug 17843: Remove C4::Koha::getitemtypeinfo

At this point there should not be any calls to this subroutine.

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2017-06-23 09:51:20 UTC
Created attachment 64584 [details] [review]
Bug 17843: [QA Follow-up] Some polishing

Resolve warning from members/summary-print.pl:
    "my" variable $itemtype masks earlier declaration in same scope

Test if find returns a Koha object in GetDescription.
Test if find returns a Koha object too in shelves.pl. While testing, I had
a crash on a biblioitem with itemtype NULL (bad record, but these things
tend to happen somehow.)
Can't call method "imageurl" on an undefined value at virtualshelves/shelves.pl line 253.
Same for opac/opac-shelves.pl.

Note: Did not add tests everywhere but generally, I have the impression that
we do not sufficiently test on the results of Koha::Object->find. Mostly we
just assume that it will find a record. Several reports include fixes to
resolve that wrong assumption.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Jonathan Druart 2017-07-05 17:05:35 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 15 Fridolin Somers 2017-07-10 10:11:38 UTC
Enhancement not pushed to 17.05.x