Bug 28955

Summary: Add option to set default branch from Apache
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: OPACAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: arthur.suzuki, dcook, fridolin.somers, jake.deery, janet.mcgowan, jonathan.druart, katrin.fischer, kyle, nick, victor
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29691
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Martin Renvoize Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/568
Text to go in the release notes:
Add support for OPAC_BRANCH_DEFAULT as an environment option. It allows setting a default branch for the anonymous OPAC session such that you can display the right OPAC content blocks prior to login if you have set up per branch website.
Version(s) released in:
22.05.00; 21.11.07
Bug Depends on: 29332    
Bug Blocks: 29691    
Attachments: Bug 28955: Add OPAC_BRANCH_DEFAULT
Bug 28955: Add OPAC_BRANCH_DEFAULT
Bug 28955: Add OPAC_BRANCH_DEFAULT
Bug 28955: (follow-up) Set to fall back
Bug 28955: Add OPAC_BRANCH_DEFAULT
Bug 28955: (follow-up) Set to fall back
Bug 28955: Add OPAC_BRANCH_DEFAULT
Bug 28955: (follow-up) Set to fall back
Bug 28955: Impact News as well
Bug 28955: Add OPAC_BRANCH_DEFAULT
Bug 28955: (follow-up) Set to fall back
Bug 28955: Impact News as well
Bug 28955: Add OPAC_BRANCH_DEFAULT [21.11]
Bug 28955: (follow-up) Set to fall back [21.11]
Bug 28955: Impact News as well [21.11]

Description Martin Renvoize 2021-09-06 09:17:57 UTC
With the moved of some system preferences to News blocks we need a way to set the default branch from apache such that we can display the right block prior to users login.

We used to use SetEnvIf to set various prefs per domain.. it would be great to be able to do the same from branchcode so the News blocks behave as expected.
Comment 1 David Cook 2021-09-08 00:01:37 UTC
With a Plack-enabled site, I imagine we'd need to send a custom http header to achieve that?
Comment 2 David Cook 2021-10-21 22:57:27 UTC
I wonder if we should actually implement this in Plack, as I think a long-term goal would be to move away from Apache httpd as the reverse proxy.

It could help move towards a more multitenanted Koha as well. Might be worthwhile looking at Julian's work on that...
Comment 3 Martin Renvoize 2021-10-27 13:04:04 UTC
Created attachment 126989 [details] [review]
Bug 28955: Add OPAC_BRANCH_DEFAULT

This patch adds support for OPAC_BRANCH_DEFAULT as an environment option
that can be passed via apache with either SetEnv or as a header for
plack.  It allows setting a default branch for the anonymous opac
session such that you can display the right opac content blocks prior to
login if you have set up per branch URI's.
Comment 4 Nick Clemens 2021-12-14 13:43:04 UTC
Created attachment 128507 [details] [review]
Bug 28955: Add OPAC_BRANCH_DEFAULT

This patch adds support for OPAC_BRANCH_DEFAULT as an environment option
that can be passed via apache with either SetEnv or as a header for
plack.  It allows setting a default branch for the anonymous opac
session such that you can display the right opac content blocks prior to
login if you have set up per branch URI's.

To test (on top of bug 29691)
1 - Add to apache conf (/etc/apache2/sites-available/kohadev.conf)
   SetEnv OPAC_BRANCH_DEFAULT "CPL"
   RequestHeader add X-Koha-SetEnv "OPAC_BRANCH_DEFAULT CPL"
2 - Restart all
3 - Confirm that news for all and CPL show on opac mainpage
4 - Sign in as a different library
5 - Confirm users library news shows

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Martin Renvoize 2021-12-14 13:53:52 UTC
I'm escalating this one a bit as really it's a regression rather than an enhancement.
Comment 6 Jonathan Druart 2021-12-15 13:10:22 UTC
1. Shouldn't only be passed to the templates for OPAC interface?

2. You are assuming that this "branchcode" variable is passed only for this purpose. Is that true?

3. Where is this "branchcode" actually passed to the template?

