Bug 27246 - Remove apache only code from C4::Context BEGIN
Summary: Remove apache only code from C4::Context BEGIN
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 enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 8389 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-12-16 10:04 UTC by Jonathan Druart
Modified: 2021-05-11 12:08 UTC (History)
7 users (show)

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


Attachments
Bug 27246: Remove obsolete BEGIN code from C4::Context (4.05 KB, patch)
2020-12-16 10:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27246: Remove obsolete BEGIN code from C4::Context (4.16 KB, patch)
2021-04-30 15:26 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 27246: Remove obsolete BEGIN code from C4::Context (4.22 KB, patch)
2021-05-04 10:48 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-12-16 10:04:41 UTC

    
Comment 1 Jonathan Druart 2020-12-16 10:10:29 UTC
Created attachment 114431 [details] [review]
Bug 27246: Remove obsolete BEGIN code from C4::Context

We don't longer enter the HTTP_USER_AGENT conditional in the BEGIN block
of C4::Context. It was used when apache only was used.

I am suggesting to remove it but keep the $CGI::VERSION < 4.08 condition
to keep support of all version (not sure it's needed but won't hurt).

I have not found a lot of occurrences of KOHA_BACKTRACES and I am
assuming not much people are using it, we should prefer the dev_install
flag instead (to set environment="development" when plack is started and
enabled the starman trace)

Test plan:
Regression test what you think is useful
Confirm the above statements
Comment 2 Jonathan Druart 2020-12-16 10:12:17 UTC
*** Bug 8389 has been marked as a duplicate of this bug. ***
Comment 3 Victor Grousset/tuxayo 2021-04-28 04:36:31 UTC
Not worth a signoff but here is some random testing:

applied patch
restarted services (in koha-testing-docker)
ran the GUI tests: prove -r t/db_dependent/selenium/
went to OPAC
went to staff interface
did a checkout, showed it in the patrons page (to test the related API call)

restarted koha-testing-docker to see an eventual difference compared to  applying when already started
Comment 4 David Cook 2021-04-28 04:59:29 UTC
Thumbs up for removing all that stuff
Comment 5 Victor Grousset/tuxayo 2021-04-29 16:15:50 UTC
dcook, any hint of what I should test?
Comment 6 David Cook 2021-04-29 23:07:51 UTC
(In reply to Victor Grousset/tuxayo from comment #5)
> dcook, any hint of what I should test?

Honestly, I don't think that functionality has worked in a long time if ever. There's not really much to test except making sure that no typo was introduced, so just loading Koha is probably sufficient. Making sure C4::Context compiles.
Comment 7 Victor Grousset/tuxayo 2021-04-30 15:26:04 UTC
Created attachment 120346 [details] [review]
Bug 27246: Remove obsolete BEGIN code from C4::Context

We don't longer enter the HTTP_USER_AGENT conditional in the BEGIN block
of C4::Context. It was used when apache only was used.

I am suggesting to remove it but keep the $CGI::VERSION < 4.08 condition
to keep support of all version (not sure it's needed but won't hurt).

I have not found a lot of occurrences of KOHA_BACKTRACES and I am
assuming not much people are using it, we should prefer the dev_install
flag instead (to set environment="development" when plack is started and
enabled the starman trace)

Test plan:
Regression test what you think is useful
Confirm the above statements

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 8 Victor Grousset/tuxayo 2021-04-30 15:26:56 UTC
Then we can sign it off :) Including David in it for the analysis ^^
Comment 9 Martin Renvoize 2021-05-04 10:48:33 UTC
Created attachment 120436 [details] [review]
Bug 27246: Remove obsolete BEGIN code from C4::Context

We don't longer enter the HTTP_USER_AGENT conditional in the BEGIN block
of C4::Context. It was used when apache only was used.

I am suggesting to remove it but keep the $CGI::VERSION < 4.08 condition
to keep support of all version (not sure it's needed but won't hurt).

I have not found a lot of occurrences of KOHA_BACKTRACES and I am
assuming not much people are using it, we should prefer the dev_install
flag instead (to set environment="development" when plack is started and
enabled the starman trace)

Test plan:
Regression test what you think is useful
Confirm the above statements

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2021-05-04 10:49:39 UTC
Nice bit of cleanup, no regressions spotted and I agree with David's conclusion that I don't think it's been used for quite some time.

Passing QA
Comment 11 Jonathan Druart 2021-05-04 13:24:34 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 12 Fridolin Somers 2021-05-11 12:08:24 UTC
Enhancement not pushed to 20.11.x