Bug 32551 - API requests don't carry language related information
Summary: API requests don't carry language related information
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-12-30 13:23 UTC by Katrin Fischer
Modified: 2024-01-19 22:36 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.02,23.05.08


Attachments
Bug 32551: Regression tests (4.13 KB, patch)
2023-12-28 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32551: HTTP_ACCEPT_LANGUAGE legacy CGI variable not set in Mojolicious (1.70 KB, patch)
2023-12-28 14:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 32551: Regression tests (4.17 KB, patch)
2023-12-28 19:12 UTC, David Nind
Details | Diff | Splinter Review
Bug 32551: HTTP_ACCEPT_LANGUAGE legacy CGI variable not set in Mojolicious (1.75 KB, patch)
2023-12-28 19:12 UTC, David Nind
Details | Diff | Splinter Review
Bug 32551: Regression tests (4.23 KB, patch)
2024-01-05 19:34 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 32551: HTTP_ACCEPT_LANGUAGE legacy CGI variable not set in Mojolicious (1.80 KB, patch)
2024-01-05 19:34 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-12-30 13:23:14 UTC
During our ILL development we noticed that there are issues with translations and the API. Using the C4::Languages::getlanguage method from the API controller is not working because the cookies is not passed and the environment variables are lost.
Comment 1 Katrin Fischer 2023-10-23 08:48:11 UTC
As part of our work on an ILL backend we have implemented this solution in  Koha/REST/V1/Illrequests.pm. It works really well, but of course only for this small part. 

Is there way the solution could be generalized?


Koha/REST/V1/Illrequests.pm
sub list {
    my $c = shift->openapi->valid_input or return;

    eval { $ENV{HTTP_ACCEPT_LANGUAGE} = $c->req->cookie('KohaOpacLanguage')->value; };

    return try {
Comment 2 Katrin Fischer 2023-12-27 15:45:55 UTC
Any way we could help to move this forward?
Comment 3 Tomás Cohen Arazi 2023-12-27 15:46:34 UTC
(In reply to Katrin Fischer from comment #2)
> Any way we could help to move this forward?

You did. Thanks for the reminder 🎗️
Comment 4 Tomás Cohen Arazi 2023-12-28 14:53:13 UTC
Created attachment 160354 [details] [review]
Bug 32551: Regression tests
Comment 5 Tomás Cohen Arazi 2023-12-28 14:53:17 UTC
Created attachment 160355 [details] [review]
Bug 32551: HTTP_ACCEPT_LANGUAGE legacy CGI variable not set in Mojolicious

This patch addresses the fact some of our code still relies on those CGI-era ENV variables.
In particular, C4::Laguages::getlanguage expects it to calculate the most suitable language.

As this is reported as required, I provide a fix.

I did a `git grep HTTP_` on the codebase, and the only other occurences are:

* C4::Output (HTTP_X_REQUESTED_WITH)
* C4::Context (HTTP_USER_AGENT, HTTP_X_FORWARDED_FOR)
* C4::Auth_with_shibboleth (using HTTP_ variables for matchpoints)
* Some handling on the 'Koha as a Mojo app' code
* Some .pl script not relevant to this report

I decided to limit this patch to the requested header which seems harmless. I leave the
other cases.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ qa -c 1
=> FAIL: Tests fail!
3. Apply this patch
4. Run:
  k$ qa -c 2
=> SUCCESS: Tests pass! All green!
5. Sign off :-D
Comment 6 David Nind 2023-12-28 19:12:21 UTC
Created attachment 160358 [details] [review]
Bug 32551: Regression tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2023-12-28 19:12:24 UTC
Created attachment 160359 [details] [review]
Bug 32551: HTTP_ACCEPT_LANGUAGE legacy CGI variable not set in Mojolicious

This patch addresses the fact some of our code still relies on those CGI-era ENV variables.
In particular, C4::Laguages::getlanguage expects it to calculate the most suitable language.

As this is reported as required, I provide a fix.

I did a `git grep HTTP_` on the codebase, and the only other occurences are:

* C4::Output (HTTP_X_REQUESTED_WITH)
* C4::Context (HTTP_USER_AGENT, HTTP_X_FORWARDED_FOR)
* C4::Auth_with_shibboleth (using HTTP_ variables for matchpoints)
* Some handling on the 'Koha as a Mojo app' code
* Some .pl script not relevant to this report

I decided to limit this patch to the requested header which seems harmless. I leave the
other cases.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ qa -c 1
=> FAIL: Tests fail!
3. Apply this patch
4. Run:
  k$ qa -c 2
=> SUCCESS: Tests pass! All green!
5. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 Nick Clemens (kidclamp) 2024-01-05 19:34:33 UTC
Created attachment 160585 [details] [review]
Bug 32551: Regression tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 9 Nick Clemens (kidclamp) 2024-01-05 19:34:35 UTC
Created attachment 160586 [details] [review]
Bug 32551: HTTP_ACCEPT_LANGUAGE legacy CGI variable not set in Mojolicious

This patch addresses the fact some of our code still relies on those CGI-era ENV variables.
In particular, C4::Laguages::getlanguage expects it to calculate the most suitable language.

As this is reported as required, I provide a fix.

I did a `git grep HTTP_` on the codebase, and the only other occurences are:

* C4::Output (HTTP_X_REQUESTED_WITH)
* C4::Context (HTTP_USER_AGENT, HTTP_X_FORWARDED_FOR)
* C4::Auth_with_shibboleth (using HTTP_ variables for matchpoints)
* Some handling on the 'Koha as a Mojo app' code
* Some .pl script not relevant to this report

I decided to limit this patch to the requested header which seems harmless. I leave the
other cases.

To test:
1. Apply the regression tests patch
2. Run:
   $ ktd --shell
  k$ qa -c 1
=> FAIL: Tests fail!
3. Apply this patch
4. Run:
  k$ qa -c 2
=> SUCCESS: Tests pass! All green!
5. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 10 Katrin Fischer 2024-01-11 14:49:46 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 11 Fridolin Somers 2024-01-15 14:16:58 UTC
Pushed to 23.11.x for 23.11.02
Comment 12 Lucas Gass 2024-01-19 22:36:29 UTC
Backported to 23.05.x for upcoming 23.05.08