Items are no longer embedded in the MARCXML and because of this the MARC data
does not need to be saved once more after changing record items data.
The "defer_marc_save" is no longer needed since bulkmarcimport.pl was the only
place this option was utilized in order to resave MARC data after possibly changing
items data. There is also a bug bulkmarcimport.pl where the record data is re-saved
without stripping items if duplicate items are found and the dedup barcodes
option is enabled that is resolved by this change.
This change enables MARC Overlay rules to be enabled for bulkmarkcimport.pl as using
the defer_marc option would previously effectively bypass any defined rules.
To test
1) Apply patch 29440 (which this depends on), but no not yet apply this
patch
2) Remove comments around "bulkmarcimport: _("bulkmarcimport.pl"),"
in koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt
line 463
3) Enable MARC overlay rules and add a rule (with source => "*" or
'bulkmarcimport') to protect some field.
4) Import a biblio using the bulkmarcimport script. Overlay rules will
only be applied on updates so a match condition matching the
record in Koha when bulkmarcimport.pl is run the next time for the same
record needs to be supplied (for example -match "control-number,001")
5) Edit the imported biblio and change the value of the protected field
6) Run bulkmarcimport with the same parameters as in 4) and verify the
the field that should have been protected has been overwritten
7) Revert changes for line 463 in marc-overlay-rules.tt (or will result in
conflict applying the patch), apply the patch and repeat steps 4-5,
verify that the field now was protected from being overwritten.
Signed-off-by: Michał Kula <148193449+mkibp@users.noreply.github.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michał Kula <148193449+mkibp@users.noreply.github.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Michał Kula <148193449+mkibp@users.noreply.github.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
1) Import biblios using ./bulkmarcimport.pl -v -b -file biblios.mrc -insert -update -c=MARC21 -match "control-number,001" -n=1 -l "/tmp/import.log"
2) Verify in /tmp/import.log that one biblio has been inserted
3) Import again without the "-n" option, verify that one biblio has been
updated and the rest inserted
4) In the staff interface search the catalog for some string appearing in
the imported biblios to verify records have been indexed
5) View/edit some of the biblios the staff interface to very have been
properly imported
6) Import authorities using ./bulkmarcimport.pl -v -m=MARCXML -a -file authorities.xml -insert -update -c=MARC21 -keepids="024a" -match "heading-main,100a" -l "/tmp/import.log" -yaml="/tmp/ids.yml"
7) Go to the authorities page and find the imported authority by for example search for a
400 or 100 field value. Verify that the 001 field of the incoming record has been moved to 024a.
Also check that the value of 024a appear as a key in /tmp/ids.yml.
8) Edit the authorities.xml file and change the value of field 400a
9) Import authorities again using the same options
10) Verify that the 400a field of the authority in Koha has not been
updated (the import was skippped since revision did not change)
11) Edit authorities.xml, replace the value of field 005 in the file with field 005
of the authority in Koha incremented by one
12) Import again with the same options
13) Verify that the 400a field of the authority in Koha now have been
updated
Signed-off-by: Michał Kula <148193449+mkibp@users.noreply.github.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch tweaks three remaining cases, that are not covered by tests.
To test:
1. Apply this patch
2. Make use of those places
=> SUCCESS: No behavior change
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch makes what the title says.
To test:
1. Run:
$ ktd --shell
k$ prove t/db_dependent/Rec*
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch removes manual YAML handling for sysprefs in C4::Circulation.
It also makes C4::Context->yaml_preference not warn when undef is
retrieved from the sysprefs.
To test:
1. Run:
$ ktd --shell
k$ prove t/db_dependent/Circulation*
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
If autolinking is enabled, there are scenarios where the authorities merge process
updates the record, then sends it to ModBiblio which then AutoLinks the record, undoing
the changes in the record (as the 'from' authority is not removed until the process is complete)
This patch simply disables the autolinking process when merging authorities
To test:
Set system preferences
AutoCreateAuthorities = don't generate
RequireChoosingExistingAuthority = don't require
AutoLinkBiblios = Do
CatalogModuleRelink = Do
LinkerKeepStale = Don't
LinkerModule = first match
LinkerRelink = Do
1. Create an authority record
1.1. Go to Authorities
1.2. Click on "New authority" and choose an authority type (I chose Corporate name)*
1.3. Fill in the mandatory fields (000, 003, 005, 008, 040)
1.4. In tab 1, click on the label of the main heading (110 for Corporate name)
1.5. Fill in subfield
1.6. Copy content of subfield
1.7. Click on "Save"
1.8. Note the authority record number
2. Duplicate the authority record
2.1. Click on "Edit" and choose "Edit as new (duplicate)"
2.2. Click on "Save"
2.3. Click on "No: Save as new authority"
2.4. Note the authority record number (should be the number from step 1.8 +1)
3. Link one of the authority records to a bibliographic record
3.1 In another tab, do a catalog search
3.2. Click on "Edit record" under one of the search results
3.3. Go to the relevant tab (for corporate name, I used field 710 in tab 7)
3.3.a. If there are no empty fields for that tag, click the "Repeat this tag" button (two rectangles)
3.4. Paste the text previously copied in subfield
3.5. Click on "Link authorities automatically" at the top of the page
3.6. Note the authority record number in subfield
--> The linked authority record should be the first authority record (step 1.8)
3.7. Click on "Save"
4. Merge the two authority records, keep the one that is not linked to the bibliographic record
4.1. Go back to the tab with the authorities
4.1. Go to Authorities
4.2. Search for the aurhorities previous created
4.2. Click on "Actions" and choose "Merge" for both records
4.3. Select the record number that is NOT linked to the bibliographic record
4.4. Click on "Next"
4.5. Click on "Merge"
5. Check the bibliographic record
5.1. Go back to the tab with the bibliographic record and refresh the page (Ctrl+F5)
5.2. Hover the mouse cursor over the name or term, without clicking
5.3. Note the record number that appears at the bottom of the screen
--> The record number was not changed
6. Apply the patch
6.1. Run prove t/AuthoritiesMarc_MARC21.t
6.2. Repeat Previous steps 1, 2, 3, 4, 5
--> The record number is change
Signed-off-by: Barbara Petritsch <barbara.petritsch@wienmuseum.at>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch adds a replacement for 'booked' to the SIP renewal handling
so we can display 'Item is booked for another borrower'.
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch adds a bookings check to CanBookBeRenewed
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Whilst writing the test, I found a minor flaw in the logic and fixed
that in CanBookBeIssued at the same time.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
There should be no change in beahavior. Following the test plan from Bug 35840.
To test:
1. APPLY PATCH, restart_all
2. Turn on RecordLocalUseOnReturn
3. Create a Statistical patron.
4. Check an item out to a regular patron.
5. Check the item out to a Statistical patron.
6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return.
7. Try checking out an item to the Stats patron that is NOT checked out.
8. You should only see 1 entry, localuse, in the statistics table.
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
To test:
1. APPLY PATCH, restart_all
2. Turn on RecordLocalUseOnReturn
3. Create a Statistical patron.
4. Check an item out to a regular patron.
5. Check the item out to a Statistical patron.
6. This should trigger a return and you will see 2 entries in the statistics table, one for localuse and one for a return.
7. Try checking out an item to the Stats patron that is NOT checked out.
8. You should only see 1 entry, localuse, in the statistics table.
Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Test Plan:
1) Place a hold on an item
2) Build the holds queue
3) Check out the item to a different patron than the one
targeted in the holds queue
4) Verify the holds queue viewer still shows that item and patron
5) Apply this patch
6) Repeat stepts 1 through 3
7) Verify the holds queue viewer no longer shows that patron and item!
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Working on bug 31791, I found myself wondering if our current recursive
code in C4::Auth::haspermission() would allow checking AND on
subpermissions.
As it is not documented in the POD or tested, I decided to write some
unit tests for it.
It turned out it was well supported, so I decided to submit the tests,
and a small tweak in the POD to reflect that.
To test:
1. Apply this patch
2. Run:
$ ktd --shell
k$ prove t/db_dependent/Auth/haspermission.t
=> SUCCESS: Tests pass! The code supports AND on subpermissions!
3. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Test plan:
Add an item to your database that has no barcode.
Run t/db_dependent/Circulation.t
It will fail without this patch, pass with this patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This patch changes CanBookBeRenewed so that automatic renewal
errors pop up before other renewal errors. This means that a book
will be considered "auto_too_soon" before things like "too_many" or
"restricted". (Otherwise, you'll get an email saying you can't renew
a book the day after using your last auto renewal, even though the
earliest renewal isn't available until later.)
Test plan:
0. Apply patch
1. prove t/db_dependent/Circulation.t
2. prove t/db_dependent/Holds.t
3. prove t/db_dependent/Koha/Account/Line.t
4. prove t/db_dependent/Koha/Account.t
Additional tests:
5. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=RestrictionBlockRenewing
6. Change to "block"
7. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?tab=&op=search&searchfield=AutoRenewalNotices
8. Change to "according to patron messaging preferences"
9. Go to http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl
10. Set "Automatic renewal" to "Yes" and "No renewal before" to 4
11. Go to http://localhost:8081/cgi-bin/koha/circ/circulation.pl?borrowernumber=51
12. Checkout 39999000001310 with a due date 4 days in the future
13. Add a manual restriction
14. Run `perl ./misc/cronjobs/automatic_renewals.pl`
15. Note that it says something like the following:
Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (auto_too_soon)
Instead of something like the following:
Issue id: 1237 for borrower: 51 and item: 73 would not be renewed. (restriction)
Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
We deal with that in the middleware, we don't want 403.pl to early exit.
If we notice that we actually need it (for other scripts), we could
eventually add a new flag to get_template_and_user to skip the CSRF check,
or build the template without using get_template_and_user for errors/*
scripts.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The easiest here is to not empty 'op' but instead redirect to an error
page.
Minor changes: to keep the patch simple it removed the 'dev only' error and
display the error for non-dev installs. It should not be a problem
anyway and will prevent errors to be hidden in the log.
We could make KOHA_ERROR an arrayref, but later (we don't need it now
anyway).
Note that the OPAC still not benefit from a friendly specific error for
invalid token.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
GET svc/authentication will return a CSRF token in a response header
POST svc/authentication requires a CSRF token which can be sourced
from the response header of GET svc/authentication or some other
place like the meta element on a HTML page
Note: misc/migration_tools/koha-svc.pl is a simple script which
can be used to practically evaluate svc/authentication and svc/bib
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Suggestion to move the CSRF check to CGI->new so that we will check it
for every request, and it will cover svc scripts as well (they are not
using get_template_and_user).
The token will be retrieve from the param list *or the csrf_token
header* (do we want to name it x-koha-csrf-token instead?).
This will be done for *every* request that are not GET: CSRF token is now
required everywhere CGI is used (side-effects possible?).
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I *think* this change fixes a bug when starting the installer with an
expired session. I am no longer able to reproduce the problem however.
Just skip if it does not make sense.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
If the cookie contain an expired sessionID we need to create another
one to correctly generate the CSRF token.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Hum this didn't make sense. We are not checking credentials after
checkauth.
This patch is suggesting to rename "userid" and "password" parameters
from login forms to "login_userid" and "login_password" to not interfere
with other parameters with the same name.
This looks quite correct, however I am seeing
"The form submission failed (Wrong CSRF token)."
in the log after a successful login. Which feels wrong, what's
happening?
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We should no longer need to check CSRF token from pl files
TODO - there is a change for some files where we returned 403
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Not sure this is the right place in get_template_and_user
Will have to test login and 2FA
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a Koha::Session module that makes it easier
to work with Koha sessions without needing the full C4::Auth module.
Test plan:
0. Apply the patch
1. Run the following unit tests:
prove ./t/db_dependent/Auth.t
prove ./t/db_dependent/Auth_with_cas.t
prove ./t/db_dependent/Koha/Session.t
2. Observe that they all pass
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It seems safer to pass the logged in user and session info at the end of
the sub.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This code is a bit weird, its purpose it to auto select the library depending on the IP.
A problem appears if the same IP is used, then the user's choice will
might be overwritten randomly by another library.
To recreate the problem:
Turn on AutoLocation
Use koha/koha @CPL for test
And the following config:
*************************** 1. row ***************************
branchcode: CPL
branchname: Centerville
branchip: 172.18.0.1
*************************** 2. row ***************************
branchcode: FFL
branchname: Fairfield
branchip: 172.18.0.1
*************************** 3. row ***************************
branchcode: FPL
branchname: Fairview
branchip: 172.18.0.4
Connect and select CPL. Randomly FFL will be picked instead.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested this on top of 35890 and 35904 because git bz said they were required dependencies.
Figured out the IP Koha was seeing me as coming from in /var/log/koha/kohadev/plack.log.
Added that IP to the branchip for Centerville, Fairfield and Fairview. Set AutoLocation = Yes.
After this I could recreate the problem: If i left the "Library" field in the login screen
at "My Library" I got logged into a random library selected from the three i had set
branchip for. Applying the patches fixed this, as expected.
Tests pass, with AutoLocation off.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch suggests to add a new flag do_not_print to
C4::Auth::checkauth to not print the headers and allow to test this
subroutine more easily.
We do no longer need to mock safe_exit and redirect STDOUT to test its
return values.
There are still 3 left:
1.
733 # checkauth will redirect and safe_exit if not authenticated and not authorized
=> Better to keep this one, not trivial to replace
2.
806 # This will fail on permissions
This should be replaced but testing $template->{VARS}->{nopermission}
fails, I dont' think the comment is better.
3.
828 # Patron does not have the borrowers permission
Same as 2.
2. and 3. should be investigated a bit more.
This patch also move duplicated code to set patron's password to a
subroutine set_weak_password.
Test plan:
Read the code and confirm that everything makes sense.
QA: Do you have a better way for this? Yes it's dirty!
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
