Bug 17845 - Printers related code should be removed
Summary: Printers related code should be removed
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 15779 24837
  Show dependency treegraph
 
Reported: 2017-01-04 11:27 UTC by Jonathan Druart
Modified: 2020-05-18 13:06 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00


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
Bug 17845: Remove unused code related to printers (44.24 KB, patch)
2020-03-09 16:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17845: Update atomic update for new format (1.87 KB, patch)
2020-03-09 17:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17845: (QA follow-up) Remove commented line (1.56 KB, patch)
2020-03-09 17:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17845: (QA follow-up) Remove hidden print input (1.50 KB, patch)
2020-03-09 17:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17845: Remove some missed references to branchprinter (4.46 KB, patch)
2020-03-09 17:00 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 17845: Remove unused code related to printers (44.32 KB, patch)
2020-03-10 10:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17845: Update atomic update for new format (1.94 KB, patch)
2020-03-10 10:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17845: (QA follow-up) Remove commented line (1.63 KB, patch)
2020-03-10 10:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17845: (QA follow-up) Remove hidden print input (1.58 KB, patch)
2020-03-10 10:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17845: Remove some missed references to branchprinter (4.54 KB, patch)
2020-03-10 10:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17845: Adjust few other occurrences (3.11 KB, patch)
2020-03-10 10:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17845: Remove the column if it exists (1.21 KB, patch)
2020-03-10 10:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17845: Schema changes (1.44 KB, patch)
2020-03-10 10:13 UTC, Jonathan Druart
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.
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