Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This is the fourth and last patch set to remove C4::Branch.
The real purpose of this patch is to standardise and refactor some code
which is related to the libraries selection/display.
Its unconfessed purpose is to remove the C4::Branch package.
Before this patch set, only 6 subroutines still existed in the C4::Branch
package:
- GetBranchName
- GetBranchesLoop
- mybranch
- onlymine
- GetBranches
- GetBranch
GetBranchName basically returns the branchname for a given branchcode.
The branchname is only used for a display purpose and we don't need to
retrieve it in package or pl scripts (unless for a few exceptions).
We have a `Branches` template plugin with a `GetName` method which does
exactly this job.
To achieve this removal, we will use this template plugin and delete the
GetBranchName from pl and pm files.
The `Branches.all()` will now select the library of the logged in user
if no `selected` parameter has been passed.
This new behavior could cause regressions, for instance there are some
places where we do not want an option preselected (batch item
modification for instance), keep that in mind when testing.
GetBranchesLoop took 3 parameters: $branch and $onlymine.
The first one was used to set a "selected" flag, for a display purpose:
select an option in the libraries dropdown lists.
The second one was useless: If not passed or set to 0, the
`C4::Branch::onlymine` subroutine was called.
This onlymine flag was use to know if the logged in user was able to see
other libraries infos.
A patron can see the infos from other libraries if IndependentBranches
is not set OR if he has the superlibrarian permission.
Prior to this patch set, the "onlymine test" was done on different
places (neworderempty.pl, additem.pl, holidays.pl, etc.), including the
Branches TT plugin. In this patch set, this test is only done on one
place (C4::Context::only_my_library, code moved from
C4::Branch::onlymine).
To accomplish the same job as this subroutine, we just need to call the
`Branches.all()` method from the `Branches` TT plugin. It already
accepts a `selected` parameter to set a flag on the option to select.
To avoid the repetitive
[% IF selected %]<option selected="selected">[% ELSE %]<option>[% END %]
pattern, a new `html_helpers` TT include file has been created, it
defines an `options_for_libraries` block, which takes a `selected`
parameter. We could imagine to use this include file for other
selects.
The 'mybranch` and `onlymine` subroutines of the C4::Branch package have
been moved to C4::Context. onlymine has been renamed with
only_my_library. There are only 4 occurrences of it, against 11 before
this patch set.
There 2 subroutines are Context-centric and it makes sense to put them
in `C4::Context` (at least it's the least worst place!)
GetBranches is the tricky part of this patch set: It retrieves all the
libraries, independently of the value of IndependentBranches.
To keep the same way as the existing calls of `Branches.all()`, I have
added a `unfiltered` parameter. If set, the `Branches.all()` will call
a usual Koha::Libraries->search method, otherwise
Koha::Libraries->search_filtered will be called. This new method will
check if the logged in user is allowed to see other libraries or only
its library.
Note that this `GetBranches` subroutine also created a `category` key:
it allowed to get the list of groups (of libraries) where this library
existed. Thanks to a previous patch set (bug 15295), this value was
not used anymore (I may have missed something!).
Note that the only use of `GetBranch` was buggy (see bug 15746).
Test plan (for the whole patch set):
The best way to test this whole patch set is to test with 2 instances: 1
with the patch set applied, 1 using master, to be sure there is no
regression.
It would be good to test the same with `IndependentBranches` and the
without `IndependentBranches`.
No difference should be found.
The tester must focus on the library dropdowns on as many forms as
possible.
You will notice changes in the order of the options: the libraries will
now be ordered by branchname (instead of branchcode in some places).
A special attention will be given to the following page:
- acqui/neworderempty.pl
- catalogue/search.pl
- members/members-home.pl (header?)
- opac/opac-topissues.pl
- tools/holidays.pl
- admin/branch_transfer_limits.pl
- admin/item_circulation_alerts.pl
- rotating_collections/transferCollection.pl
- suggestion/suggestion.pl
- tools/export.pl
Notes for QA:
- There are 2 FIXMEs in the patch set, I have kept the existing behavior,
but I am not sure it's the good one. Feel free to open a bug report and
I will fill a patch if you think it's not correct. Otherwise, remove the
FIXME lines in a follow-up patch.
- The whole patch set is huge and makes a lot of changes.
But it finally will tremendously reduce the number of lines:
716 insertions for 1910 deletions
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch adds the frameworkcode option param, using each record's frameworkcode
as expected by the filter. Otherwise the ViewPolicy filter falls back to the
default framework.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Koha::RecordProcessor and the defined filters are supposed to bring us
joy and happiness. Let's keep the code compact, simple and clean.
This patch removes record cloning all over the place.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
There are still some leaks, but it is not as a result
of the filter, but rather a result of poorly written
template files.
Bug fixing template files is beyond the scope of this
set of patches.
TEST PLAN
---------
1) Backup your DB
2) run the following SQL on your DB.
> UPDATE marc_subfield_structure set hidden=-8;
-- this should set EVERYTHING to hidden across the board.
3) In staff client, set OPACXSLTDetailsDisplay to blank
4) In OPAC, view any detail.
-- Normal view may mostly leak values still.
-- MARC view may leak values.
-- ISBD view may leak values.
5) In staff client, set OPACXSLTDetailsDisplay to default
6) In OPAC, view any detail.
-- same issues as step 4
-- 'View Plain' may leak too.
7) 'Save record' -> 'Dublin Core'
8) Apply this patch
9) run koha qa test tools
-- should be fine
10) prove -v t/db_dependent/Filter_MARC_ViewPolicy.t
-- should pass
-- this proves Koha/Filter/MARC/ViewPolicy.pm tweaks too
11) In OPAC, view any detail.
-- Normal view:
-- Material type comes from the LEADER field.
-- Lists this is on will still display
-- 'Tags from this library' will still display
-- Item information in table will still display
(THIS IS BEYOND SCOPE)
-- MARC view:
-- Record number is leaked
(THIS IS BEYOND SCOPE)
-- 'View plain' leaks LEADER field.
-- ISBD view may leak field headings, but not values.
(THIS IS BEYOND SCOPE)
12) In staff client, set OPACXSLTDetailsDisplay to blank
13) In OPAC, view any detail.
-- same kind of output as step 10
14) 'Save record' -> BIBTEXT
-- Should be next to nothing leaked.
15) 'Save record' -> Dublin Core
-- Should be the same or less leaked between the two versions.
-- (XML FILTERING IS BEYOND SCOPE)
16) In the staff client, go view the same record.
-- it should be mostly hidden in ISBD View.
17) run the following SQL on your DB.
> UPDATE marc_subfield_structure set hidden=1;
-- this should set EVERYTHING to hidden in OPAC, but not
the STAFF across the board.
18) Refresh the staff ISBD page
-- values should reappear.
19) View the ISBD details in the OPAC
-- values should still be hidden.
20) Check out the OPAC Cart and List
-- while the intermediate pages may still leak
the download links should leak very minimally.
-- (CARTS AND LISTS ARE BEYOND SCOPE, THOUGH
THE INTRANET ISBD AND SOME CART/LIST STUFF
WERE FIXED BECAUSE OF THE GetISBDView REFACTOR)
Expectations:
Before Patch - all the OPAC Detail pages will display things
After Patch - all the OPAC Detail pages will display much less,
and hopefully nothing (though there are known limits).
the ISBD detail page in the Staff client will be
filtered as well based on STAFF settings.
The saving/exporting should generate nearly empty
files.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Applying the filtering and then...
Debugging opac/opac-detail.pl filtering
Debugging opac/opac-ISBDdetail.pl more
Debugging opac/opac-export.pl
Tweak opac/opac-export.pl fix variable declarations, conditional assignments
Debugging opac/opac-showmarc.pl
https://bugs.koha-community.org/show_bug.cgi?id=11592
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Why not clean up the License Agreement stuff while the files
are being changed? Used the current one found at:
http://wiki.koha-community.org/wiki/Coding_Guidelines#Licence
Changed the strict and warning lines into just a Modern::Perl.
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
s/Koha::Cache->get_instance/Koha::Caches->get_instance
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
https://bugs.koha-community.org/show_bug.cgi?id=11921
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jason M. Burds <JBurds@dubuque.lib.ia.us>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
This patch adds a configuration option which allows to define which
fields should be mandatory for a patron purchase suggestion form in OPAC.
Test plan:
1/ Apply patch.
2/ Play with the new OPACSuggestionMandatoryFields system preference
(select some fields as manadatory, select all, deselect all, try to
submit some suggestions with mandatory fields filled and/or not
filled etc.) to ensure that required fields (and only required fields)
are enforced in the browser to be filled.
3/ With all options deselected, 'Title' field should still be
mandatory (by default).
Signed-off-by: barbara johnson <barbara.johnson@bedfordtx.gov>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
opac-ics.pl was added to Koha back in 2007, but there has not been a link to it
from anywhere. This patch adds the link from the main view on
/cgi-bin/koha/opac-user.pl, and makes some changes to opac-ics.pl itself.
Changes to opac-ics.pl:
- Update the code to use date and time for when a loan is due
- Use dt_from_string to turn a MySQL time and date into a DateTime
- Add a timestamp and a UID (these were reported as necessary by a couple of
iCal validators)
- Remove DTEND from events, since we now have time as well as date for when a
loan is due
- For loans that are overdue, set the DTSTART to now(), to avoid creating
calendar events in the past
- Use a template to generate text strings, to make them translatable
To test:
- Apply the patch
- Make sure you have at least a couple of loans, including one overdue
- Go to /cgi-bin/koha/opac-user.pl and click on the link labeled "Download as
iCal/.ics file"
- Inspect the file in a text editor and/or view the result in a calendar
appplication. Make sure the data makes sense.
- Sign off and go thee merrily on thine way.
Update 2016-07-25
- Force date_due to UTC to avoid problematic timezone info like this:
DTSTART:TZID=Europe/Zurich:20160724T235900
- Remove quotes from summary and description
Followed test plan, koha.ics file successfully imported to Thunderbird calendar
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch adds a message to the patron home page in the OPAC to be
shown if the user is restricted because of a discharge.
To test, apply the patch and log into the OPAC as a patron who has been
discharged.
- You should see a message which says so, including a link to
the discharge notice.
- Log in to the OPAC as a patron with a manual restriction and confirm
that the correct notice is displayed.
Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
At the intranet, the pagination has been broken by bug 12478 (Elastic
Search). There was a confusion between the offset and the page number.
At the OPAC, it is broken since bug 2735 which has mimicked the intranet
script.
Test plan:
Search for a term which will return more than 1 page of results.
Click on the second page
=> Without this patch, the first result of the second page is the second
result of the first page
=> With this patch applied, the offset will be corrected
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If a patron edit his/her details a second time, we need to delete the
first ones to avoid a "duplicate entry for key PRIMARY" error.
Test plan:
Log in at the OPAC
Edit your details
Edit them again
=> Without this patch, Koha will crash
=> With the patch applied, everything should work as expected
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Followed steps in comment #14, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The changes from opac-memberentry do not reach the table, since the
Patron::Modifications object does not work well.
Test Plan:
1) Apply this patch
2) Create some patron modification requests
3) Ensure you can approve and deny modifications
4) Ensure patron self registration works
Signed-off-by: Bob Birchall <bob@calyx.net.au>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
At the OPAC, if a patron modify his/her information and at least 1
mandatory field is missing, Koha will crash with
Template process failed: undef error - Can't call method "description"
It is raised by Koha::Template::Plugin::Categories::GetName called with
an undefined categorycode.
The problem is that the values sent originaly are not sent back to the
template if the user missed something.
This patch makes that all info are resent to the template in order to
show the same form to the user.
Test plan:
0. Apply patch
1. Make sure that the categorycode is not hidden in the OPAC in:
PatronSelfRegistrationBorrowerUnwantedField or PatronSelfModificationBorrowerUnwantedField
2. Add a required field in PatronSelfRegistrationBorrowerMandatoryField, for example email.
3. Go to Your Personal Details in the OPAC (opac-memberentry.pl) and leave blank the required field of point 2
4. Press "Submit update request"
There should be no software error.
NOTE: The warns will be floody on the logs.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch adds logging for several holds actions. Specifically for:
- CREATE
- CANCEL
- DELETE
- RESUME
- SUSPEND
- MODIFY
To test:
- Enable the HoldsLog syspref
- Add a hold on a record/item
=> SUCCESS: The log view shows the CREATE action
- Click on the <Suspend> button
=> SUCCESS: The log view shows the SUSPEND action
- Click on the <Unsuspend> button
=> SUCCESS: The log view shows the RESUME action
- Click on the red cross, to delete the hold
=> SUCCESS: The log view shows the CANCEL action
Note: The DELETE action is logged when DelMember is called, with bug 16819 patches applied.
Sponsored-by: NEKLS
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
I also wonder about this going in defaulted on, but since the other logs are as well it seems ok to me.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To test
1/ Hit the page, notice the warning in the log
2/ Apply patch
3/ Hit page, notice no warning in the log
4/ Test functionality all still works
Works as expected. (Note: See Bug 16960 for updating patron details).
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If language is set by external link language switcher does not work
To reproduce:
- Have a multilingual koha installation, let's say en, de-CH, fr-FR and it-IT
- Create external links e.g. from your multilingual library website to each language variant of OPAC:
...cgi-bin/koha/opac-main.pl?language=en
...cgi-bin/koha/opac-main.pl?language=de-CH
...cgi-bin/koha/opac-main.pl?language=fr-FR
...cgi-bin/koha/opac-main.pl?language=it-IT
Each link opens fine displaying the language as appropriate.
Now try to switch language on the page you just opened via the external link.
It won't work, it will always stay on the language defined by the external link.
To test:
- Apply patch
- Repeat steps above
- Verify that the language switcher works
- Verify in browser address field that after switching language, the param 'language= 'has vanished from url string
- Try links with multiple params and with params containing 'language', e.g.
...cgi-bin/koha/opac-search.pl?language=fr-FR&q=language&mylanguage=test
- Verify that only the parameter 'language=' vanishes
Tested counter patch, works as well.
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Resolves the following comments:
I'd prefer to see a generate_csrf method than a CSRF flag.
It'd be better to use instead of require the 2 modules.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Move calls to WWW::CSRF to Koha::Token.
Send a safe random string to WWW::CSRF instead of letting CSRF make a
blocking call to Bytes::Random::Secure. If your server has not enough
entropy, opac-memberentry will hang waiting for more characters in
dev/random. Koha::Token uses Bytes::Random::Secure with the NonBlocking
flag.
Test plan:
[1] Do not yet apply this patch.
[2] If your server has not enough entropy, calling opac-memberentry may
take a while. But this not may be the case for you (no worries).
[3] Apply this patch.
[4] Verify that opac-memberentry still works as expected.
[5] Run t/Token.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Yes, my server had entropy trouble (reason for finding the problem).
This patch resolves the delay.
Tested all 3 patches together, works as expected.
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The vars are gotten from the url and sent to the template as it. They
must be escaped.
Test plan:
I have not managed to create the original issue, so there is no test
plan for the XSS fix, but you can confirm there is no regression.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
This subroutine just reads the content of a pref, split it, add an
empty string and returns.
It is not really useful and the code in the script (memberentry.pl) uses
the only occurrence of CGI::popup_menu
Let's remove it and build the dropdown list in the template.
Test plan:
1/ Empty BorrowersTitles, edit a patron and confirm that the "Salutation"
does not appear
2/ Fill BorrowersTitles with "Mr|Mrs|Miss|Ms", edit a patron and confirm
that the "Salutation" dropdown list is correctly filled.
The default option should be selected if you are editing a patron with a
title defined.
This should also be tested at the OPAC.
Followed test pan, works as expected in Staff and OPAC
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch erase all traces of C4::Csv since it's not used anymore.
All occurrences have been replaced by previous patches to use
Koha::CsvProfiles.
Note that GetMarcFieldsForCsv was not used prior this patch set.
Test plan:
git grep 'C4::Csv'
should not return any result.
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No more traces of the file.
This produces a koha-qa fail, due to the missing file.
No other errors
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This subroutine returned the csv profiles for a given type.
This could be done easily with the new Koha::CsvProfiles->search method.
Test plan:
To do at the OPAC and staff interface!
1/ Export a list using a CSV profile
2/ Export your CART using a CSV profile
Note that only MARC profiles should be available.
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested on staff/opac & cart/list
Small problem on filename extension fixed in followup.
No errors
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch moves the code from C4::Members::changepassword to
Koha::Patron->update_password
Test plan:
Change your password at the OPAC and the staff interface
This should work as before
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I rebased this on top of 16849 because they were conflicting.
Tests pass, code looks good (as usual) and I checked both OPAC
and staff password change work as expected.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In order to move IsMemberBlocked to Koha::Patron it makes sense to move
the code from Koha::Patron::Debarments::IsDebarred to
Koha::Patron->is_debarred.
Test plan:
1/ Add a restriction to a patron
2/ make sure he is not able to checkout items any more
3/ Make sure he cannot get a discharge
4/ Put a hold and make sure you get "Patron has restrictions"
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch fixes issues due to sentence splitting in
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
Note: This is a string patch. It does not add or change functionallity.
To test:
- Examine code and verify that the text changes remove sentence splitting
and that the changes make sense.
- Log in to OPAC and verify that messages look correct
- with blocked /debarred user (with and without comment and date)
- with user who has to much fines (needs syspref OpacRenewalAllowed
and syspref OPACFineNoRenewals)
- with user who has his card marked as lost
- with user with an account that is about to expire
- with user with expired account
- with user with uncertain address information (with and without
syspref OPACPatronDetails set to Allow)
Note: Amended to make patch apply 2016-07-10 mv
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The authority search pagination displays only 15 pages, even when there
are more.
To fix that, this patch mimic the authorities-home.pl intranet script to
manage the pagination.
Test plan:
Without this patch, do some authority search and click on different
pages.
Apply this patch, redo the same search and make sure the same records
are displayed (i.e. confirm that the pagination still works as before).
Signed-off-by: Nicole C Engard <nengard@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
TEST PLAN
---------
1) Apply Jonathan's test patch
2) prove t/db_dependent/Letters.t
-- dies before finishing tests
3) Apply second test patch
4) prove t/db_dependent/Letters.t
-- dies before finishing tests
-- 'addalert' is changed to 'getalert'
5) Log into OPAC with database admin user.
-- see error given in comment #0
6) Apply this patch
7) prove t/db_dependent/Letters.t
-- says 'getalert'
-- all tests pass.
8) Log into OPAC with database admin user.
-- logs in, but gives warning with a nice logout button.
9) run koha qa test tools.
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
No kaha qa errors
In debian display diferent error:
Can't use an undefined value as an ARRAY reference at /usr/lib/perl5/DBI.pm line 2054.
Works as advertised
NOTE: Revised test plan, as Jonathan added useful test case.
Works as I've tested above.
Hector tested older test plan which was steps
5,6,8 and 9.
Revised test plan again while tweaking to address comment #9.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If limit is X and patrons has X-1 pending suggestions, when a new one is
added, the "New suggestion" should not been added.
to avoid another call to SearchSuggestions, we assume than the
suggestion has been correctly added.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This does not work correctly and should be handle on another bug report.
The behavior existed prior to this patchset.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Only check limit for signed in borrower
Show all suggestions after a new one is added
Signed-off-by: Barbara Walters <bwalters@ncrl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Walters <bwalters@ncrl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Also fixes the issue that the add suggestion link would sometimes
show even if the patron could not make an additional suggestion.
Signed-off-by: Barbara Walters <bwalters@ncrl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To test:
1 - Define a number of MaxOpenSuggestions
2 - Add some suggestions for a patron
3 - Note they cannot add any more suggestions if limit is reached
4 - Delete a suggestion and note you can add another
5 - Add them to limit again
6 - Accept a suggestion in the staff client and note user can add more
7 - Repeat above with rejecting or deleting
8 - Sign off.
Sponsored by:
North Central Regional Library (http://ncrl.org/)
Signed-off-by: Barbara Walters <bwalters@ncrl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes the new sysprefs work as the previously defined. Instead of falling back
to what *ResultsDisplay is set, it now has its own 'default' (that still defaults to the
*Results*.xslt).
The default values are set to 'default' as the rest of XSLT-related sysprefs, and the upgrade
picks whatever is set in OPACXSLTResultsDisplay and XSLTResultsDisplay so current behaviour
is preserved.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
New XSLTs for Lists work as advertised
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan noticed the current behaviour is that the lists rendering
falls back to the XSLTResults, and if not defined (i.e. != 'default' and
!= some_path) it falls back to a legacy display (non-XSLT).
The patchset changed this behaviour because 'default' is not a valid value
for the lists. So it should fallback to the current behaivour (i.e. Results
XSLT configuration) if not defined. This patch fixes this by adding
|| C4::Context->preference('XSLTResultsDisplay')
(and the OPAC counterpart).
It also fixes minor glitches on the update message (oops) and bad default value
in sysprefs.sql for 'XSLTListsDisplay'.
Thanks Jonathan!
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes the shelves.pl (staff) and opac-shelves.pl scripts
use the new sysprefs for specifying custom XSLTs for lists display.
XSLT.pm is patched so it defaults to the corresponding *Results.xsl
files if none is specified.
To test:
- Create a list
- Open the list in the staff interface
- On a new tab, open the list in the OPAC.
- Apply this patches
=== default behaviour
- Open the list (both opac and staff) on new tabs
=> SUCCESS: They look exactly the same (hint: the syspref is set to ''
so it should fallback to using the one we were using.
=== using the new functionality
- Create custom XSLTs for lists, for example:
$ cd /home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/en/xslt
$ cp MARC21slim2OPACResults.xsl MARC21slim2OPACLists.xsl
- Edit your sysprefs, setting OPACXSLTListsDisplay to:
/home/vagrant/kohaclone/koha-tmpl/opac-tmpl/bootstrap/{langcode}/xslt/MARC21slim2OPACLists.xsl
- Reload the OPAC list view
=> SUCCESS: Looks exactly as before
- Make some minor tweak (for example in line 423 replace
<xsl:text> </xsl:text>
for
<xsl:text> BLAH </xsl:text>
- Reload the list
=> SUCCESS: BLAH shows in several places on the title.
- Repeat for the staff interface
- Sign off :-D
So we can now set custom XSLTs for lists.
Sponsored-by: Carnegie Stout Library
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Deb Stephenson <DStephen@dubuque.lib.ia.us>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If an attacker can get an authenticated Koha user to visit their page
with the code below, they can update the victim's details to arbitrary
values.
Test plan:
Trigger
/cgi-bin/koha/opac-memberentry.pl?action=update&borrower_B_city=HACKED&borrower_firstname=KOHA&borrower_surname=test
=> Without this patch, the update will be done (or modification
request)
=> With this patch applied you will get a crash "Wrong CSRF token" (no
need to stylish)
Do some regression tests with this patch applied (Update patron infos)
QA note: I am not sure it's useful to create a digest of the DB pass,
but just in case...
Reported by Alex Middleton at Dionach.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
