Can be moved to a separate bug report.
Sponsored-by: BULAC - http://www.bulac.fr/
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 11401 introduced code to support Norwegian national library card.
This code is too specific to be part of Koha as it, it should be a
plugin instead.
Moreover nobody uses it, but a modified version (see comment 3).
Test plan:
Add/edit/delete patron and make sure there are no regressions introduced
by these patches
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Improvements:
1) Mappings UI now has button that allows one to reset the mappings.
2) Mappings UI displays the items in alphabetical order.
3) Indexing script drops and recreates the index right away, which
helps prevent ES from autocreating a bad index if someone does something
while the first batch of records is being processed.
4) Indexing script has nicer output.
To test:
1) Change mappings.yaml file
2) Reset mappings in UI in mappings.pl
3) Verify the mappings have been changed in UI
4) The field order is alphabetical
5) Rebuild script has clean output
6) Run test t/db_dependent/Koha_Elasticsearch_Indexer.t
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Code and variables to deal with the update child feature are not
centralized but copied/pasted in several scripts. Which leads to issues
obsviously (bug 20805 for instance).
Moreover the strings used by the templates are also in several template
files (or .inc)
To deal with that this patch introduces the idea to create 1 .inc file
per .js file
Here we have members-menu.inc for members-menu.js
Test plan:
- Remove all your adult categories (categories.category_type='A')
- Create a patron with a child category
- Try to update to adult category
=> The entry does no longer appears! (This is a change in the behaviour)
- Create one adult category
- Update to adult category
=> There is a JS confirmation message, if you accept the patron will
be updated to the adult category
- Create (at least) another adult category
- Create another child
- Update to adult category
=> No more confirmation message but a popup to select the adult category
- Pick one
=> The patron has been updated to the adult category
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch addresses minor issues highlighted by QA.
- The POD is enhanced based on the proposal in bug 11983.
- The introduced offset type value is made consistent with the rest of
them.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch makes ->apply get passed an object set class instead of
single account lines.
The change is fully covered by tests.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Account/Lines.t
=> Test should pass
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch implements the ->apply() method for account lines.
It will only work on credit lines, and raise an exception otherwise.
It implements ->is_credit() as well.
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Account/Lines.t
=> SUCCESS: Tests pass!
- Check all use cases are covered by the tests
- Sign off :-D
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The error codes 1 to 7 are used in Uploader.pm or tools/upload.pl.
It would be nice to use alphanumeric codes instead.
No behavior change expected.
Test plan:
[1] Run t/db_dependent/Upload.t
[2] Verify that a regular upload with tools/upload.pl still works.
[3] Rename upload_path in your koha-conf.xml. Restart Plack, flush the cache
and try to upload to a category. Correct error message?
[4] Upload the same file twice to the same category.
Correct error message the second time?
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 19633: (QA follow-up) Really remove these ugly numbers
See BZ comment5. We now remove the numbers also from the constant names.
Test plan:
Read the changes.
Git grep "ERRCODE_"
Run t/db_dependent/Upload.t (Note: You may see one failure here; it is fixed
on bug 20727. So depends on who reaches master first.)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created the Koha::RDF module to mint URIs for RDF documents that
represent Koha bibliographic records.
_TEST PLAN_
To test the module, just run 'prove t/Koha/RDF.t'.
You'll get
some warnings about not being able to find koha-conf.xml, but
you can safely ignore them or you can point to any koha-conf.xml
file since it's not used by the test in actuality.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as expected. No warnings when run like:
sudo koha-shell -c "prove t/Koha/RDF.t" kohadev
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To test:
1 - Add the same guarantor to a number of patrons, ensuring random order
alphabetically
2 - View the guarantors record, note unordered list of guarantees
3 - Apply patch
4 - Guarantees are alphabetical
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
- Try a simple search (OPAC or staff) like "title:foo",
- click on a facet: badaboum!
- apply this patch,
- retry
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
It's a follow-up of
commit 9c6f634469
Bug 16912: Koha::Patrons - Move AddEnrolmentFeeIfNeeded to->add_enrolment_fee_if_needed
One of the known issue:
> update categories set enrolmentfee=2;
The use misc/devel/create_superlibrarian.pl to create a new
superlibrarian patron
Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
koha_1 | # Looks like you planned 20 tests but ran 9.
koha_1 |
koha_1 | # Failed test 'approve tests'
koha_1 | # at t/db_dependent/Koha/Patron/Modifications.t line
259.
koha_1 | [Something went wrong]# Looks like your test exited with
255 just after 5.
koha_1 | [21:13:47] t/db_dependent/Koha/Patron/Modifications.t
.....................
"Something went wrong" should never been thrown,
Koha::Patron::Modification->approve raises $@, but it's not defined, it
should raise $_ instead
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
When ->store is called on an existing patron we must not save the
password, but keep the one in DB instead.
There is a dedicated method (update_password) to call when the password
need to be updated
Signed-off-by: John Doe <you@example.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: John Doe <you@example.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Some libraries are paperless and require all payment receipts to be emailed. Koha should give libraries the option to send email receipts if a patron has an email address. If a notice for the type of "credit" exists ( payment or writeoff ), then an email receipt will be sent.
These notices only support Template Toolkit syntax.
Test Plan:
1) Apply this patch and dependencies
2) Add the two new notices, you can find them in installer/data/mysql/en/mandatory/sample_notices.sql
3) Note two new notices exist in the notices editor, ACCOUNT_PAYMENT and ACCOUNT_WRITEOFF
4) Find or create a patron with an email address that owes some amount of money
5) Make a payment for one or more fees
6) Note a new email is queued for the patron
7) Make a writeoff for one or more fees
8) Note a new new email is queued for the patron
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Move to aqinvoice_adjustment
Move to Koha::Acquisition::Invoice::Adjustments
Test if variable exists before count
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patchset adds the ability to add adjustments to an invoice, one can
provide a reason, an adjustment amount, select a budget, and choose
whether to encumber the funds before the invoice is closed or not
To test:
1 - Create a new invoice with or without a shipping cost
2 - Note there are no existing adjustments
3 - Add an adjustment
4 - Submit the form withno changes, nothing happens
5 - Update the adjustment you created, ensure changes are saved but no
extra adjustment created
6 - Add another invoice prodiving only reason or amount (you can have 0
value adjustments)
7 - Verify the adjustment total at bottom is correct
8 - Recieve some orders
9 - Verify totals are correct
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
- Context: Koha working with elasticsearch,
- Apply BZ 19582 (that make auth_finder.pl works again),
- apply this patch,
- go to cgi-bin/koha/authorities/auth_finder.pl,
- type at least 3 chararcters in a search field and
check that the autocomplete returns results
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To test:
1 - Do some authority searches in Zebra
2 - Switch to ES and repeat, results will vary and some may fail
3 - Apply patch and dependencies
4 - Reindex ES
5 - Repeat searches, they should suceed and results should be similar to
Zebra
6 - Slight differences are okay, but results should (mostly) meet
expectations
A few notes:
We add a 'normalizer' to ensure we get a single token from the heading
indexes, this makes 'starts with' work as expcted
We switch to 'AND' for fields searched from cataloging editor - this
matches Zebra results
We force the '__sort' fields for sorting - if sorting looks wrong try
reducing the heading field to a single subfield - this will need to be
addressed on a future bug (multiple subfields create an array, ES sorts
those randomly)
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Add/Update would update a field or create new if it existed, but didn't
allow for creating new if the field existed.
This patchset splits the options to 'Add & Update' so that 'Add' will always
add a field and 'Update' will operate as it always has
To test:
1 - Have a record with a known existing field (make a copy)
2 - Define a marc modification template that 'Add/update' on that field
3 - Define an 'Add/Update' on a field that doesn't exist
4 - Batch modify the copy of record using the above template
5 - Verify the existing field was updated
6 - Verify the non-existing field was updated
7 - Apply patch and update database
8 - Make another copy
9 - Modify the copy with the same template as above
10 - Should match initial modification
11 - Add a new rule to add a new field
12 - Modify using the updated template
13 - Ensure your new field is created
14 - Test various options in the modification tool
15 - prove t/db_dependent/MarcModificationTemplates.t
Signed-off-by: Victor Grousset <victor.grousset@biblibre.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
I do not think we should allowed branchode and Koha::Library objects, it
adds confusion in callers (we never know if we have a branchcode of a
library object).
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds a new method Koha::Biblio->can_be_transferred. The method checks
if at least one of the item of that biblio can be transferred to desired location.
This method will be useful for building a smarter pickup location list for holds,
because we will be able to hide those libraries to which none of the items of
this biblio can be transferred to due to branch transfer limits (see Bug 7614).
To test:
1. prove t/db_dependent/Koha/Biblios.t
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch changes Koha::Item->can_be_transferred to accept a HASHref as follows:
$item->can_be_transferred({ to => $library, from => $library2 })
To test:
1. prove t/db_dependent/Koha/Items.t
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds a new method Koha::Item->can_be_transferred.
Includes unit test.
To test:
1. prove t/db_dependent/Koha/Items.t
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch also fixes documentation in Koha/Item/Transfer.pm and
Koha/Item/Transfers.pm.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Some libraries would like to allow arbitrary lists to be used by all
librarians with the 'manage_patron_lists' permission.
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Create or find two patrons with the manage_patron_lists permission
4) Using the first patron, create two new lists, mark one of them as
shared
5) Log in as the second patron, browse to the patron lists page
6) Note the second patron can view, add and remove patrons from
the shared list owned by the first patron
Signed-off-by: George Williams <george@nekls.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch makes the controller code use $lines->total_outstanding
instead of expecting ->outstanding_debits and ->outstanding_credits
return the total.
Tests should pass as before. No behaviour change is expected.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> SUCCESS: Tests still pass, no behaviour change.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch splits the balance to match this object schema:
{
balance => #,
outstanding_credits => {
total => #,
lines => [ credit_line_1, ..., credit_line_n ]
},
outstanding_debits => {
total => #,
lines => [ debit_line_1, ..., debit_line_m ]
}
}
This change is made to ease usage from the UI. Also because the
outstanding credits need to be applied to outstanding debits in order to
the balance value to make sense. So we still need to have each total.
Tests are added for this change, and the schema files are adjusted as
well.
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> SUCCESS: Tests pass!
- Sign off :-D
staff_id is changed into user_id as voted on the dev meeting the RFC got
approved.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch implements the /patrons/{patron_id}/account endpoint. It can
be used to get the patron's balance information, including outstanding
lines.
To test:
- Apply this patchset
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> SUCCESS: tests pass!
- Make your favourite REST testing tool (RESTer on Firefox?) do:
GET /api/v1/patrons/{patron_id}/account
to a valid patron_id. Play with Koha's UI to add credits/payments and
notice the endpoint returns the right info following the voted RFC. [1]
- Sign off :-D
[1] https://wiki.koha-community.org/wiki/Patrons_account_lines_endpoint_RFC
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
But actually we could remove it if it does not make sense for other use.
Callers could deal with it since the password is not generated here
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
If there are no patrons in DB:
Use of uninitialized value in addition (+)
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
May be part of a separate bug report if needed
TODO - add POD + tests
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We shouldn't have it overloaded on the base class, as it could get huge
and difficult to find things, and read.
This patch moves things to Koha::Exceptions::Object. We should overload
the full_message method on each exceptions class, as needed.
To test:
- Run:
$ kshell
k$ prove t/Koha/Exceptions.t
=> SUCCESS: Tests still pass!
- Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch makes 'full_message' use the passed exception message instead
of trying to build it from the parameters. This is particularly useful
for some situations in which we don't have all the information but would
like to keep using the same exception.
To test:
- Apply this patchset
- Run:
$ kshell
k$ prove t/Koha/Exceptions.t
=> SUCCESS: Tests pass!
- Sign off
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan (shamelessly ripped from Bug20953):
1) Set system preference 'useDischarge' to 'Allow'
2) Choose a patron without checkouts nor fines nor restrictions
3) Log at OPAC and go to patron page /cgi-bin/koha/opac-user.pl
4) Click on 'ask for a discharge' tab
=> You see /cgi-bin/koha/opac-discharge.pl
with text 'What is a discharge? ...'
5) Click on 'Ask for a discharge' link
=> You see /cgi-bin/koha/opac-discharge.pl?op=request
with text 'Your discharge request has been sent ...'
6) In a new browser tab/page, go to intranet on /cgi-bin/koha/members/discharges.pl
=> You see one discharge requets for the patron
=> Click on 'allow' on patron discharge request
7) Come back to OPAC and refresh /cgi-bin/koha/opac-discharge.pl
=> You see link 'Get your discharge'
8) Click on the link to get your generated PDF
=> An error message should appear informing you to contact your website's admin
9) Apply this patch
10) Click again on the link to get your generated PDF
=> This time, a download prompt should appear
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch was discussed with Jonathan on a QA conversation. It is
better to keep this simpler and more reusable. And is the right approach
in this case.
This patch makes Koha::Account::outstanding_credits return the account
lines, code that used the $total value, will just use
$lines->total_outstanding;
Tests are adjusted accordingly.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Test pass.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds a method that retrieves (for a patron's account) the
outstanding credits (i.e. those that haven't been applied to any debit.
To test:
- Apply this patches
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This avoid hardcoding '10000' in two different places and allow users to
adjust this setting.
Also, this patch fixes a bug when the search return less than 10000
results
Test plan:
1. Do a search that returns 10000+ records.
2. Note the warning above the pagination buttons
3. Go to the last page, no error
4. Change the ES setting:
curl -XPUT http://elasticsearch/koha_master_biblios/_settings -d \
'{"index": {"max_result_window": 20000}}'
5. Do another search that returns more than 10000 but less than 20000
6. Note that the warning does not show up
7. Go to the last page, still no error
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
New MySQL column: aqorders.createdby
Creator's name is displayed on order's receive pages (acqui/orderreceive.pl
and acqui/parcel.pl)
On acqui/orderreceive.pl it replace the name of basket's creator
On acqui/parcel.pl, to avoid adding more data in the table of pending
orders, it is shown in a popup like MARC and Card views
Test plan:
1/ Run updatedatabase.pl
2/ Create a new order and go to the receipt page (acqui/parcel.pl)
3/ Click on "Order" link in column "More" (previously "View record")
4/ A javascript popup should appear with your name in it. Close the
popup.
5/ Click on "Receive" link
6/ Your name should appear in front of "Created by" label, to the right
of the page.
Patch updated with use of atomic update.
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch makes 'credit' account types generate 'Manual Credit' offset.
The original patch used 'Payment' but I think the code wants to
distinguish between payments and manually added credits (otherwise they
would've picked the same accounttype value, and they don't.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch makes Koha::Account::add_credit record statistics logs
(through C4::Stats::UpdateStats) only for the 'payment' and 'writeoff'
credit types.
Both credit types are whitelisted in UpdateStats, so we keep the current
behaviour. Another option would've been to add new account types to the
white list, but with the account offsets it seems that using the
statistics table for account changes won't be useful for too long, as
offsets contain the same (and more) information.
To test:
- Apply the patch
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds:
- Koha::Account::add_credit
- A single source of truth for accounttype and offset_type values
To test:
- Apply the first patch
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Account.t
=> FAIL: Tests fail!
- Apply this patch
- Run:
k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
At this time, only payments may be voided. There is no reason to have this limitation. There are situations where a librarian may need to void an accidental writeoff, or perhaps void an automatic credit that was created by Koha. For illustration, this is directly from a partner library:
"For example a lost book refund becomes a credit on account. Presently the credit may be applied to a fine for a different item charged to patron. We perform a write off to clear the remaining credit, then add the fine back to the account and give the patron a refund for the lost/refunded amount. Our accounting system ask that we keep the Lost funds/refunds separate from all fines."
Test Plan:
1) Create a fine and write it off
2) Note there is no 'void' button for the writeoff
3) Apply this patch
4) Note the buttons now show
5) Test each button on a writeoff
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch was discussed with Jonathan on a QA conversation. It is
better to keep this simpler and more reusable. And is the right approach
in this case.
This patch makes Koha::Account::outstanding_debits return the account
lines, and a method is added to Koha::Account::Lines so the outstanding
amount is calculated on the resultset. This is done the dame way it was
done before, and the tests got adjusted.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Added a few tests with credit lines and found we got warnings, adjusted
the routine to avoid this
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds a handy method that returns the total for outstanding
debits (i.e. those that haven't been canceled with credits), and the
corresponding Koha::Account::Line objects.
Unit tests are added.
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: tests pass!
- Sign off :-D
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We remove the error numbers 1 to 7 by readable codes.
And remove the errstr attribute (not used widely).
Make XSLT_Handler a little bit less noisy by defaulting print_warns to
false unless $ENV{DEBUG} is set. (See also bug 19018).
The unit has been changed accordingly.
(A few warnings are no longer tested.)
Test plan:
Run t/db_dependent/XSLT_Handler.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
If ISBN has 10 numbers only the first 9 numbers are being added to the Edifact-message (PIA+5+3540556753:IB')
This is caused by wrong positioning of capturing parentheses in a regular expression.
Sponsored-by: Gothenburg University Library
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We do no longer need "use Koha::UploadedFile" in a few places.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Trivial changes.
Adding C4::Context->temporary_directory to subsequent tmpdir call in Plugins.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
As explained in bug 20428 use tmpdir can cause issues and it just makes sense to standardize our temp directory in a universal way.
Test Plan:
1) Apply this patch
2) Verify you can still log in and use Koha
3) Verify the web installer still works
4) Verify EDI module can still download files via FTP
5) Verify fines.pl still runs with -o option
6) prove t/db_dependent/Plugins.t
7) prove t/db_dependent/Sitemapper.t
8) prove t/db_dependent/Templates.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
"return" statement with explicit "undef" at line 245, column 5. See
page 199 of PBP. (Severity: 5)
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.
To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records using the -d parameter with misc/search_tools/rebuild_elastic_search.pl to recreate the index
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 20073: Move Elasticsearch yaml files back to admin directory
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Koha::Patron::Discharge was missing use Koha::Patron::Debarments used in discharge() method.
Test plan :
1) Enable pref useDischarge
2) Go to OPAC user page
3) Go to 'ask for a discharge' tab
4) Click on 'Ask for a discharge'
5) Go to staff interface : /cgi-bin/koha/members/discharges.pl
6) Click on Allow
=> Without patch you get software error
=> With patch the discharge is validated
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
Run qa tools - it shouldn't complain about pod levels
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Again a regression caused by
commit fa54100dff
Bug 18403: Use patron-title.inc when hidepatronname is used [SPECIFIC for issuehistory]
GetBiblioIssues does a union all with issues and old_issues, so we
should old_issues as well.
To make the join on the items table we need to define the item and
patron methods. For consistency the relationships have been redefined
(item instead of itemnumber, borrower instead of borrowernumber) in the
DBIx::Class definition.
This is not perfect but I think the best way to provide an easy to
backport patch.
It highlights that we need improvements in this area.
Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
The patch works as expected.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We should not cast undefined values to 0.
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This was a bug I introduced. I think the fix is right, and if something
gets broken for this, it means it was making the wrong assumptions on
the data.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Instead of a list of Koha::Library::Group
Ideally it should be a Koh::Libraries iterator
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
When a method is not allowed yet (not covered by tests), developers get
an error the is not really useful: "The method XXX is not covered by
tests!"
It would be more useful to have a stack trace in order to know where
this error come from.
Test plan:
Modify some code to display this stack trace and confirm it makes sense
to have it, i.e. it will make debug easier
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This reverts commit f489d2034b.
This commit breaks the install process when using debian packages.
Reverting as we are very close to the 18.05.00 release
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Hmm. I would have liked the opac parameter even better ;)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is the alternative patch of Kyle's
"""
If a hold is 'waiting' for the patron to collect then the patron should
be prevented from cancelling the hold via their account in the opac.
If a patron tries to cancel the hold, Koha will give an 'are you sure'
alert and when you click Yes the page just refreshes and the hold
remains.
Staff can cancel the hold from the staff interface but they can then
action the waiting hold.
I think therefore that it is correct behaviour that a patron cannot
cancel a hold when it reaches waiting state via the opac but it would be
useful to either have a warning to prevent the cancellation or a useful
message when they attempt to do so.
The template was using a method that tells Koha if *staff* can cancel a
hold,
instead of patron.
Test Plan:
1) Set up a waiting hold
2) Try to cancel it from the opac
3) Note you cannot
4) Apply this patch
5) Reload the page
6) Note the cancel button has disappeared for found holds
"""
It sounds better to keep the ->is_cancelable method, for readability
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Upload creates its own upload folder in the temp folder, so there is no
need to add another level in temporary_directory.
Removing the creation of this folder in koha-create-dirs too.
Also removing the use Koha::UploadedFiles in about.pl. No longer needed.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes Koha::UploadedFile->temporary_directory try to use the
new configuration entry. It will fallback to File::Spec->tmpdir
otherwise.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This 'bug' existed long enough now to finally remove it ;)
We do so by deleting the invitation if the borrower already has a share
on this list. Actually not that hard.
We still need: a unit test and a db revision.
Test plan:
[1] Share a list. Let user B accept.
[2] Without this patch: Share again and let B accept again.
[3] Verify that you have two shares for this list in virtualshelfshares.
[4] With this patch: Share another list, let B accept.
[5] Share this other list again, let B accept again.
[6] Verify that virtualshelfshares does not contain double entries now.
(Note: This pertains to the second list only.)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
While we get packaging sorted, Net::OAuth2::AuthorizationServer is not a
hard dependency for Koha and the feature requiring it is disabled by
default.
This patch:
- Makes the dependency optional
- Makes the unit tests for the OAuth2 client credentials flow skip if
the dependency is not met.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the /token endpoint and the authenticate_api_request
method behave correctly in the event of missing deps for OAuth2.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/oauth.t
=> FAIL: The behaviour is not implemented
- Apply this patch
- Run:
k$ prove t/db_dependent/api/v1/oauth.t
=> SUCCESS: Tests pass!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the /api/v1/oauth/token enpoint respect the
RESTOAuth2ClientCredentials syspref. It will return 400 (with
'Unimplemented grant type' error message) on the event of the syspref
being disabled and the grant_type => 'client_credentials' value passed.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/oauth.t
=> FAIL: It fails because the off-switch is not implemented
- Apply this patch
- Run:
k$ prove t/db_dependent/api/v1/oauth.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
As Martin correctly highlighted, the method name is not correctly
spelled in POD. This patch fixes it.
To test:
- Look carefully
=> FAIL: Method name is _verify_client_cb but POD says _verify_client_db
- Apply this patch
- Look carefully
=> SUCCESS: Notice the POD is fixed!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch wires the OAuth related code so it leverages on the new
Koha::ApiKey(s) classes and tools introduced by bug 20568 instead of the
hardcoded entries in koha-conf.xml originally proposed by bug 20402.
To test revisit the test plan for bug 20402, and verify that it works.
But create API key pairs instead of writing them down in koha-conf.xml.
Also:
- Run:
$ prove t/db_dependent/api/v1/oauth.t
=> SUCCESS: Tests pass!
- Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch changes the table structure adding fields usually found on
this kind of api management pages.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This introduces the concept of API keys for use in the new REST API.
A key is a string of 32 alphanumerical characters (32 is purely
arbitrary, it can be changed easily).
A user can have multiple keys (unlimited at the moment)
Keys can be generated automatically, and then we have the possibility to
delete or revoke each one individually.
Test plan:
1/ Go to staff interface
2/ Go to a borrower page
3/ In toolbar, click on More -> Manage API keys
4/ Click on "Generate new key" multiple times, check that they are
correctly displayed under the button, and they are active by default
5/ Revoke some keys, check that they are not active anymore
6/ Delete some keys, check that they disappear from table
7/ Go to opac interface, log in
8/ In your user account pages, you now have a new tab to the left "your
API keys". Click on it.
9/ Repeat steps 4-6
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It implements only the "client credentials" flow with no scopes
support. API clients are tied to an existing patron and have the same
permissions as the patron they are tied to.
API Clients are defined in $KOHA_CONF.
Test plan:
0. Install Net::OAuth2::AuthorizationServer 0.16
1. In $KOHA_CONF, add an <api_client> element under <config>:
<api_client>
<client_id>$CLIENT_ID</client_id>
<client_secret>$CLIENT_SECRET</client_secret>
<patron_id>X</patron_id> <!-- X is an existing borrowernumber -->
</api_client>
2. Apply patch, run updatedatabase.pl and reload starman
3. Install Firefox extension RESTer [1]
4. In RESTer, go to "Authorization" tab and create a new OAuth2
configuration:
- OAuth flow: Client credentials
- Access Token Request Method: POST
- Access Token Request Endpoint: http://$KOHA_URL/api/v1/oauth/token
- Access Token Request Client Authentication: Credentials in request
body
- Client ID: $CLIENT_ID
- Client Secret: $CLIENT_SECRET
5. Click on the newly created configuration to generate a new token
(which will be valid only for an hour)
6. In RESTer, set HTTP method to GET and url to
http://$KOHA_URL/api/v1/patrons then click on SEND
If patron X has permission 'borrowers', it should return 200 OK
with the list of patrons
Otherwise it should return 403 with the list of required permissions
(Please test both cases)
7. Wait an hour (or run the following SQL query:
UPDATE oauth_access_tokens SET expires = 0) and repeat step 6.
You should have a 403 Forbidden status, and the token must have been
removed from the database.
8. Create a bunch of tokens using RESTer, make some of them expires
using the previous SQL query, and run the following command:
misc/cronjobs/cleanup_database.pl --oauth-tokens
Verify that expired tokens were removed, and that the others are
still there
9. prove t/db_dependent/api/v1/oauth.t
[1] https://addons.mozilla.org/en-US/firefox/addon/rester/
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
TEST PLAN
---------
apply unit test patch
prove t/db_dependent/OAI/Server.t
-- should fail
apply this patch
prove t/db_dependent/OAI/Server.t
-- should pass
run koha qa test tools
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patchs adds the ability to calculate the end of the suspension date
(debarment date) using the finesCalendar syspref.
Prior to this patch it was never calculating without taking into account
the calendar.
calculated without taking holidays into account.
This was a problem because the restriction could end in the middle of a
period the library is closed.
Test plan:
- Set finescalendar to 'not including days the library is closed'
- Set a circulation condition with no fine/maxfine, but fine days and
max fine days instead
- Check out an item with a due date in the past
- Check the item in and verify the restriction date
- Clean the restriction
- Add holidays to your calendar on the calculated restriction date
- Check the item out again with the same due date in the past
- Check in the item again
- Verify the calculated restriction end date has changed, it's set to the day
after the holiday.
Fines in days restriction calculation is correctly taking calendar
into account.
Sponsored-by: Goethe-Institut
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test Plan:
1) Apply this patch and its' dependencies
2) Create and pay some various fees and fines
3) View the payments for fees, and fees paid by payments by
using the new 'Details' button available on boraccount.pl
Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Running updatedatabase.pl after patch application makes dependencies
happy. Passes QA Tools and works as intended.
Rebased (2017-12-21): Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Bug 2696: (QA follow-up) - Remove use of GetMember
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch avoid the explosion in the air when creating a new patron
category and no active currency is defined.
Test plan:
Remove the currencies, create a new patron category
Without this patch applied you will get
Can't call method "p_sep_by_space" on an undefined value
With this patch the form will be displayed correctly and you will be
able to create the patron category
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
"return" statement with explicit "undef" at line 245, column 5. See
page 199 of PBP. (Severity: 5)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Improvements:
1) Index settings moved from code to etc/searchengine/elasticsearch/index_config.yaml. An alternative can be specified in koha-conf.xml.
2) Field settings moved from code to etc/searchengine/elasticsearch/field_config.yaml. An alternative can be specified in koha-conf.xml.
3) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch. An alternative can be specified in koha-conf.xml.
4) Default settings have been improved to remove punctuation from phrases used for sorting etc.
5) State variables are used for storing configuration to avoid parsing it multiple times.
6) A possibility to reset the fields too has been added to the reset operation of mappings administration.
7) mappings.yaml has been moved from admin/searchengine/elasticsearch to etc/searchengine/elasticsearch.
8) An stdno field type has been added for standard identifiers.
To test:
1) Run tests in t/Koha/SearchEngine/Elasticsearch.t
2) Clear tables search_fields and search_marc_map
3) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
4) Verify that admin/searchengine/elasticsearch/mappings.pl displays the mappings properly, including ISBN and other standard number fields.
5) Index some records using the -d parameter with misc/search_tools/rebuild_elastic_search.pl to recreate the index
6) Verify that you can find the records
7) Put <elasticsearch_index_mappings>non_existent</elasticsearch_index_mappings> to koha-conf.xml
8) Verify that admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1 fails because it can't find non_existent.
9) Copy etc/searchengine/elasticsearch/mappings.yaml to a new location and make elasticsearch_index_mappings setting in koha-conf.xml point to it.
10) Make a change in the new mappings.yaml.
11) Clear table search_fields (mappings reset doesn't do it yet, see bug 20248)
12) Go to admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
13) Verify that the changes you made are now visible in the mappings UI
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch removes 3 subroutines from C4::Letters:
- getalert
- addalert
- delalert
And add 3 methods to Koha::Subscription:
- subscribers
- add_subscriber
- remove_subscriber
It makes the code cleaner for future cleanup.
TODO - we should remove alert.alertid and alert.type, and rename
alert.externalid with alert.subscriptionid
That way alert will be renamed borrowers_subscriptions (or similar) and
will become a simple join table between borrowers and subscriptions.
We will need to deal with FK that could not be satisfied.
Let's do that after this patch is pushed.
Test plan:
Subscribe and unsubscribe to email notifications sent when a new issues
is available.
Make sure everything works as before and you receive the emails.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
- "your routing lists" tab is now highlighted when active
- get_routinglists was renamed to get_routing_lists
- Koha::Patron->get_routing_lists returns the ->search result
directly
- Koha::Subscription::RoutingList->subscription uses DBIC
relationship
- Undo changes to C4/Auth.pm
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds the base for the new feature:
Show a list of the serial titles a patron is on routing
lists for in the OPAC.
Test plan applies to the complete patch set:
To test:
- Apply all patches
- Make sure RoutingSerials is not activated
- Check patron account in OPAC - no tab should appear
- Activate RoutingSerials
- Create subscriptions and different routing lists, test with:
- Patron with no routing list entries = no tab
- Patron with one or more routing list entries = tab appears
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 20400: Rewrite using Koha::Objects
Adds
- Koha::Subscription::Routinglist
- Koha::Subscription::Routinglists
Adds 2 methods
- Koha::Patron::get_routinglists
- Koha::Routinglist::subscription
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 20400: Add unit tests
prove t/db_dependent/Koha/Subscription/Routinglists.t
prove t/db_dependent/Koha/Patrons.t
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 20400: Display new tab in OPAC only for patrons with routing lists
The visibility of the routing list tab in the OPAC depends
on the system preference RoutingSerials and the existence
of routing list entries for the patron.
Some libraries only offer routing lists to certain user groups and
would not want it generally visible. As there are currently no
actions you can perform from the list, this appears to be a
reasonable behaviour.
See test plan in first patch.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 20400: (follow-up) Use Asset TT plugin on opac-routing-lists.tt
Patch applies and functions as described.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 20400: (QA follow-up) Redirect to 404 if routing is disabled
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch moves the Koha::Patrons->merge method into
Koha::Patron->merge_with in the line of the discussed implementation for
bug 15336. I agree with that implementation so I provide this follow-up.
Tests are adjusted, the controller script is adapted too. The behaviour
remains.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It would be great if there were a merge patrons feature. If you
accidentally end up with one patron with two cards it would be nice to
merge their records together so that you don't lose their history or
holds or anything.
This patch adds a basic patron merge feature. It attempts to relink all
patron related tables from the patron(s) to be merged. It does not
attempt to relink librarian account related tables at this time. This
feature does not attempt to automatically resolve issues such as
duplicate holds. Such a feature could build upon this one though.
Test Plan:
1) Apply this patch
2) Find two or more patrons
3) Perform a patron search that will bring them up on the same page of
results, or add them all to a list of patrons.
4) Use the 'merge' button to begin the merging process
5) Choose a patron to keep
6) Verify the deleted patrons data ( checkouts, holds, etc )
are now linked to the kept patron
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - prove t/db_dependent/Koha/Z3950Servers.t
2 - Load cataloging and authority home pages
3 - Verify you can add form z395 on each page
4 - Delete all z3950 servers
5 - Viery option to add from Z3950 is removed on both pages
Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes Koha::Exceptions::Object include
Koha::Exceptions::Exception so it is not required to include it
excplicitly for using this exceptions.
To test:
- Run:
$ kshell
$k prove t/Koha/Exceptions.t
=> FAIL: Tests fail because Koha::Exceptions::Exception is not
(implicitly or explicitly included)
- Apply this patch
- Run
k$ prove t/Koha/Exceptions.t
=> SUCCESS: Tests pass!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I was working on bug 20580 and noticed that the exceptions are not
stringified and so not really useful when displayed in string context.
This patch is for discussion and not ready for inclusion
Still need to fix:
Koha/REST/V1/Patrons.pm (The error is "Given xxx does not exist")
Koha/Object.pm: in ->store, Koha::Exceptions::Object::FKConstraint is
raised but we cannot set 'value'
Another note: In Koha::Exceptions::Exception I do not have access to
$self->line_number and $self->filename
See also
http://search.cpan.org/~drolsky/Exception-Class-1.44/lib/Exception/Class/Base.pm#OVERRIDING_THE_as_string_METHOD
Test plan:
Apply patch from bug 20580 and execute create_superlibrarian.pl with an
invalid categorycode
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We've had the ability to 'reverse' a payment for a long time, but it
would be much better if we had a true void payment function that
replaces the paid amounts into the fee so that it appears as if the
payment was never made.
Test Plan:
1) Apply this patch and dependent patches
2) Run updatedatabase.pl
3) Create some fines
4) Pay those fines
5) Use the new 'void' button to void the payments
6) Note the fines were restored to their pre-payment amounts
Signed-off-by: Koha-us conference <koha-us@koha-us.net>
Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Some libraries would like to be able to specify more specific payment types such as cash, check, credit card, or even canned food ( for food drives ). This feature will allow a library to specify payment types as authorised values of the type PAYMENT_TYPE.
Test Plan:
1) Apply patches
2) Update database
3) Add authorised value PAYMENT_TYPE with values 'Cash', 'Check', 'Credit card'
4) Go to the fines section of a patron who has several fines
5) Tab 'Fines'
6) Click on 'Pay' for an individual fine
7) Verify that in the form a 'Pay an individual fine' dropdown appears with payment types
8) Select a payment type, confirm payment
9) Verify that payment type appears in "Description of charges" in tab 'Account'
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Bug 18786: Remove use of Koha::Patron::Images
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Bug 18786: Remove duplicate block with bad IF
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
prove t/db_dependent/Koha/Subscription.t
must return green
NOTE: My preference is for $subscription to be object,
and perhaps something like $built_subscription to
be from the builder. Because, $object is a poor
variable name.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Moving the status to the invalid 'processing' might well have unwanted
side-effects. (The status column will be set to empty string and we have
a problem if it is not processed.)
This patch allows pass-through of DBIX's make_column_dirty in
Koha::Object (simple tests included) and uses it to force an update.
If the update does not return true, it still exits.
Test plan:
[1] Read the changes.
[2] Run t/db_dependent/Koha/Object.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Having to write [% KOHA_VERSION %] for each url is bad because:
- It's easily forgettable when adding new <script> or <link>
- It prevents grep'ing for the full filename
- It violates the DRY principle
- If at some point we want to change the "force js and css reload"
mechanism, it will be tedious
This patch:
- adds a Template::Toolkit plugin that generates <script> and
<link> tags for JS and CSS files, and inserts automatically the Koha
version in the filename
- use the new plugin to remove all occurences of [% KOHA_VERSION %]
- remove the code that was adding KOHA_VERSION as a template variable
Test plan:
1. Apply patch
2. Go to several different pages in Koha (opac and intranet) while
checking your browser's dev tools (there should be no 404 for JS and
CSS files, and the Koha version should appear in filenames) and the
server logs (there should be no "File not found")
3. `git grep KOHA_VERSION` should return nothing
4. prove t/db_dependent/Koha/Template/Plugin/Asset.t
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
As requested by RM, this patch replaces using the global $cached_indicators
by saving state temporarily during the (limited) lifetime of the object.
Essentially this affects two places in code:
[1] blinddetail-biblio-search.pl (loading auth record in editor)
[2] AuthoritiesMarc::merge (merging authority into biblios)
Concurrent runs of [1] and/or [2] together with a simultaneous pref change
just in between could cause slight (hypothetical) side-effects.
The current approach of keeping state in the object makes that a series of
controlled_indicators calls during an immediate merge of one specific
authority is not affected by a simultaneous pref change. So the same rules
are applied to the set of attached biblio record for that authority.
Note also that the cron job ignores a simultaneous pref change, since
it reads from the unchanged L1 cache (yes, also hypothetical).
Test plan:
[1] Run t/Koha/Authority/ControlledIndicators.t
[2] Run t/db_dependent/Authority/Merge.t
[3] Run t/db_dependent/Koha/Authorities.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In sub merge the Koha::Authority object was already created, so it was kind
of silly to recreate it each time with new.
In Koha::Authority, the ControlledIndicators object can be saved in the
object too for reuse during its life time. Similarly, we prevent repeated
lookups for the reporting tag (via authority type).
Test plan:
[1] Run t/db_dependent/Authority/Merge.t
[2] Run t/db_dependent/Koha/Authorities.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Limited benchmarking shows me a reduction of 11% on Merge.t and a
reduction of 4% on Authorities.t when comparing with and without this
last patch. The larger impact on Merge.t could be expected.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch does:
[1] Adds Koha::Authority->controlled_indicators (with a test).
[2] Adds a call to controlled_indicators in AuthoritiesMarc::merge.
Unit test Merge.t is extended too.
[3] Simplifies the code in authorities/blinddetail-biblio-search.pl by
calling controlled_indicators.
Test plan:
[1] Run t/db_dependent/Koha/Authorities.t
[2] Run t/db_dependent/Authority/Merge.t
[3] Steps 3 to 7 for MARC21:
Create a PERSO_NAME authority with 008/11=r and ind1=3
[4] Edit a biblio and add a 600 linked to the new authority.
[5] Verify that the biblio has ind1==3 and ind2==7 and $2==aat.
(If $2 is not visible, check the metadata in biblio_metadata.)
[6] Edit the PERSO_NAME authority and change 008/11 to '|' (bar).
[7] Verify that merge updated your biblio record: $ind2==4 and $2 gone.
[8] UNIMARC: Follow the pattern from steps 3 to 7.
Create authority, link it in a biblio, check indicators (they should
be copied both). Edit authority, change indicators and verify the
merge results in the biblio record.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested PERSO_NAME and UNIF_TITLE.
For UNIF_TITLE the second authority indicator is copied to ind1 or ind2,
depending on the biblio tag involved.
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Apart from adding tests for this sub, we make the following change
too and support it with a test.
The fallback to $code in the construction
$thes_mapping{ $code } // $code // '4'
is removed by this patch. When $code is not in $thes_mapping, we cannot
assume that it is a valid code for a biblio indicator. In this case it is
more safe to mark it as 4 (Source not specified).
Test plan:
Run t/Koha/Authority/ControlledIndicators.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Original patch from Janusz Kaczmarek on November 24, 2017.
Amended by Marcel de Rooy on February 6, 2018.
Code moved from AuthoritiesMarc.pm to ControlledIndicators.pm.
Special attention has been paid to the proper application of 008/11
while controlling 6XX in MARC 21, specially if 008/11 =~ /[rsz]/
(and if it is 'z' and 040 $f is defined).
Test plan:
See next patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The construction $thes_mapping{ $code } // $code // '4' will still get
some attention on a follow-up.
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This module reads its settings from pref AuthorityControlledIndicators.
The get method allows you to retrieve indicator values for specific
biblio fields.
Test plan:
Run t/Koha/Authority/ControlledIndicators.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: Added a test to show that "ind1:" returns empty string.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Changes in MarcTagStructure.pm
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Found this regression when working on other patches:
If you edit a patron and blank the userid field, it will be regenerated
with an incremented value (firstname.surname will be firstname.surname1)
This is because we use a non-existing patron and ->in_storage in
has_valid_userid is always false.
The trick here is to backup the value, generate the userid, then reset
userid to the previous value.
As the POD says, it will be fix later, when AddMember and ModMember will
be replaced with Koha::Patron->store
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
C4::Members did not "use warnings;"
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We previously prove that the method and the subroutine were equivalent,
we know update the controller calls.
Test plan:
- Add and update a patron with different variations of userid
(automatically generated or not)
- Import patrons with and without userid, as well as with existing
userid
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Reuse how C4::Members::Check_Userid works and adapt it to write
Koha::Patron->check_userid
Adapt the tests to use this new method.
The tests still pass, we can adapt the different callers
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It should also be noted that
/etc/apache2/sites-enabled/{configuration file}
should also contain:
RequestHeader add X-Koha-SetEnv "TZ {timezone}"
Where {timezone} is the desired instance timezone.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Most of Koha depends on the local timezone of the server, except for Koha::Database which support an ENV override with the key TZ.
We should take this a step further. We should not only accept the TZ environment variable for all of Koha, we should really be able to set the timezone in the koha conf file as well so we don't have to pass that environment variable to things like cronjobs and one-off scripts.
Test Plan:
1) Apply this patch
2) Set a timzone in your koha_conf file, that is *not* your local time zone
Available timzones are listed here: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
3) Restart apache/plack
4) Perform some actions, check the timestamps in the database and in the
html output, note they are for the set timezone and not the local
timezone.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Stolen from bug 20244
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This option in combination with append seems to add null entries in the indexes
for fields that don't exist. These fields then affect sorting. Removing it solves
the issue
To test:
1 - Have an ES setup
2 - Catalog and index some records
3 - View a record in ES:
curl 'localhost:9200/koha_kohadev_biblios/data/4?pretty=yes'
4 - Note there are 'null' entries in the indexes, save this result for
comparison
5 - Apply patch
6 - Reindex, amking sure to delete existing indexes
perl misc/search_tools/rebuild_elastic_search.pl -d
7 - View the record again
8 - Note the null entries are removed
9 - Note indices are otherwise intact
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Replaced with distinct.
Fix for:
'koha_kohadev.me.shelfname' isn't in GROUP BY
t/db_dependent/Virtualshelves.t
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
A number of Koha plugins have been written that enhance Koha's public catalog. These plugins often make due to adding css and javascript to the various opac system preferences. It would be nice if the plugin system had hooks so plugin developers could add code the the head block and the area where we include javascript in the opac template.
Test Plan:
1) Apply this patch
2) Download and install the Kitchen Sink plugin ( v2.1.12 or later )
https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases/download/v2.1.12/koha-plugin-kitchen-sink-v2.1.12.kpz
3) Install the plugin
4) Restart all the things if you can ( restart_all if you are using kohadevbox )
This will ensure the plugin takes effect right away, it should be
necessary but it won't hurt anything!
5) Load the opac, notice you get an alert message and the background
for your opac is now orange ( assuming you've not customized the
opac in any way )
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Caused by
commit 04aea91de0
Bug 15685: (QA follow-up) Address QA issues
->find is called on Koha::Object instead of the set class (Koha::Objects)
and raises the following error:
Can't use string ("Koha::Acquisition::Basket") as a HASH ref while
"strict refs" in use at /usr/share/koha/lib/Koha/Object.pm line 275.
This patch also makes sure $basketno refers to an existing basket in DB
I cannot provide a test plan, I have no idea how this code is used.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jenkins fails (run 287) on a test in t/db_dependent/Letters/TemplateToolkit.t:
With the historical syntax:
# Your request for an article from tQYRS (c3Av58O0P5xkkIGu) has been canceled for the following reason:
With the TT syntax:
# Your request for an article from tQYRS_ (c3Av58O0P5xkkIGu) has been canceled for the following reason:
The last character of the biblio's title has been removed because it's a punctuation character.
It comes from: C4::Letters::_parseletter
893 $val =~ s/\p{P}$// if $val && $table=~/biblio/;
The same replacement is done for patron's attributes too.
Test plan:
- Confirm that the new tests pass. That should be enough to confirm this change make sense.
Test plan (manual):
- Create a biblio with a title ending with a punctuation (like "with_punctuation_"), or any other fields of biblio/biblioitem
- Generate a notice which will display this field (CHECKIN for instance)
Use the historical syntax and the TT syntax, both should display the title without the punctuation character at the end
CHECKIN historical:
The following items have been checked in:
----
<<biblio.title>>
----
CHECKIN TT syntax:
The following items have been checked in:
----
[% biblio.title %]
----
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces two methods to be used by plugin authors:
->output
->output_html
They are basically wrappers for the helper methods from C4::Output
(output_html_with_http_headers and output_with_http_headers).
Plugin authors can use them, or keep the current flexibility of handling
the headers themselves in their code.
The KitchenSink plugin should be updated to highlight this.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/Plugins.t
=> FAIL: The methods are not implemented
- Apply this patch
- Run:
k$ prove t/db_dependent/Plugins.t
=> SUCCESS: Tests pass, and they are meaningful
- Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We should actually use Koha::Patron->checkouts here to avoid the
prefetch.
Test plan:
A patron with checkouts cannot get a discharge
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To move this subroutine out of the C4 namespace we face the same
problematic as bug 17553 (with GetOverduesForPatron).
We need to provide an equivalent method and so return all the related
value for a given checkout.
We can acchieve the easily using Koha::Object->unblessed_all_relateds,
but we need to keep in mind that it is a temporary move.
Indeed we will want to use our API to only access/retrive values we really need.
The whole trick could be removed when the current syntax for notices
will be deprecated and removed.
Note: this method returns the same number of elements than ->checkouts
They indeed returns the same things, but it sounds better to me to have a
different method to highlight (from the callers) where does it come
from (C4::Members::GetPendingIssues).
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In order to refactor our legacy code step-by-step, we will need to
provide code with the exact same behaviors and then improve it.
The same idea appears for the TT syntax for notices: we will want to
deprecate the existing syntax in order to support only one syntax.
Currently we fetch all the values from the related tables, without
knowing which are actually used.
I am suggestion to introduce a Koha::Object->unblessed_all_relateds
method which will return a hash containing all the fields from the
related tables (with the problems we know: collision in column names).
It is the existing behavior of GetOverduesForPatron and GetPendingIssues
for instance, they are used to send notices and so we have to provide
all the data needed.
See dependent bugs to understand the context and test this patch.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
prove -v t/db_dependent/Koha/Reports.t
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The first patch makes Hold.t fail on:
not ok 8 - Hold is suspended with a date, truncation takes place automatically
Failed test 'Hold is suspended with a date, truncation takes place automatically'
at Hold.t line 94.
got: '2018-03-22'
expected: '2018-03-22T00:00:00'
We could remove the midnight time there, but we also could replace ymd by
datetime. The cpan doc tells us:
Same as $dt->ymd('-') . 'T' . $dt->hms(':')
Test plan:
With this patch, run Hold.t again. It should pass now.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When a hold is suspended, 'suspend_until' is represented as a DateTime object, complete with locale. All told, this is about 800 lines of text.
Test Plan:
1) Enable the HoldsLog syspref
2) Add a hold on a record/item
3) Suspend the hold with a date to resume
4) Note the massive amount of date in the suspend_until field
5) Apply this patch
6) Suspend another hold with a date to resume
7) Note the log has an acutal date in the suspend_until field
Check the logs using module 'Holds' and Action 'Suspend'
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
TO test:
0 - Have a koha with ES enabled and some auth records
1 - backup your db
2 - sudo koha-mysql <<instance>>
3 - UPDATE auth_types SET authtypecode="PERSO_NAMB" WHERE
authtypecode="PERSO_NAME" (or adjust as necessary)
4 - Search for some authorities where a PERSO_NAME record will be
returned
5 - KA_BOOM!
6 - Apply patch
7 - Results are returned, they do not display a code, viewing record you
won't see code
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces two functions to the patrons endpoint:
- _to_api
- _to_model
This are in charge of field mappings in order to comply with the
guidelines.
Koha::REST::V1:Auth is adjusted to handle 'patron_id' as well. 'borrowernumber'
handling is kept until the existing endpoints get updated.
To test:
- Apply the patches
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
prove t/db_dependent/api/v1/patrons.t
--> without patch the list subtest fail
--> with patch test pass
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch removes previously added validation code from Koha::Patron
as we will rely on the DB structure and relationships to catch the same
problems. This is implemented on bug 19828.
This patch also adapts the API controller class to expect this behaviour
change from Koha::Patron. The expected exceptions are adjusted, and some
minor changes take place. The API tests are adjusted as well.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Patrons.t
k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests should still pass
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Works perfectly. Well done everyone!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch refactors the original work so it implements the controllers
and the spec using Mojolicious::Plugin::OpenAPI, and OpenAPI for the specification.
It removes the ability for patrons without permissions to edit their own data or their
guarantee's. This will be moved to a patron modification requests endpoint for simplicity.
It makes use of bugs 19410 and 19686 and their dependencies to deal with parameters handling,
query building and pagination.
Tests are adapted.
To test:
- Apply this patches and the dependencies
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
- Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds support for add, edit and delete patrons via REST API.
GET /api/v1/patrons Get patron list from params
GET /api/v1/patrons/<borrowernumber> Get single patron
POST /api/v1/patrons Create a new patron
PUT /api/v1/patrons/<borrowernumber> Update data about patron
DEL /api/v1/patrons/<borrowernumber> Delete a patron
Revised Test plan:
1) Apply this patch
2) Run tests perl t/db_dependent/api/v1/patrons.t
3) Add a user with proper rights to use the REST API
4) play with your favourite REST client (curl/httpie, etc.):
Authenticate with the user created above and get a CGISESSION id.
Use the CGISESSION to add, edit and delete patrons via the API.
5) Use PUT /patrons/<borrowernumber> for a patron without borrowers
flag. This should go into pending patron modification status and
needs to be accepted by a librarian.
Please note there is no validation of body input in PUT/POST other
than branchcode,category,userid,cardnumber.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We likely shoudln't pass through an uncoverted sort order for now, but
it does allow us to look ahead to implementing the orders directly so
seems a good option to have.
Either this patch should be used, or the commented out tests should be
removed
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Currently sort order is extracted from sort condition by splitting the
field, instead use regular expression to extract the last part preceded
by underscore.
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Enable Zebra
2 - Perform an auth search
3 - note results
4 - Enable ES
5 - Repeat search, note (likely) diff results
6 - Open a record in cataloging and use the button to launch auth search
7 - Perform same search as above, note results match for eiher engine
selected
8 - NOTE: Disbale sorting for ES search - this will be dealt with in
another report
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Edit a framework and add the 'upload/pl' plugin to the 856 field
2 - Attach a pdf to one record
3 - Attach a different type of file to another
(To attach, edit a record in the framework edited above and find the
helper by the 856 field)
4 - Click the links on the record
5 - Verify the pdf opens inline
6 - Verify the other file does not
7 - prove t/db_dependent/Upload.t
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The symbol of currencies are not displayed (or not correctly) so far.
This patch set adds the ability to configure the display of the symbol
(with or without a whitespace between the symbol and the price).
Test plan:
1. Execute the update DB entry, go to the currency admin page and tick the
new "Space separation between symbol and value" checkbox
2. Add a fine to a patron and use their credentials to login at the OPAC
3. You should see the "$ 42 due fines and charges" info in the
dashboard
4. Untick the new checkbox to remove the space and reload the OPAC main
page.
=> The space should not be longer displayed ($42)
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The template plugin did not pass the with_symbol flag to the module and
so was not taken into account
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Run through the password reset process, and your server
should send the message immediate, not waiting for the
cronjob.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
If you have an item-level hold, changing an attribute (like priority,
pickup location or suspended until date) makes the hold a biblio-level
hold, because ModReserve is not passed the current itemnumber.
This patch uses the hold's itemnumber and passes it to the ModReserve
call.
To test:
- Run the regression tests from the previous patch:
$ kshell
k$ prove t/db_dependent/api/v1/holds.t
=> FAIL: Tests fail
- Apply this patch
- Run the tests:
$ kshell
k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We do not need to format the date, DBIC handles DateTime correctly.
It fixes t/db_dependent/Letters.t with new SQL modes
Test plan:
prove t/db_dependent/Letters.t
prove t/db_dependent/Koha/Acquisition/Order.t
Must return green
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
As said previously, GetMemberAccountBalance returns ( the balance, the
non issues charges, the other charges)
The other charges are the balance - the non issues charges.
In this patch we remove the subroutine from C4::Members and use the
new Koha::Account->non_issues_charges subroutine instead
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The GetMemberAccountRecords may be a perf killer, it retrieves all the
account lines of a patron and then the related item and biblio
information.
Most of the time we only want to know how much the patron owns to the
library (sum of amountoutstanding). We already have this information in
Koha::Patron->account->balance.
This patch replaces the occurrences of this subroutine by fetching only
the information we need, either the balance, the detail, or both.
It removes the formatting done in the module, to use the TT plugin
'Price' instead.
There is a very weird and error-prone behavior/feature in
GetMemberAccountBalance (FIXME): as the accountlines.accounttype is a
varchar(5), the value of the authorised value used for the
ManInvInNoissuesCharge pref (category MANUAL_INV) is truncated to the 5
first characters. That could lead to unexpected behaviors.
On the way, this patchset also replace the GetMemberAccountBalance
subroutine, which returns the balance, the non issues charges and the
other charges. We only need to have the balance and the non issues
charges to calcul the third one.
Test plan:
Add several fees for a patron and play with HoldsInNoissuesCharge,
RentalsInNoissuesCharge and ManInvInNoissuesCharge.
The information (biblio and item info, as well as the account line) must
be correctly displayed on the different screens: 'Fines' module, fine
slips, circulation module
Note that this patchset could introduce regression on price formatting,
but will be easy to fix using the TT plugin.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch fixes two errors that slipped in the patchset.
To test:
- Create a dummy branch for testing:
$ cd kohaclone
$ git fetch
$ git checkout v17.11.00 -b dummy
- Reset your working DB
$ reset_all (y)
- Set your branch to current master
$ git reset --hard origin/master
- Update the DB
$ updatedatabase
- Update the schema files
$ kshell
k$ misc/devel/update_dbix_class_files.pl \
--db_name koha_kohadev \
--db_user koha_kohadev \
--db_passwd password
k$ exit
$ git diff
=> FAIL: There are discrepancies on upgrades
- Reset to v17.11.00 revision and DB:
$ git reset --hard v17.11.00
$ reset_all (y)
- Set your branch to current master
$ git reset --hard origin/master
- Apply this patch
- Update the DB
$ updatedatabase
- Update the schema files
$ kshell
k$ misc/devel/update_dbix_class_files.pl \
--db_name koha_kohadev \
--db_user koha_kohadev \
--db_passwd password
k$ exit
$ git diff
=> SUCCESS: No discrepancies!
- Reset to HEAD to get rid of the schema changes
$ git reset --hard HEAD
- Regenerate the schema files on top of this patch
$ dbic ; cd /home/vagrant/kohaclone
$ git diff
=> SUCCESS: No discrepancies!
- Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We should call Koha::Patron->is_expired in CanBookBeIssued instead of
doing the same calculation.
Tests have been adapted to pass with new SQL modes.
We should not need to update the values in DB, we already have
Bug 14717: Prevent 0000-00-00 dates in patron data (3.21.00.023)
Test plan:
prove t/db_dependent/Circulation/dateexpiry.t
prove t/db_dependent/Koha/Patrons.t
must return green
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a new "Not for loan" status filter to the lost items
report.
Test plan:
0/ Apply all patches from this patch set
1/ Use the new "Not for loan" filter to search lost items
2/ The table result must be consistent and a new "Not for loan" column
should be there
3/ Confirm that you are able to hide/show this column with the column
settings tool.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Sponsored-by: Hotchkiss School
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Sponsored-by: Hotchkiss School
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch implements the changes required by the cities endpoint RFC
[1].
It uses the objects.search helper, and relies on bug 19686.
To test:
- Apply the patches
- Compare the spec with the RFC (api/v1/swagger/definitions/city.json)
=> SUCCESS: It makes sense
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/cities.t
=> Tests pass!
- Sign off :-D
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In our installation procedure we ask the administrator to edit the
MySQL|MariaDB configuration file to specify the SQL modes we support
(see bug 17258 comment 6 and 7 for more information).
We are on the way to catch and fix all these issues and support these
stricter modes (as they highlight problem in our codebase/DB structure)
but in the meanwhile it may be good to remove this step and revert the
changes when we are ready.
TODO:
- Remove that for dev installations (to let developers catch these bugs)
- Edit the wiki page to remove this step
Test plan:
0. Do not apply this patch
1. Edit your MySQL|MariaDB config and add:
sql-mode = "ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION"
to the [mysqld] section (or edit it)
2. Restart your DBMS
3. Try to make the app explode (watch the logs)
(tips: you should get "'koha_kohadev.me.id' isn't in GROUP BY" when
editing an order)
4. Apply the patch, restart_all, restart your DBMS
5. Try to recreate the failure
=> You should no longer see the error in the logs
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds (yet) another param to objects.search: a reference
to a to_api function to be applied when processing the search results
for output.
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> SUCCESS: Test count raised, and tests pass!
- Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces a new parameter to the objects.search Mojo helper.
The idea behind this, is that if there's any attribute name mapping
between the DB objects and the API exposed ones, we should be able to
pass it to objects.search so the filtering query params are mapped
correctly for building the DBIC query, like this example:
my $patrons_set = Koha::Patrons->new;
my @patrons = $c->objects->search( $patrons_set,
\&to_model
)->as_list;
# and probably
@patrons = map {to_api($_)} @patrons;
The to_model function needs to avoid autovivification, to prevent
messing with the query parameters (undef params).
To test:
- Apply this patches
- Run:
$ kshell
k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> SUCCESS: Tests pass! Params get mapped!
- Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When a test was introduced to not add duplicate userids
it overlooked that if you were matching on userid you
do not want the update to be rejected.
Add a test to overcome this logical flaw if updating
by matching userid
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch will have to be squashed with previous ones, but for
readability I prefer to let it alone for now.
It reintroduces 2 missing changes:
commit fd12390365
Bug 15010: Import patron tool creates 'duplicate' restrictions (
debarments )
commit 011d8e9855
Bug 12543 - Add userid as matchpoint for "Import patrons" tool
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Tests will have to be written for this change
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test Plan:
1) Apply this patch
2) Test importing patrons from command line,
options are availble with --help.
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested with minimal csv
(cardnumber,surname,firstname,categorycode,branchcode,password,userid)
Overwrite does not change category or branch.
Patrons are loaded, userid & password works
Updated license to GPLv3
No other koha-qa errors.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Tidy import_borrowers.pl
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Move importing code to a subroutine
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Update command line script to use patron import subroutine
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 [QA Followup]
* Fix copyright on import_borrowers.pl
* Changes -c --csv to -f --file
* Adds -c --confirm option
* Renames misc/import_borrowers.pl to misc/import_patrons.pl
* Restore userid matchpoint option
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Fix merge to master. Backport 3 updates from latest import_borrowers.pl
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Started regression tests. Fix missing C4::Members::Attributes package
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - More refactoring and regression tests in Koha::Patrons::Import
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Creating objects in misc/import_patrons.pl and tools/import_borrowers.pl
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Refactoring Koha::Patrons::Import includes bug fixed for critical date types and header column parsing
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Rebase + backport of 16426 plus fixing 16426
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598: catch warnings raised by import_patrons in tests
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch add 2 methods to the Koha::Patron module: is_adult and
is_child.
That way the business logic is at only one place and covered by tests.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Not trivial, more attention needed on this one!
Fix for:
'koha_kohadev.me.cardnumber' isn't in GROUP BY
Looks like the query is complexified, perf tests needed, see bug 19599.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Fix at ->store level, this issue appears in lot of places.
Fix for:
Field 'datecreated' doesn't have a default value
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Add POD for new sub in Koha/IssuingRules.pm.
Adding use Koha::IssuingRules to opac-detail and opac-MARCdetail.
Adding use Koha::Items to opac-detail and opac-MARCdetail.
Correct typo $items => $item in opac-MARCdetail.pl.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Following the pattern introduced by bug 19300, we are going to move the
OnShelfHoldsAllowed logic to Koha::IssuingRules->get_onshelfholds_policy
Test plan:
Make sure the onshelfholds policy is correct when placing a hold
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
- Checkout an item
- Place hold on this item,
- Return the item
- Make sure the hold is waiting (found W) and AllowOnShelfHolds is
not to 'Allow'
- Check that the button "Place hold" appears in opac detail page of
the biblio
- do the samewith items/reserves in transit
Changes on C4::Reserves::IsAvailableForItemLevelRequest
Make sure this tests pass:
- t/db_dependent/Reserves.t
- t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
Rebased - 2017-12-12 - Alex Arnaud
Bug 4319 - [QA fix] Create Koha::Biblio->hasItemswaitingOrInTransit
Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Perl::Critic found these violations in "Koha/Patron.pm":
"return" statement followed by "sort" at line 798, column 5. Behavior
is undefined if called in scalar context. (Severity: 5)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I do not exactly why but there is a conflict in the name of the method
prove t/db_dependent/api/v1/patrons.t failed with
[Mon Feb 12 17:13:16 2018] [error] Can't use string ("TO_JSON") as a
HASH ref while "strict refs" in use at
/home/vagrant/kohaclone/C4/Auth.pm line 2053.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Instead of basing the group searches on the group name, which is an
inherently touchy system, we should use the same checkbox style that
Jonathan introduced for the patron limits by group feature.
Test Plan:
1) Check to ensure existing group searches still show as they used to
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In order to control this feature correctly it needs to be enabled for
group of libraries and not for all groups defined in the system.
Groups will be used for different usages and so the feature must not be
enabled by default for all of them.
Test plan:
Retest bug 18403 with the feature turned on/off for a given tree
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
With one root group and two child groups and it works fine.
But if I create any groups under *those* groups, not only does it not work,
I get the following error:
Template process failed: undef error - SQL::Abstract::puke(): [SQL::Abstract::__ANON__]
Fatal: SQL::Abstract before v1.75 used to generate incorrect SQL when the -IN operator was
given an undef-containing list: !!!AUDIT YOUR CODE AND DATA!!! (the
upcoming Data::Query-based version of SQL::Abstract will
emit the logically correct SQL instead of raising this
exception) at /home/vagrant/kohaclone/Koha/Objects.pm line 269
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
On first login, Koha explodes before the logged in user does not exist
in DB.
This patch deals with that by adding several checks when it's needed.
Test plan:
Use the DB user to create a superlibrarian user.
The DB user should no be allowed to do anything else.
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Bug 18403: Fix patron creation
memberentry.pl can be called to create a new patron, in that case the
patron does not exist yet.
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There is something wrond here, the userenv is no set and so we cannot
user search_limited.
Should we set the userenv or filter on the libraries using
libraries_where_can_see_patrons?
WAITING FOR FEEDBACK HERE.
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Same as previously but for article requests.
Test plan:
Test article requests and make sure you do not need the requests for
patrons that
are attached to a group that is not part of your library's group
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch deals with patron's discharges.
Test plan:
Same as previously you will need to request dischages at the OPAC.
On the staff interface the logged in user should not be allowed to see
discharge
from patrons outside his library group.
The number of discharges waiting displayed on the mainpage should be
correct as well.
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Bug 18403: (follow-up) Patron discharges
Fix QA issue:
forbidden pattern: Do not assume male gender, use they/them instead (bug 18432) (line 150)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a new method Koha::Reviews->search_limited to return the
reviews
a logged in user is allowed to see depending his permissions.
Test plan:
Create some reviews at the OPAC and make sure a staff user is limited
(or not) to approve
or decline it.
The number of reviews displayed on the mainpage should be correct as
well.
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Limit patron's modifications based on logged in patron permissions.
Test plan:
Create some patron's modification requests at the OPAC
Make sure the logged in staff user see (or not) the modification depending his
permissions.
The number of modification displayed on the mainpage should be correct as well.
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Technical note:
Sometimes we do not have the patron object, for instance for the patron modifications
we will need to know if the logged in user can modify patron's from a given library.
This new subroutine 'can_see_patrons_from' will then be useful
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Technical note:
Here we are just refactoring a code that have been copied into 3 different places.
libraries_where_can_see_patrons is a terrible method's name, feel free to suggest
something better. The method return a list of branchcodes to be more efficient,
instead of Koha::Libraries
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Most of the time when we search for patrons we do not want to search for all patrons,
but just the ones the logged in user is allowed to see the information.
This patch takes care of that by adding a new search_limited method to Koha::Patrons.
When called this method only search for patrons that the logged in user is allowed
to see.
Test plan:
Patron autocomplete search should be limited
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There is already a HidePatronName syspref to hide patron's information
on bibliographic
record detail pages and the hold list.
Test plan:
With the HidePatronName enabled, make sure the patron's information are
hidden from
the catalogue and hold list pages. If the logged in user is not allowed
to see the
patron's info, no link and no cardnumber will be displayed
With he HidePatronName disabled, make sure the patron's information are
displayed
if the logged in user is allowed to see the patron's info.
Technical note:
This patch improves the existing patron-title.inc include file to
display patron's
information. Using it everywhere patron's details are displayed will
permit to
homogenise the way they are displayed. The file takes now a patron
object (what
should be, in the future, the only way to use it), that way we can call
the new
method on it to know if patron's information can be shown by the logged
in used.
NOTE: I am not sure this syspref makes sense anymore. Should not we
remove it?
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
From where patrons it's about patrons, we do not want to display the libraries
from all the system, but only the ones from the group.
Test plan:
- See the overdues (circ/overdue.pl) and make sure you can only see overdues from
patrons part of your group (do not forget to test the CSV export).
- Search for patrons, the 'library' filters (headers and left side) should only
display libraries from your group
- Search for article request by patron's library: only the libraries from your
group should be displayed
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Technical note:
This is the method that will be called on the logged_in_user variable sent to
the template. Moreover we will check that the logged in user can access patron'
information when access to members/* and some circulation scripts will be done.
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is more a follow-up for bug 15707. It could be moved on its own bug report
if necessary.
IMPORTANT NOTE: At the moment the feature only works for 1 level depth, see
bug 15707 comment 166+ for the discussion
It means that if we have:
root_group
+ groupA
+ groupA1
+ groupA1_library2
+ groupA_library1
+ groupA2
+ groupB
+ groupB_library1
groupA1_library2 is not considered a child of groupA1.
Note that this can change.
Test plan:
prove t/db_dependent/LibraryGroups.t
should return green
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is more a follow-up for bug 15707. It could be moved on its own bug report
if necessary.
Test plan:
prove t/db_dependent/LibraryGroups.t
should return green
Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test Plan:
1) Apply this patch set
2) Note your existing search groups have been ported over to the new
__SEARCH_GROUPS__ group if you had any
3) Create the group __SEARCH_GROUPS__ if one does not already exist
4) Add some first level subgroups to this group, add libraries to those groups
5) Search the library group searching in the intranet and opac
6) Note you get the same results as pre-patch
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch removes the /api/v1/app.pl prefix from generated baseUrl.
It has the consequence of hardcoding / as the base path for the API.
This is ok because we don't currently support mounting Koha pieces in
different than hardcoded paths (/cgi-bin/koha?), but certainly worth
mentioning because this will need to depend on a syspref or config entry
when someone chages this behviour.
To test:
- On master, point your browser to:
http://kohadev.mydnsname.org:8080/api/v1/
=> FAIL: basePath looks like /api/v1/app.pl/api/v1
- Apply this patch
- Restart memcached and plack:
$ restart_all
or
$ sudo systemctl restart memcached && sudo systemctl restart
koha-common
- Reload http://kohadev.mydnsname.org:8080/api/v1/
=> SUCCESS: basePath is /api/v1
- Sign off :-D
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
The warn comes from undefined C4::Context->userenv->{branch}.
Can be triggered by calling mainpage.pl when being logged out in staff.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
1/ Use your usual "REST testing" tool to place a title-level hold with
an itemtype. The request should look like this:
POST /api/v1/holds
{
"borrowernumber": 1234,
"biblionumber": 456,
"branchcode": "CPL",
"itemtype": "A"
}
2/ Check that the hold was placed and the itemtype is correctly selected
3/ prove t/db_dependent/api/v1/holds.t
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The default AR_PENDING letter does not print populate <<article_requests.*>> fields.
Test Plan:
1) Enable article requests ( syspref: ArticleRequests => Enable, Circ and fine rules ALL/ALL (or a given category/itemtype): Article requests => Yes )
2) Search for a title, click in to catalog detail, click 'Request Article' button.
3) Select Patron
4) Fill out request information
5) Click 'Place request'
6) Go to notices in patron record, inspect 'Article Request Received' notice. You should see the request information from step 4, but it doesn't display.
7) Apply this patch
8) Repeat steps 2-6, not the fields now display!
Signed-off-by: Eric Phetteplace <phette23@gmail.com>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
At several places we got the following error if we use numbers too big
for Number::Format
Template process failed: undef error - round() overflow. Try smaller
precision or use Math::BigFloat at /home/koha/src/Koha/Number/Price.pm line 44
It make the app explodes.
The goal here is to handle these errors gracefully and easily.
Test plan:
- Add a manual fine to a patron of 100000000000000
- Create a patron category with an enrolment fee of 123456789012345
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Illrequest->available_backends always returned only 1 element
TEST PLAN
---------
See bug 7317 comment #20 for configuring ILL
Make sure to restart_all
Strangely, ILLIBS didn't exist, but IL did in default install,
so use that as the patron code in your koha-conf.xml
Make sure to turn on the ILL related system preference.
1) Configure ILL.
-- this is the hard part. Hopefully the above will give you
enough.
2) open OPAC and log in
3) Click the 'your interlibrary loan request' in the left pane
4) Click the 'Create a new request' button.
-- when you hover your mouse over the links for the backends,
they will have ARRAY(...) instead of a meaningful name.
This is what triggers the error (see comment #7).
5) Run the following commands
git checkout -b bug_20001 origin/master
git bz apply 20001
restart_all
6) Refresh the OPAC page, and repeat step 4.
-- when hovering your mouse over the links for the backends,
they will have meaningful values now.
7) run koha qa test tools.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
0 - Apply Unit test patch only
1 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
2 - Should fail
3 - Apply this patch
4 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
5 - should pass
6 - search for 'Local-number.raw:"4"' (or a vlid biblionumber)
7 - should get expected result
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
If memcached or the connection to it is misconfigured, show simple warnings to help
identify the problem.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This subroutine is quite trivial and can be replaced easily with a new
method of Koha::Patron
Test plan:
Overdue notices and shelf sharing must be send the to an email address,
according to the value of the pref AutoEmailPrimaryAddress
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This new Koha::Patron->first_valid_email_address already exists, it
should be called directly
Test plan:
- With a logged in user who have an email address defined, send a basket.
The email should be marked sent by this email address
- On the "Holds awaiting pickup", the email address must be displayed as well
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patchset move The OPACItemHoldsAllowed logic
(issuingrules.opacitemholds) to a new class method of
Koha::IssuingRules: get_opacitemholds_policy
On the way, this patch will certainly fix the same problem as bug
19298 with onshelfholds.
Test plan:
Make sure the opacitemholds policy is correct when placing a hold at the
OPAC or the staff interface.
Followed test plan which worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch proves that we will not introduce trivial regression.
With the same tests, we will execute the existing code and the new code.
Test plan:
With only this patch applied, prove t/db_dependent/Koha/IssuingRules.t
should return green
Followed test plan, patches worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces Koha::Object(s)-derived classes for budgets and funds (aqbudgetperiods and aqbudgets respectively).
C4::Budgets already uses DBIC calls on those tables so transition should be easy on top of this.
To test:
- Check the referenced schema files are the right ones.
- QA scripts shoudl be happy.
- Sign off :-D
Note: the POD coverage report throws a false negative result on singular classes which is trivially wrong.
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces a try/catch block in store() and parses the error
when an exceptional situation takes place.
It only deals with FK constraint violations and duplicate IDs. The rest of the DBIC exceptions
are rethrown.
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We got a lot of support questions because scripts are executed without
the correct environment/user.
We could die and provide more useful information
Tested on my kohadevbox VM by:
1) sudo chmod a-rx /etc/koha/sites/kohadev/koha-conf.xml
2) run help.pl from the command line to produce the desired die message
3) sudo chmod a+rx /etc/koha/sites/kohadev/koha-conf.xml
4) rerun help.pl successfully
Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Bug 19760: Change wording
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Since bug 17196, biblioitems.timestamp is not always updated after a
change in the MARC record.
Filtering should be based on biblio_metadata.timestamp instead.
Test plan:
1. prove t/db_dependent/OAI/Server.t
2. Verify that it SUCCEEDS
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The query generated by anonymise_issue_history is unecessary complex and
can be very slow on large DB.
With this patch we will execute one query per patron instead.
See patch "Bug 19599: Simplify the query" on the bug report for an
alternative.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
prove t/db_dependent/Koha/Patrons.t
must return green
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It turns out the modules had the option expected built in, we just didn't call
it. This patch set expands the options for passing to the export.
To test:
1 - Go to Tools->Export data
2 - Perform an export and check 'Remove non-local items'
3 - Note that file contains only 'local' records, but includes all items
on those records
4 - Apply patch
5 - Note Tools->Export data has a new option to remove records not owned
by logged in branch
6 - Export as before checking records option, file should be as before
7 - Now check 'Remove items not owned by logged in branch'
8 - File should now only have local items (may have empty records)
9 - Check both boxes and recieve only 'local' records and items
Signed-off-by: Scott Kehoe <scott@masslibsystem.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch fixes a perl critic issue, hopefully it will not break
something else.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch formats timestamps and datetimes to meet date-time definition
in RFC3339, as required by Swagger documentation.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1. prove t/DateUtils.t
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch creates the 'build_query_params' helper, instead of the
original function in Koha::Objects.
Unit tests are removed for Koha::Objects::_build_query_params_from_api and
written for the helper plugin.
The objects.search helper gets a call to build_query_params added. Tests for it
updated to match this behaviour change.
To test:
- Apply this patches
- Run:
$ kshell
k$ prove t/Koha/REST/Plugin/Query.t \
t/db_dependent/Koha/Objects.t \
t/db_dependent/Koha/REST/Plugin/Objects.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Following the discussion about the best way to make things simpler for
developing the REST api, I quote Lari's email:
"As many other endpoint will have the exact same usage, by looking at your example, I would prefer to avoid writing parameter / pagination / sorting / header handling for each list operation in our API controllers. Do you think it's possible to centralize all of this e.g. by passing $c into a customized search sub? Perhaps in Koha::Objects?
so instead we could have something like (ignore my bad choice of naming)...:
sub list_vendors {
my $c = shift->openapi->valid_input or return;
my $args = $c->validation->output;
my $vendors;
return try {
$vendors = Koha::Acquisition::Booksellers->api_list_search($c);
return $c->render(status => 200, openapi => $vendors);
} catch {
...
}
}"
We all agreed we neeed something like that. Here's a possible implementation. I take
advantage of the previously written Mojo helpers, that are fully covered by tests.
I submit this early so anyone can take a look and gather ideas to make it even better.
I'm already using it (effectively) for the /acquisitions/orders endpoint I'm writing
on bug 18731.
Thanks!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the helper handling _order_by params expect a list of
values instead of a (to-be-splitted) string.
The idea is that the OpenAPI plugin will take care of splitting
pipe-delimited values if the spec is correctly defined.
Note: In the process I noticed + on the URL represents a space, so the
helper function is updated to handle both + and %2B as ascending.
To test:
- Run:
$ kshell
k$ prove t/Koha/REST/Plugin/Query.t
=> SUCCESS: Tests pass!
- Sign off :-D
Edit: Removed rebasing leftover making the tests fail.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces a helper function called 'dbic_merge_sorting'
to Koha::REST::Plugin::Query.
This simple function adds SQL::Abstract order_by attribute to the passed
$filter hashref, as explained in the POD.
It introduces a syntax for passing sorting params on the request to the REST api.
The proposed syntax has been found in the wild, and is pretty trivial to parse/work with:
GET /api/v1/<endpoint>?order_by=+column_1|-column_2|column_3
As explained on the POD, + stands for 'asc' and - for 'desc'. If ommited, it defaults to the
DB engine default (usually asc).
To test:
- Apply this patches
- Run:
$ sudo koha-shell kohadev
k$ cd kohaclone
k$ prove t/Koha/REST/Plugin/Query.t
=> SUCCESS: Tests pass! And they make sense! :-P
- Sign off :-D
Edit: renamed params to match DBIC terminology. My bad :-D (tcohen)
Sponsored-by: Camden County
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds information about RESTdefaultPageSize usage
when the per_page parameter is absent on the query.
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This bug introduces a new syspref RESTdefaultPageSize that is used on
the Koha::REST::Plugin::Pagination plugin to default to a valid value
for the page size if the param is not present on the query.
This patch also considers the situation in which no 'page' param is
passed, and no header should be added.
To test:
- Run:
$ sudo koha-shell kohadev
k$ cd kohaclone
k$ prove t/Koha/REST/Plugin/Pagination.t
=> FAIL: Tests fail because the plugin doesn't behave as the tests
expect.
- Apply this patch
- Run:
k$ prove t/Koha/REST/Plugin/Pagination.t
=> SUCCESS: Tests pass!
- Sign off :-D
Sponsored-by: Camden County
Sponsored-by: ByWater Solutions
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces a new helper function to the Koha::REST::Plugin::Pagination
plugin, called 'dbic_merge_pagination'.
This simple function adds SQL::Abstract pagination attributes ('page' and 'rows') to the
passed $filter hashref.
To test:
- Apply this patches
- Run:
$ koha-shell kohadev
k$ cd kohaclone
k$ prove t/Koha/REST/Plugin/Pagination.t
=> SUCCESS: Tests pass!
- Sign off :-D
Sponsored-by: Camden County
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
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: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces a Mojolicious plugin to be used on the REST api.
It adds a helper method:
generate_dbic_query
===================
When used, it generates what's needed to perform a search on
DBIC/Koha::Objects like this:
my $params = $c->validation->output;
my ($filtered_params, $reserved_params) = $c->extract_reserved_params($params);
my $filter = do_smth($filtered_params, $reserved_params);
my $attributes = do_smth_reserved($reserved_params);
my $patrons = Koha::Patrons->search( $filter, $attributes );
It introduces reserved param names:
- _match
- _order_by
- _page
- _per_page
They are reserved for later usage (pagination, matching algorithm on building DB queries)
All the plugin's behaviour is tested.
To test:
- Run:
$ sudo koha-shell kohadev
k$ cd kohaclone
k$ prove t/Koha/REST/Plugin/Query.t
=> SUCCESS: Tests pass!
- Sign off :-D
Sponsored-by: ByWater solutions
Sponsored-by: Camden County
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a new statuc function to Koha::SearchEngine::ElasticSearch
which is instended to replace most of get_elasticsearch_params. This function
reads the configuration from C4::Context->config('elasticsearch') and raises
relevant exceptions when mandatory entries are missing.
Its behaviour is covered by tests.
To test:
- Run:
$ kshell
k$ prove t/Koha/SearchEngine/Elasticsearch.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This code seems a bit odd, $self->sort_fields()->{$name} is checking if
there is any mapping in ES for a field, so if one marc field in an index
is marked as sortable this will exists. We need to check the individual
marc field to see if it should be added to the sort index here.
If you apply the first patch, reindex, and view a record in es:
curl -XGET 'localhost:9200/koha_kohadev_biblios/data/19/?pretty' | grep
-A 10 author__sort
You will see that 245$c is included in the record without that
additional check
Apply this, reindex again, and that field should not be included
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Fix incorrect condition for if clause for generating <field>__sort mappings
for Elasticsearch. Also remove redundant check for same condition when
generating fixer rules.
Test plan:
1. Inspect current mappings for example by viewing: http://<elasticsearch_host>:9200/koha_<koha_instance_name>_biblios/_mapping.
2. If using the default configuraion only "author" has a sort field (author__sort).
4. Appy patch.
5. Reindex using rebuild_elastic_search.pl.
6. All fields except those with sort sort set to "0" should now have sort fields, which in the default configuration is all but "author".
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch drops the notifys table and its related code in C4::Overdues.
A second patch should remove the 2 columns notify_id and notify_level
from the accountlines table.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
After feedback from the dev mailing list, it seems appropriate here to
propose making the Default framework authoritative for Koha to MARC
mappings. This implies checking only the Default framework in the
routines:
[1] GetMarcFromKohaField: The parameter frameworkcode is removed. A
follow-up report (19097) will update the calls not adjusted here.
This is safe since the parameter is silently ignored.
[2] GetMarcSubfieldStructureFromKohaField: Framework parameter is removed
and calls are adjusted. Includes acquisitions_stats.pl.
[3] TransformKohaToMarc: The parameter is removed; all calls are verified
or adjusted.
[4] TransformMarcToKoha: The parameter is no longer used and will be
removed in a follow-up report (19097). It always goes to Default now.
[5] TransformMarcToKohaOneField: The parameter is removed and all calls
are adjusted. Including: Breeding, XISBN and MetadataRecord modules.
[6] C4::Koha::IsKohaFieldLinked: This routine was called only once (in
C4::Items::_build_default_values_for_mod_marc. It can be replaced by
calling GetMarcFromKohaField. If there is no kohafield linked, undef
is returned. (Corresponding unit test is removed here.)
[7] C4::Items::ModItemFromMarc: The helper routine
_build_default_values_for_mod_marc does no longer have a framework
parameter. The cache key default_value_for_mod_marc- is no longer
combined with a frameworkcode. Three admin scripts are adjusted
accordingly; some tests will be corrected in the next patch.
Test plan:
See next patch. That patch adjusts all tests involved.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In order to allow multiple Koha to MARC mappings (for one kohafield), we
need to adjust a few key routines in C4/Biblio.pm. This results in a few
changes in dependent modules too.
Note: Multiple mappings also include 'alternating' mappings. Such as the
case of MARC21 260 and 264: only one of both fields will be used. Sub
TransformMarcToKoha will handle that just fine; the opposite transformation
is harder, since we do no longer know which field was the source. In that
case TransformKohaToMarc will fill both fields. We only use that operation
in Koha for items (in Acquisition and Cataloging).
Sub GetMarcSubfieldStructure
This sub used a selectall_hashref, which is fine as long as we have only
one mapping for each kohafield. But as DBI states it: If a row has the same
key as an earlier row then it replaces the earlier row. In other words,
we lose the first mapping if we have two.
This patch uses selectall_arrayref with Slice and rearranges the output so
that the returned hash returns an arrayref of hashrefs for each kohafield.
In order to improve consistency, we add an order clause to the SQL
statement used too.
Sub GetMarcFromKohaField
This sub just returned one tag and subfield, but in case of multiple
mappings we should return them all now.
Note: Many calls still expect just one result and will work just fine:
my ($tag, $sub) = GetMarcFromKohaField(...)
A possible second mapping would be silently ignored. Often the sub is
called for biblionumber or itemnumber. I would not recommend the use of
multiple mappings for such fields btw.
In case the sub is called in scalar context, it will return only the first
tag (instead of the number of tags and subfields).
Sub GetMarcSubfieldStructureFromKohaField
This sub previously returned the hash for one kohafield.
In scalar context it will behave like before: it returns the first hashref
in the arrayref that comes from GetMarcSubfieldStructure.
In list context, it returns an array of all hashrefs (incl. multiple
mappings).
The sub is not used in C4::Ris. Removed the use statement.
Sub TransformKohaToMarc
This sub got a second parameter: frameworkcode.
Historically, Koha more or less assumes kohafields to be defined across all
frameworks (see Koha to MARC mappings). Therefore it falls back to Default
when it is not passed.
When going thru all mappings in building a MARC record, it also supports
multiple mappings. Note that Koha uses this routine in Acquisition and in
Cataloging for items. Normally the MARC record is leading however and the
Koha fields are derivatives for optimization and reporting.
The added third parameter allows for passing a new option no_split => 1.
We use this option in C4::Items::Item2Marc; if two item fields are mapped to
one kohafield but would have different values (which would be very unusual),
these values are glued together. When transforming to MARC again, we do not
want to duplicate the item subfields, but we keep the glued value in both
subfields. This operation only affects items, since we are not doing this
reverse operation for biblio and biblioitem fields.
Sub _get_inverted_marc_field_map
This sub is a helper routine of TransformMarcToKoha, the opposite
transformation. When saving a MARC record, all kohafields are extracted
including multiple mappings.
Suppose that you had both 260c and 264c in your record (which you won't),
than both values get saved initially into copyrightdate like A | B. The
additional code for copyrightdate will extract the first year from this
string.
A small fix in TransformMarcToKoha makes that it only saves a value in a
kohafield if it is defined and not empty. (Same for concatenation.)
Sub TransformMarcToKohaOneField
This sub now just calls TransformMarcToKoha and extracts the requested
field. Note that since we are caching the structure, this does not result
in additional database access and is therefore performance-wise
insignificant. We simplify code and maintenance.
Instead of modifying the passed hashref, it simply returns a value. A call
in C4::Breeding is adjusted accordingly. The routine getKohaField in
Koha::MetadataRecord is redirected to TransformMarcToKohaOneField.
NOTE: The fourth patch restructures/optimizes TransformMarcToKoha[OneField].
Sub get_koha_field_from_marc
This sub can be removed. A call is replaced by TransformMarcToKohaOneField
in C4::XISBN.
Note: The commented lines for sub ModZebrafiles are removed (directly under
TransformMarcToKohaOneField).
Test plan:
For unit tests and interface tests, please see follow-ups.
Run qa tools in order to verify that the modules still compile well.
Read the code changes and verify that they make sense.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan assumes
QueryAutoTruncate = automatically
SearchEngine = Elasticsearch
To test:
0 - Apply Unit test patch only
1 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
2 - Should fail
3 - Apply this patch
4 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
5 - should pass
6 - search for 'Local-number:"4"' (or a vlid biblionumber)
7 - should get expected result
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
JSON does not allow real line-breaks. If a field contains them and they are not escaped, a JSON parser will be unable to convert the stringified JSON back into an object.
This is clearly exemplified by the guarantor search, where a multi-line note on the guarantor's record will break the ability to select that guarantor.
Test Plan:
1) Create Patron A with a "Circulation note" that has multiple lines in it
2) Create Patron B
3) Attempt to set Patron A to be the guarantor for Patron B
4) Note selecting the patron does nothing
5) Apply this patch
6) Repeat step 3
7) Selecting the guarantor now works!
Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch removes references to UnmediatedILL. This feature is introduced on
a separate bug.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch fixed terminology issues on the UI, and removes CSS-based
case forcing for column names.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
5/ This patch makes Koha::Illrequest->load_backend raise an exception
if the passed backend is invalid. This way we will catch more errors introduced.
The patch also disables the 'New Ill request' when no backends are available. Gets
rid of a related warnings.
Both OPAC and Intranet now display a warning message when no backends
are available.
Tests are added for the load_backend changes.
4/ This patch fixes the path for the checkboxes jquery plugin, and removes the include
for tablesorter, as this implementation uses Datatables. This is obviously code for older
Koha, ported to master.
TODO: There's something wrong on the styling. My idea is to get rid
of the custom column visualization tool, and have it display as regular
DataTables. We can then introduce the use of colvis on a separate bug
report.
Note: POD coverage for the exceptions file is wrongly tested. It is a false positive.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch moves the current endpoint implementation from Swagger2 to
the OpenAPI plugin.
It also takes advantage of the overloaded Koha::Illrequest::TO_JSON method
which has now the option to embed what's needed for the REST api.
The path spec is adjusted to fit OpenAPI, and some minor fixes are
applied:
- Missing 'metadata' query param
- 'ill' permissions should be required instead of 'borrowers'
- Full test coverage
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/illrequests.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch implements an overloaded TO_JSON method, that introduces
the option to (selectively) embed information required on the REST
api.
Tests are included for the new method.
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/Illrequests.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This Commit is at the heart of adding an interlibrary loans framework
for Koha. The framework does not prescribe a particular workflow.
Instead it provides a general framework that can be extended &
implemented by individual backends whose responsibility it is to
implement a specific workflow.
The module is largely self-sufficient: it adds new tables to the Koha
database and touches only a few files in the Koha source tree.
Primarily, we add our files to the Makefile and the koha-conf.xml,
define ill paths for the REST API, and introduce links from the main
intranet, opac pages & user permissions.
Outside of this we simply add new files & functionality.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The test needs the code level in the error hash now.
And POD is adjusted accordingly.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - sudo chown www-data:www-data /tmp/koha_kohadev_upload/ (or your
relevant dir and user)
2 - Attempt to stage a file
3 - Error message is: Upload status: Failed
4 - Apply patch
5 - Repeat upload
6 - Error is now: Failed - Directory is not writeable:
/tmp/koha_kohadev_upload
7 - Trigger the other errors if you can and note better responses
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Similar to the full path test in sub themelanguage, this patch makes a
change in _get_template_file. This allows you to pass a template
outside the modules directory to get_template_and_user. (Note: the sub
badtemplatecheck already blocks bad paths.)
Especially, this would be helpful for plugins using templates. As can be
seen in Templates.pm, a change was made earlier to overwrite the filename
for a plugin in sub gettemplate. This exception can now be removed.
Also note the small change in Koha/Plugin/Base.pm; mbf_path is already
absolute and if we pass a full path, we do not need it. This allows use of
a regular Koha template or a shared template between plugins (as long as
badtemplatecheck allows the path).
What are the side-effects of this change?
[1] We should not pass absolute paths if we mean relative ones.
A follow-up patch deals with one occurrence in the codebase.
No regressions for regular use.
[2] Plugins can call get_template_and_user directly or go via get_template
in Koha/Plugin/Base (absolute paths don't go via mbf_path).
Note: replaced two single quotes in Auth.pm to show template name in test
description.
Test plan:
[1] Open some page on OPAC or staff client to trigger a template.
[2] Run t/db_dependent/Auth.t to verify not allowing some bad templates.
[3] Run t/db_dependent/Templates.t to verify an absolute path.
[4] Run t/db_dependent/Plugins.t to verify using templates in a plugin.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The bad template check in get_template_and_user can be removed, since
this check has been added in gettemplate on bug 18010.
The check moves here to a new sub in C4::Templates. And will be extended
in a follow-up.
Removed unused variable $path in gettemplatefile.
Test plan:
[1] Run t/db_dependent/Auth.t (get_template_and_user bad calls).
[2] Run t/db_dependent/Templates.t (bad call to gettemplate).
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
Run t/db_dependent/Koha/Patrons.t
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Subroutine prototypes used at line 6, column 1. See page 194 of PBP.
(Severity: 5)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
[1] Replace corrosponding => corresponding
[2] Replace containts => contains
[3] Replace item_level-itypes => item-level_itypes
[4] Replace Managment => Management
[5] Replace should returns => should return
Test plan:
Note that this patch only deals with POD lines or test descriptions.
So there is nothing to test, just read the patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Patch amended by RM: The release notes should not be modified
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jonathan Druart
Nick Clemens
Kyle M Hall
Ere Maijala
Tomas Cohen Arazi
Josef Moravec
Marcel de Rooy
David Cook
Alex Arnaud
Kyle M Hall
Kyle M Hall
Lari Taskula
Charles Farmer
Julian Maurice
xljoha
Fridolin Somers
Katrin Fischer
Janusz Kaczmarek
Mark Tompsett
Benjamin Rokseth
David Gustafsson
Tomas Cohen Arazi
Colin Campbell
Olli-Antti Kivilahti
Magnus Enger
Alex Sassmannshausen