Bug 17845

Summary: Printers related code should be removed
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Katrin Fischer <katrin.fischer>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: 1joynelson, ajgar17374, jonathan.druart, katrin.fischer, kyle, martin.renvoize, oleonard, raghuram88585
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=2045
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16764
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8352
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00
Bug Depends on:    
Bug Blocks: 15779, 24837, 29026    
Attachments: Bug 17845: Remove unused code related to printers
Bug 17845: Remove unused code related to printers
Bug 17845: Remove unused code related to printers
Bug 17845: Remove unused code related to printers
Bug 17845: Update atomic update for new format
Bug 17845: (QA follow-up) Remove commented line
Bug 17845: (QA follow-up) Remove hidden print input
Bug 17845: Remove some missed references to branchprinter
Bug 17845: Remove unused code related to printers
Bug 17845: Update atomic update for new format
Bug 17845: (QA follow-up) Remove commented line
Bug 17845: (QA follow-up) Remove hidden print input
Bug 17845: Remove some missed references to branchprinter
Bug 17845: Adjust few other occurrences
Bug 17845: Remove the column if it exists
Bug 17845: Schema changes

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.
Comment 10 Martin Renvoize 2020-03-09 16:59:59 UTC
Created attachment 100396 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2020-03-09 17:00:02 UTC
Created attachment 100397 [details] [review]
Bug 17845: Update atomic update for new format

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2020-03-09 17:00:05 UTC
Created attachment 100398 [details] [review]
Bug 17845: (QA follow-up) Remove commented line

A commented line still refered to the removed page from the
admin-menu.inc

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2020-03-09 17:00:08 UTC
Created attachment 100399 [details] [review]
Bug 17845: (QA follow-up) Remove hidden print input

As highlighted by Jonathan, the hidden 'print' form field is no longer
read and thus no longer required in the template.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2020-03-09 17:00:12 UTC
Created attachment 100400 [details] [review]
Bug 17845: Remove some missed references to branchprinter

Remove a few last references to branchprinter, mostly from tests.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2020-03-09 17:02:06 UTC
Nice work Katrin!

It would be great to see this one finished off.. I've added a few followups to fix the missing parts, address QA concerns and bring it up to date.. and as such didn't feel I should PQA as part of it..

Back to you Jonathan (or any other friendly QA person)
Comment 16 Jonathan Druart 2020-03-10 10:12:55 UTC
Created attachment 100427 [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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2020-03-10 10:12:59 UTC
Created attachment 100428 [details] [review]
Bug 17845: Update atomic update for new format

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Jonathan Druart 2020-03-10 10:13:03 UTC
Created attachment 100429 [details] [review]
Bug 17845: (QA follow-up) Remove commented line

A commented line still refered to the removed page from the
admin-menu.inc

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Jonathan Druart 2020-03-10 10:13:06 UTC
Created attachment 100430 [details] [review]
Bug 17845: (QA follow-up) Remove hidden print input

As highlighted by Jonathan, the hidden 'print' form field is no longer
read and thus no longer required in the template.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 20 Jonathan Druart 2020-03-10 10:13:10 UTC
Created attachment 100431 [details] [review]
Bug 17845: Remove some missed references to branchprinter

Remove a few last references to branchprinter, mostly from tests.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Jonathan Druart 2020-03-10 10:13:14 UTC
Created attachment 100432 [details] [review]
Bug 17845: Adjust few other occurrences

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 22 Jonathan Druart 2020-03-10 10:13:18 UTC
Created attachment 100433 [details] [review]
Bug 17845: Remove the column if it exists

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 23 Jonathan Druart 2020-03-10 10:13:21 UTC
Created attachment 100434 [details] [review]
Bug 17845: Schema changes
Comment 24 Jonathan Druart 2020-03-10 10:15:39 UTC
(In reply to Jonathan Druart from comment #9)

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

I opened bug 24837.
Comment 25 Martin Renvoize 2020-03-10 15:17:31 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 26 Katrin Fischer 2020-03-11 18:12:35 UTC
Thx a lot for finishing this one!
Comment 27 Joy Nelson 2020-04-02 20:08:16 UTC
enhancement not backported to 19.11.x