Bug 17845 - Remove printer related code
Summary: Remove printer related code
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 15779
  Show dependency treegraph
 
Reported: 2017-01-04 11:27 UTC by Jonathan Druart
Modified: 2018-09-29 18:59 UTC (History)
3 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 17845: Remove unused code related to printers (43.78 KB, patch)
2018-09-16 21:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17845: Remove unused code related to printers (43.92 KB, patch)
2018-09-27 21:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17845: Remove unused code related to printers (43.97 KB, patch)
2018-09-28 13:58 UTC, Owen Leonard
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 11:27:47 UTC
It seems that we want to keep this code, just open this bug report for reference.

See

Bug 8352 - Add automatic printing of 'hold to pull' notices

Bug 16764 - Update printers administration page

Bug 2045 - Network Printers Setting Does Nothing?
Comment 3 Katrin Fischer 2018-09-16 16:24:12 UTC
I am proposins to remove the code until it's needed. At the moment we have a page we need to maintain and keep secure that is not used at all. I think if there was a patch needing that kind if configuration page, it would be easy to bring it back.
Comment 4 Katrin Fischer 2018-09-16 21:31:46 UTC
Created attachment 79008 [details] [review]
Bug 17845: Remove unused code related to printers

There is some quite old and unused code in Koha related
to printer configuration and network printing. These code
hasn't been functional in a long time and should be removed.

This patch:

- Removes printcirculationslips system preference
- Removes table printers
- Removes branchprinter column from branches

Check that:
- Go to administration
- Open any age there, but change the last bit to: printers.pl
- Apply patch, run the database update
- Verify the hidden page no longer exists
- Verify that logging in and out still works correctly
- Verify that checkout and returns work correctly
- Switch to another branch using the "Set library" option
Comment 5 Owen Leonard 2018-09-26 19:09:39 UTC
Got one QA tool error:

 FAIL   circ/returns.pl
   OK     critic
   OK     forbidden patterns
   OK     git manipulation
   OK     pod
   OK     spelling
   FAIL   valid
                Can't locate C4/Print.pm in @INC (you may need to install the C4::Print module) (@INC contains: /home/vagrant/kohaclone /home/vagrant/qa-test-tools /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2 /usr/lib/x86_64-linux-gnu/perl5/5.20 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.20 /usr/share/perl/5.20 /usr/local/lib/site_perl .)

I assume the "POD coverage was greater before" warning is not relevant.
Comment 6 Katrin Fischer 2018-09-27 21:42:47 UTC
Created attachment 79553 [details] [review]
Bug 17845: Remove unused code related to printers

There is some quite old and unused code in Koha related
to printer configuration and network printing. These code
hasn't been functional in a long time and should be removed.

This patch:

- Removes printcirculationslips system preference
- Removes table printers
- Removes branchprinter column from branches

Check that:
- Go to administration
- Open any age there, but change the last bit to: printers.pl
- Apply patch, run the database update
- Verify the hidden page no longer exists
- Verify that logging in and out still works correctly
- Verify that checkout and returns work correctly
- Switch to another branch using the "Set library" option
Comment 7 Katrin Fischer 2018-09-27 21:43:35 UTC
Hi Owen,
I think I forgot to submit that - there was one use C4::Print; left. Fixed now, thx for testing!
Comment 8 Owen Leonard 2018-09-28 13:58:54 UTC
Created attachment 79598 [details] [review]
Bug 17845: Remove unused code related to printers

There is some quite old and unused code in Koha related
to printer configuration and network printing. These code
hasn't been functional in a long time and should be removed.

This patch:

- Removes printcirculationslips system preference
- Removes table printers
- Removes branchprinter column from branches

Check that:
- Go to administration
- Open any age there, but change the last bit to: printers.pl
- Apply patch, run the database update
- Verify the hidden page no longer exists
- Verify that logging in and out still works correctly
- Verify that checkout and returns work correctly
- Switch to another branch using the "Set library" option

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Jonathan Druart 2018-09-29 18:59:37 UTC
Found 3 smalls things:

1. installer/data/mysql/atomicupdate/bug_17845_remove_printers should be a .perl and DROP COLUMN should only be executed if the column exists (column_exists)

2. koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc:    <!-- <li><a href="/cgi-bin/koha/admin/printers.pl">Network Printers</a></li> -->
Can be removed

3. in circulation.tt:
 604           <input type="hidden" name="print" value="maybe" />
Can be removed as well

TODO later: rename selectbranchprinter(.pl|.tt)

Note: I have checked the set_userenv calls and they are all good.