Looking at "etc/koha-httpd.conf", it looks like the erm and preservation rules have been put into the OPAC virtualhost by accident instead of the staff interface virtualhost. And it does look like the others are missing all together. This should've been caught during QA of those changes. Created attachment 188883 [details] [review] Proposed patch for Bug 41167 – Add missing rewrite rules in koha-httpd.conf Proposed patch for Bug 41167. This patch adds missing RewriteRules for ERM, Preservation and Acquisition modules in koha-httpd.conf to restore staff client functionality. Test plan: 1. Apply patch 2. Restart Apache 3. Access acquisitions and ERM pages 4. Confirm functionality restored Signed-off-by: Vivek Kumar <vivek.kumar@bestbookbuddies.com>" Thanks for the patch, Vivek, but it looks like there's quite a few mistakes. For instance, you're missing asterisks throughout the rules. You'll also need to remove the two erroneous RewriteRules from the OPAC virtualhost: RewriteRule ^/cgi-bin/koha/erm/.*$ /cgi-bin/koha/erm/erm.pl [PT] RewriteRule ^/cgi-bin/koha/preservation/.*$ /cgi-bin/koha/preservation/home.pl [PT] For reviewers, the following rules come from:
RewriteRule ^/cgi-bin/koha/erm/.*$ /cgi-bin/koha/erm/erm.pl [PT]
RewriteCond %{REQUEST_URI} !^/cgi-bin/koha/preservation/.*.pl$
RewriteRule ^/cgi-bin/koha/preservation/.*$ /cgi-bin/koha/preservation/home.pl [PT]
RewriteRule ^/cgi-bin/koha/admin/record_sources(.*)?$ /cgi-bin/koha/admin/record_sources.pl$1 [PT]
RewriteCond %{QUERY_STRING} booksellerid=(.*)
RewriteRule ^/cgi-bin/koha/acqui/supplier.pl$ /cgi-bin/koha/acquisition/vendors/%1? [R]
RewriteRule ^/cgi-bin/koha/acquisition/vendors(.*)?$ /cgi-bin/koha/acqui/vendors.pl$1 [PT]
For reviewers, the following rules come from debian/templates/apache-shared-intranet.conf:
RewriteRule ^/cgi-bin/koha/erm/.*$ /cgi-bin/koha/erm/erm.pl [PT]
RewriteCond %{REQUEST_URI} !^/cgi-bin/koha/preservation/.*.pl$
RewriteRule ^/cgi-bin/koha/preservation/.*$ /cgi-bin/koha/preservation/home.pl [PT]
RewriteRule ^/cgi-bin/koha/admin/record_sources(.*)?$ /cgi-bin/koha/admin/record_sources.pl$1 [PT]
RewriteCond %{QUERY_STRING} booksellerid=(.*)
RewriteRule ^/cgi-bin/koha/acqui/supplier.pl$ /cgi-bin/koha/acquisition/vendors/%1? [R]
RewriteRule ^/cgi-bin/koha/acquisition/vendors(.*)?$ /cgi-bin/koha/acqui/vendors.pl$1 [PT]
(In reply to vivek from comment #2) > Test plan: > 1. Apply patch > 2. Restart Apache > 3. Access acquisitions and ERM pages > 4. Confirm functionality restored This test plan won't work for anyone using koha-testing-docker, which means it'll be tricky to get someone to test. I'm still thinking about how I'd test this in koha-testing-docker. Because of the nature of the patch, we might be able to get away with just a visual review, but I'm not sure. When you provide an updated patch, I might tweak my KTD to expose a couple extra ports, and then do a "single" installation which will work with it. Created attachment 189141 [details] [review] Follow-up patch for Bug 41167 – Correct rewrite rules in koha-httpd.conf This is a follow-up patch for Bug 41167 – Add missing rewrite rules in koha-httpd.conf. Changes made in this follow-up: - Added missing asterisks (*) in rewrite rule patterns. - Removed two erroneous RewriteRules from the OPAC virtualhost. - Ensured all ERM, Preservation, and Acquisition rewrite rules are correctly placed under the intranet virtualhost. These corrections address the issues noted by David Cook in Comment 3 and Comment 5. Test plan: 1. Apply patch. 2. Restart Apache. 3. Access ERM, Preservation, and Acquisitions modules in the staff interface. 4. Confirm all pages load correctly without 404 errors or misrouted URLs. Created attachment 189248 [details] [review] Bug 41167: Fix missing rewrite rules in koha-httpd.conf This patch adds missing RewriteRules for ERM, Preservation and Acquisition modules in koha-httpd.conf to restore staff client functionality. Test plan: 1. Apply patch 2. Restart Apache 3. Access acquisitions and ERM pages 4. Confirm functionality restored Signed-off-by: Vivek Kumar <vivek.kumar@bestbookbuddies.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Created attachment 189249 [details] [review] Bug 41167: (follow-up) Correct rewrite rules in koha-httpd.conf This follow-up patch fixes issues from the initial patch for Bug 41167: - Adds missing asterisks (*) in rewrite rule patterns - Removes erroneous RewriteRules from the OPAC virtualhost - Ensures all ERM, Preservation, and Acquisition rewrite rules are correctly placed under the intranet virtualhost Test plan: 1. Apply patch 2. Restart Apache 3. Access ERM, Preservation, and Acquisitions modules 4. Confirm pages load correctly without 404 or routing errors Signed-off-by: Vivek Kumar <vivek.kumar@bestbookbuddies.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Fixed up the commit title on the second patch. These probably could've been squashed into two patches, but I left them as two separate ones anyway. Overall, it looks good. I'll include my test plan in a separate comment. Test plan:
0. sudo mkdir /opt/koha
1. sudo chown -R kohadev-koha:kohadev-koha /opt/koha
2a. perl Makefile.PL
2b. Installation mode (dev, single, standard) [standard] single
2c. Please specify the directory in which to install Koha [/var/lib/koha/kohadev/koha] /opt/koha
2d. Use defaults for the rest
3. make
4. make install
5. Update /opt/koha/etc/koha-httpd.conf to use the same ports as in
/etc/apache2/sites-enabled/kohadev.conf
6. sudo a2dissite kohadev.conf
7. sudo cp /opt/koha/etc/koha-httpd.conf /etc/apache2/sites-enabled/koha-httpd.conf
8. Copy any important details (like DB details) from
/etc/koha/sites/kohadev/koha-conf.xml to /opt/koha/etc/koha-conf.xml
9. Add following to VirtualHosts in /etc/apache2/sites-enabled/koha-httpd.conf
<Directory "/opt/koha">
Require all granted
</Directory>
10. sudo service apache2 restart
11a. Go to http://localhost:8081/cgi-bin/koha/acqui/acqui-home.pl
and click "Search" in the "Search vendors" area
11b. Note the UI eventually loads
12. Enable the syspref ERMModule
13. Go to http://localhost:8081/cgi-bin/koha/mainpage.pl
14. Click on "E-resource management"
15. Note that the ERM interface eventually loads
16. Enable the syspref PreservationModule
17. Go to http://localhost:8081/cgi-bin/koha/mainpage.pl
18. Click on "Preservation"
19. Note the UI eventually loads
20. Go to http://localhost:8081/cgi-bin/koha/admin/admin-home.pl
21. Click on "Record sources"
22. Note the UI eventually loads
Good catch, Vivek, and thanks for the patch. Created attachment 189252 [details] [review] Bug 41167: Fix missing rewrite rules in koha-httpd.conf This patch adds missing RewriteRules for ERM, Preservation and Acquisition modules in koha-httpd.conf to restore staff client functionality. Test plan: 1. Apply patch 2. Restart Apache 3. Access acquisitions and ERM pages 4. Confirm functionality restored Signed-off-by: Vivek Kumar <vivek.kumar@bestbookbuddies.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Bug 41167: (follow-up) Correct rewrite rules in koha-httpd.conf This follow-up patch fixes issues from the initial patch for Bug 41167: - Adds missing asterisks (*) in rewrite rule patterns - Removes erroneous RewriteRules from the OPAC virtualhost - Ensures all ERM, Preservation, and Acquisition rewrite rules are correctly placed under the intranet virtualhost Test plan: 1. Apply patch 2. Restart Apache 3. Access ERM, Preservation, and Acquisitions modules 4. Confirm pages load correctly without 404 or routing errors Signed-off-by: Vivek Kumar <vivek.kumar@bestbookbuddies.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Squashed them now. Created attachment 189595 [details] [review] Bug 41167: Add test to compare both apache conf Patch from commit 6df0f48 Marcel, David, I've written a quick test for this, but it fails: The following line is in the debian version, not in koha-httpd.conf, is that correct? # RewriteRule ^/cgi-bin/koha/(C4|debian|docs|etc|installer/data|install_misc|Koha|misc|selenium|t|test|tmp|xt)/|\.PL$ /notfound [PT] (In reply to Jonathan Druart from comment #16) > Marcel, David, I've written a quick test for this, but it fails: > > The following line is in the debian version, not in koha-httpd.conf, is that > correct? > > # RewriteRule > ^/cgi-bin/koha/(C4|debian|docs|etc|installer/ > data|install_misc|Koha|misc|selenium|t|test|tmp|xt)/|\.PL$ /notfound [PT] That is forgotten in httpd.conf. Doesnt hurt there too. xt/check_apache_conf.t .. Can't locate Data/Printer.pm in @INC (In reply to Marcel de Rooy from comment #18) > xt/check_apache_conf.t .. Can't locate Data/Printer.pm in @INC Will remove it. Created attachment 189626 [details] [review] Bug 41167: Fix missing rewrite rules in koha-httpd.conf This patch adds missing RewriteRules for ERM, Preservation and Acquisition modules in koha-httpd.conf to restore staff client functionality. Test plan: 1. Apply patch 2. Restart Apache 3. Access acquisitions and ERM pages 4. Confirm functionality restored Signed-off-by: Vivek Kumar <vivek.kumar@bestbookbuddies.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Bug 41167: (follow-up) Correct rewrite rules in koha-httpd.conf This follow-up patch fixes issues from the initial patch for Bug 41167: - Adds missing asterisks (*) in rewrite rule patterns - Removes erroneous RewriteRules from the OPAC virtualhost - Ensures all ERM, Preservation, and Acquisition rewrite rules are correctly placed under the intranet virtualhost Test plan: 1. Apply patch 2. Restart Apache 3. Access ERM, Preservation, and Acquisitions modules 4. Confirm pages load correctly without 404 or routing errors Signed-off-by: Vivek Kumar <vivek.kumar@bestbookbuddies.com> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 189627 [details] [review] Bug 41167: Add test to compare both apache conf Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] chmod 755, shebang, remove Data::Printer, enable NoWarnings one regex delimiter; replaced a grep by a map Created attachment 189628 [details] [review] Bug 41167: Add missing line to etx/koha-httpd.conf Test plan: Run xt/check_apache_conf.t again. Should pass now. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Resolved
[FAIL] xt/check_apache_conf.t
FAIL critic
Don't modify $_ in list functions at line 32, column 9. See page 114 of PBP. (Severity: 5)
Nice work everyone! Pushed to main for 25.11 Nice work everyone! Pushed to 25.05.x Pushed to 24.11.x for 24.11.11 With a patch to not add rewrites for vendor |
Dear Koha Team, In the Koha 25.05.x version, I noticed that the file koha-httpd.conf in the tarball (or Git-based) installation is missing several important rewrite rules under the intranet section. Because of this, when Koha is installed from source (tar or Git), many modules on the staff interface (such as Acquisitions) do not function correctly. However, when Koha is installed via APT packages, the correct koha-httpd.conf file is included, and everything works properly. The following rewrite rules are missing and should be added: RewriteRule ^/cgi-bin/koha/erm/.$ /cgi-bin/koha/erm/erm.pl [PT] RewriteCond %{REQUEST_URI} !^/cgi-bin/koha/preservation/..pl$ RewriteRule ^/cgi-bin/koha/preservation/.$ /cgi-bin/koha/preservation/home.pl [PT] RewriteRule ^/cgi-bin/koha/admin/record_sources(.)?$ /cgi-bin/koha/admin/record_sources.pl$1 [PT] RewriteCond %{QUERY_STRING} booksellerid=(.) RewriteRule ^/cgi-bin/koha/acqui/supplier.pl$ /cgi-bin/koha/acquisition/vendors/%1? [R] RewriteRule ^/cgi-bin/koha/acquisition/vendors(.)?$ /cgi-bin/koha/acqui/vendors.pl$1 [PT] Please include these rules in the official koha-httpd.conf source TAR package file for future releases to ensure full functionality when Koha is installed from the tarball or Git source. Thank you for your attention to this issue.