4. Did you see that sometimes "logged_in_user.branchcode" is used (opac-main.tt)?
Not sure this is correct actually.
Comment 7 Jonathan Druart 2021-12-15 13:13:16 UTC
(In reply to Martin Renvoize from comment #5)
> I'm escalating this one a bit as really it's a regression rather than an
> enhancement.

How is it a regression? I don't think it was possible before either.
Comment 8 Martin Renvoize 2021-12-15 13:39:23 UTC
It's a regression in so much as when these were system preferences rather than news blocks you could set them in the apache environment to get a different OPAC look depending on the URL of the virtual host.  That is impossible since the move from syspref to news and is causing a number of our upgrades pain.
Comment 9 Martin Renvoize 2021-12-15 14:38:10 UTC
Created attachment 128569 [details] [review]
Bug 28955: Add OPAC_BRANCH_DEFAULT

This patch adds support for OPAC_BRANCH_DEFAULT as an environment option
that can be passed via apache with either SetEnv or as a header for
plack.  It allows setting a default branch for the anonymous opac
session such that you can display the right opac content blocks prior to
login if you have set up per branch URI's.
Comment 10 Martin Renvoize 2021-12-15 14:38:14 UTC
Created attachment 128570 [details] [review]
Bug 28955: (follow-up) Set to fall back

This patch updates the original fix to only set the template parameter
for opac sessions and updates all occurences in templats to check first
for logged_in_user.branchcode before falling back to default_branch
Comment 11 Martin Renvoize 2021-12-15 14:40:50 UTC
(In reply to Jonathan Druart from comment #6)
> 1. Shouldn't only be passed to the templates for OPAC interface?

Done

> 2. You are assuming that this "branchcode" variable is passed only for this
> purpose. Is that true?

Renamed to 'default_branch'

> 3. Where is this "branchcode" actually passed to the template?

I've switched to using a new default_branch and it's always passed for OPAC templates

> 4. Did you see that sometimes "logged_in_user.branchcode" is used
> (opac-main.tt)?
> Not sure this is correct actually.

It looks like most occurrences have no actually switched from 'branchcode' to 'logged_in_user.branchcode'.. I've updated all templates to use `logged_in_branch.branchcode || default_branch`
Comment 12 Jonathan Druart 2022-02-18 09:43:47 UTC
Can you rebase pleae?

error: sha1 information is lacking or useless (C4/Auth.pm).
Comment 13 Martin Renvoize 2022-02-18 11:24:15 UTC
Created attachment 130819 [details] [review]
Bug 28955: Add OPAC_BRANCH_DEFAULT

This patch adds support for OPAC_BRANCH_DEFAULT as an environment option
that can be passed via apache with either SetEnv or as a header for
plack.  It allows setting a default branch for the anonymous opac
session such that you can display the right opac content blocks prior to
login if you have set up per branch URI's.
Comment 14 Martin Renvoize 2022-02-18 11:24:19 UTC
Created attachment 130820 [details] [review]
Bug 28955: (follow-up) Set to fall back

This patch updates the original fix to only set the template parameter
for opac sessions and updates all occurences in templats to check first
for logged_in_user.branchcode before falling back to default_branch
Comment 15 Martin Renvoize 2022-02-18 11:27:39 UTC
Created attachment 130823 [details] [review]
Bug 28955: Add OPAC_BRANCH_DEFAULT

This patch adds support for OPAC_BRANCH_DEFAULT as an environment option
that can be passed via apache with either SetEnv or as a header for
plack.  It allows setting a default branch for the anonymous opac
session such that you can display the right opac content blocks prior to
login if you have set up per branch URI's.

To test (on top of bug 29691)
1 - Add to apache conf (/etc/apache2/sites-available/kohadev.conf)
    SetEnv OPAC_BRANCH_DEFAULT "CPL"
    RequestHeader add X-Koha-SetEnv "OPAC_BRANCH_DEFAULT CPL"
2 - Restart all
3 - Confirm that news for all and CPL show on opac mainpage
4 - Sign in as a different library
5 - Confirm users library news shows

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Martin Renvoize 2022-02-18 11:27:46 UTC
Created attachment 130824 [details] [review]
Bug 28955: (follow-up) Set to fall back

This patch updates the original fix to only set the template parameter
for opac sessions and updates all occurences in templats to check first
for logged_in_user.branchcode before falling back to default_branch
Comment 17 Martin Renvoize 2022-02-18 11:30:02 UTC
Rebased :)
Comment 18 Jonathan Druart 2022-02-18 14:52:36 UTC
> 3 - Confirm that news for all and CPL show on opac mainpage

This is failing. We are not impacting "News", but "HTML customizations". Is the test plan wrong or we are missing something in the patch? What was the previous behaviour?
Comment 19 Jonathan Druart 2022-02-18 14:55:28 UTC
Created attachment 130862 [details] [review]
Bug 28955: Impact News as well
Comment 20 Jonathan Druart 2022-02-18 14:56:04 UTC
(In reply to Jonathan Druart from comment #19)
> Created attachment 130862 [details] [review] [review]
> Bug 28955: Impact News as well

We could go with this trivial patch, do you agree?
Comment 21 Martin Renvoize 2022-02-21 07:54:40 UTC
I lifted the test plan from Nicks comment, but I forgot to set bug 29691 as a dependency.

However, I like the simplicity of your patch and would happily go with it in the interim and reverse the dependencies whilst we work out the fix for the current FQA bug above.
Comment 22 Jonathan Druart 2022-02-21 11:40:33 UTC
Created attachment 130949 [details] [review]
Bug 28955: Add OPAC_BRANCH_DEFAULT

This patch adds support for OPAC_BRANCH_DEFAULT as an environment option
that can be passed via apache with either SetEnv or as a header for
plack.  It allows setting a default branch for the anonymous opac
session such that you can display the right opac content blocks prior to
login if you have set up per branch URI's.

To test (on top of bug 29691)
1 - Add to apache conf (/etc/apache2/sites-available/kohadev.conf)
    SetEnv OPAC_BRANCH_DEFAULT "CPL"
    RequestHeader add X-Koha-SetEnv "OPAC_BRANCH_DEFAULT CPL"
2 - Restart all
3 - Confirm that news for all and CPL show on opac mainpage
4 - Sign in as a different library
5 - Confirm users library news shows

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 23 Jonathan Druart 2022-02-21 11:40:39 UTC
Created attachment 130950 [details] [review]
Bug 28955: (follow-up) Set to fall back

This patch updates the original fix to only set the template parameter
for opac sessions and updates all occurences in templats to check first
for logged_in_user.branchcode before falling back to default_branch

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 24 Jonathan Druart 2022-02-21 11:40:43 UTC
Created attachment 130951 [details] [review]
Bug 28955: Impact News as well

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Fridolin Somers 2022-02-25 00:36:26 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 26 Kyle M Hall 2022-02-25 14:48:50 UTC
Doesn't apply to 21.11.x. If this should be backported please rebase and ping me!
Comment 27 Martin Renvoize 2022-03-17 11:28:20 UTC
Created attachment 131833 [details] [review]
Bug 28955: Add OPAC_BRANCH_DEFAULT [21.11]

This patch adds support for OPAC_BRANCH_DEFAULT as an environment option
that can be passed via apache with either SetEnv or as a header for
plack.  It allows setting a default branch for the anonymous opac
session such that you can display the right opac content blocks prior to
login if you have set up per branch URI's.

To test (on top of bug 29691)
1 - Add to apache conf (/etc/apache2/sites-available/kohadev.conf)
    SetEnv OPAC_BRANCH_DEFAULT "CPL"
    RequestHeader add X-Koha-SetEnv "OPAC_BRANCH_DEFAULT CPL"
2 - Restart all
3 - Confirm that news for all and CPL show on opac mainpage
4 - Sign in as a different library
5 - Confirm users library news shows

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 28 Martin Renvoize 2022-03-17 11:28:24 UTC
Created attachment 131834 [details] [review]
Bug 28955: (follow-up) Set to fall back [21.11]

This patch updates the original fix to only set the template parameter
for opac sessions and updates all occurences in templats to check first
for logged_in_user.branchcode before falling back to default_branch

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 29 Martin Renvoize 2022-03-17 11:28:28 UTC
Created attachment 131835 [details] [review]
Bug 28955: Impact News as well [21.11]

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 30 Arthur Suzuki 2022-06-20 14:35:44 UTC
Pushed to 21.11 for 21.11.07, thx!
Comment 31 Victor Grousset/tuxayo 2022-06-25 23:52:16 UTC
Missing dependencies for 21.05.x, it shouldn't be affected, no backport.