Bug 29111 - Remove dead code from intranet
Summary: Remove dead code from intranet
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Joonas Kylmälä
QA Contact: Testopia
URL:
Keywords:
Depends on: 27526
Blocks:
  Show dependency treegraph
 
Reported: 2021-09-26 07:24 UTC by Joonas Kylmälä
Modified: 2021-10-22 15:02 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Text to go in the release notes:
Version(s) released in:
21.11.00


Attachments
Bug 29111: Remove dead code from acqui/addorderiso2709.pl (1.41 KB, patch)
2021-09-26 08:21 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 29111: Remove unused indicator modification code for items (10.29 KB, patch)
2021-09-26 08:22 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 29111: Remove unused function from cataloguing/additem.pl (1.26 KB, patch)
2021-09-26 08:22 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 29111: Remove dead code from acqui/addorderiso2709.pl (1.46 KB, patch)
2021-10-01 11:09 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 29111: Remove unused indicator modification code for items (10.34 KB, patch)
2021-10-01 11:09 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 29111: Remove unused function from cataloguing/additem.pl (1.31 KB, patch)
2021-10-01 11:09 UTC, Peter Vashchuk
Details | Diff | Splinter Review
Bug 29111: Remove dead code from acqui/addorderiso2709.pl (1.52 KB, patch)
2021-10-02 20:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29111: Remove unused indicator modification code for items (10.45 KB, patch)
2021-10-02 20:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29111: Remove unused function from cataloguing/additem.pl (1.37 KB, patch)
2021-10-02 20:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29111: Remove dead code from acqui/addorderiso2709.pl (1.52 KB, patch)
2021-10-10 08:44 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 29111: Remove unused indicator modification code for items (9.02 KB, patch)
2021-10-10 08:44 UTC, Joonas Kylmälä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2021-09-26 07:24:48 UTC
At a quick glance it looks like Koha doesn't support placing indicators to the item field (952) but we have code for doing that in the .pl scripts, for example in:

tools/batchMod.pl
cataloguing/additem.pl
acqui/addorderiso2709.pl

Please search for "indicator" and "ind_tag" in these files to locate the unused code. For cataloguing/additem.pl it is clear the code is unused because it is hard coded in the .tt (template toolkit) file that the indicator values are always " " / empty.

This code should be removed so we are not unnecessarily spending time doing modifications relating to this part of code to keep it "working" (bug 28832...).
Comment 1 Joonas Kylmälä 2021-09-26 08:21:58 UTC
Created attachment 125311 [details] [review]
Bug 29111: Remove dead code from acqui/addorderiso2709.pl

Test plan:
 1) open acqui/addorderiso2709.pl in your code editor and make sure
 nothing references $item in the if block where it was removed from
Comment 2 Joonas Kylmälä 2021-09-26 08:22:01 UTC
Created attachment 125312 [details] [review]
Bug 29111: Remove unused indicator modification code for items

The indicator value for 952 was hard coded in every case to " ". In
order to achieve that we can simply pass undef to TransformHtmlToXml()
and it will set the indicator values to " ".

To test:
 1) Make sure the submission of (at least some) the modified files
 still work, e.g. test that making a new item via
 cataloguing/additem.pl works.
Comment 3 Joonas Kylmälä 2021-09-26 08:22:05 UTC
Created attachment 125313 [details] [review]
Bug 29111: Remove unused function from cataloguing/additem.pl

To test:
 1) Open cataloguing/additem.pl and search for any usages of
 find_value, you should find 0 usages.
Comment 4 Joonas Kylmälä 2021-09-26 08:22:46 UTC
There was some additional dead code around the indicator values so those are removed here as well.
Comment 5 Peter Vashchuk 2021-10-01 11:09:00 UTC
Created attachment 125601 [details] [review]
Bug 29111: Remove dead code from acqui/addorderiso2709.pl

Test plan:
 1) open acqui/addorderiso2709.pl in your code editor and make sure
 nothing references $item in the if block where it was removed from

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 6 Peter Vashchuk 2021-10-01 11:09:05 UTC
Created attachment 125602 [details] [review]
Bug 29111: Remove unused indicator modification code for items

The indicator value for 952 was hard coded in every case to " ". In
order to achieve that we can simply pass undef to TransformHtmlToXml()
and it will set the indicator values to " ".

To test:
 1) Make sure the submission of (at least some) the modified files
 still work, e.g. test that making a new item via
 cataloguing/additem.pl works.

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 7 Peter Vashchuk 2021-10-01 11:09:10 UTC
Created attachment 125603 [details] [review]
Bug 29111: Remove unused function from cataloguing/additem.pl

To test:
 1) Open cataloguing/additem.pl and search for any usages of
 find_value, you should find 0 usages.

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Comment 8 Katrin Fischer 2021-10-02 20:15:13 UTC
I've also tested the ordering, receiving, adding and batch modifying items in cataloguing and adding items from the serial module.
Comment 9 Katrin Fischer 2021-10-02 20:16:22 UTC
Created attachment 125652 [details] [review]
Bug 29111: Remove dead code from acqui/addorderiso2709.pl

Test plan:
 1) open acqui/addorderiso2709.pl in your code editor and make sure
 nothing references $item in the if block where it was removed from

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2021-10-02 20:16:27 UTC
Created attachment 125653 [details] [review]
Bug 29111: Remove unused indicator modification code for items

The indicator value for 952 was hard coded in every case to " ". In
order to achieve that we can simply pass undef to TransformHtmlToXml()
and it will set the indicator values to " ".

To test:
 1) Make sure the submission of (at least some) the modified files
 still work, e.g. test that making a new item via
 cataloguing/additem.pl works.

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Katrin Fischer 2021-10-02 20:16:32 UTC
Created attachment 125654 [details] [review]
Bug 29111: Remove unused function from cataloguing/additem.pl

To test:
 1) Open cataloguing/additem.pl and search for any usages of
 find_value, you should find 0 usages.

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Jonathan Druart 2021-10-06 08:47:43 UTC
Comment on attachment 125654 [details] [review]
Bug 29111: Remove unused function from cataloguing/additem.pl

Done on bug 27526 (Adjust code to use Koha::Items), obsoleting.
Comment 13 Jonathan Druart 2021-10-06 08:59:01 UTC
Does not apply on top of bug 27526. At first glance I'd say that the changes to additem are no longer valid (I cleaned them already).
Comment 14 Joonas Kylmälä 2021-10-10 08:44:43 UTC
Created attachment 125996 [details] [review]
Bug 29111: Remove dead code from acqui/addorderiso2709.pl

Test plan:
 1) open acqui/addorderiso2709.pl in your code editor and make sure
 nothing references $item in the if block where it was removed from

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Joonas Kylmälä 2021-10-10 08:44:47 UTC
Created attachment 125997 [details] [review]
Bug 29111: Remove unused indicator modification code for items

The indicator value for 952 was hard coded in every case to " ". In
order to achieve that we can simply pass undef to TransformHtmlToXml()
and it will set the indicator values to " ".

To test:
 1) Make sure the submission of (at least some) the modified files
 still work, e.g. test that making a new item via
 cataloguing/additem.pl works.

Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Rebased-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 16 Joonas Kylmälä 2021-10-10 08:45:03 UTC
(In reply to Jonathan Druart from comment #13)
> Does not apply on top of bug 27526. At first glance I'd say that the changes
> to additem are no longer valid (I cleaned them already).

Rebased.
Comment 17 Jonathan Druart 2021-10-15 09:29:32 UTC
Pushed to master for 21.11, thanks to everybody involved!