[Koha-bugs] [Bug 5010] Fix OPACBaseURL to include protocol

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5010

--- Comment #107 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
QA Comment:
Great work, Mark. This took some time :)
Code looks good to me. No complaints from qa tools.
I understood that Martin tested the shibboleth case.
I did not test cas or plack. Altough I do not expect something there when
looking at the code, I leave it to RM to request additional tests. A very early
push of this patch in 3.22 could perhaps resolve that btw..

Some small remarks:
+        if ($oldprotocol ne 'https') {
+            $debug
+                and warn
+                  'Shibboleth requires OPACBaseURL to use the https
protocol!';
+        }
Perhaps we could argue here that the debug flag is not needed. Just warn.

C4/Context.pm:
+    # force explicit protocol on OPACBaseURL
+    if ($var eq 'opacbaseurl' && substr($value,0,4) !~ /http/) {
+        $value = 'http://' . $value;
+    }
This is bit more of a philosophical nature: Should we enforce it and make users
lazy? Or should we enforce it differently: Perhaps with a select combo with
protocol? Or in javascript? Note that if you now save the pref, you still have
a wrong value. Should we depend on modules to correct pref values? I would be
more in favor of correcting it earlier in the process. (No blocker)

What about staffClientBaseURL now?  :-)
svc/import_bib:    my $url = 'http://'.
C4::Context->preference('staffClientBaseURL')
.'/cgi-bin/koha/catalogue/detail.pl?biblionumber='. $biblionumber;
[This pref seems to be the easier case.]

Passed QA

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are watching all bug changes.