This patch acknowledges the fact that some checks here are done in a way
that is not standard these days. This checks are originally done
multiple times in different ways. output_and_exit_if_error should be
used.
This implies a minor behaviour change: if the passed (in the URL)
borrowernumber doesn't exist, it sends the user to a 'Patron doesn't
exist page' instead of the circulation page for the borrowernumber.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
I think unsafe SQL modes and the fact that manager_id has no FK allowed this to go unnoticed. But now we catch it!
To test:
- On a patron, try adding a new manual credit of any type
=> FAIL: It fails telling the userid of the logged user is not a valid integer!
- Apply this patch
- Try adding a manual credit of any type
=> SUCCESS: Manual credit added!
- Sign off!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch changes the logic inside the method, to make it match the
behaviour described on the tests.
It uses the existing offsets on the account_offsets table to gather
information about the right things to refund.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/Circulation.t
=> FAIL: Tests don't pass!
- Apply this patch
- Run
k$ prove t/db_dependent/Circulation.t
=> SUCCESS: Tests pass!
- Sign off :-D
Followed test plan, patch works as described. All three patches pass QA
test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch introduces tests for all the use cases for
_FixAccountForLostAndReturned. This includes the following scenarios
before _FixAccountForLostAndReturned is called:
- Full write off (the original issue)
- Full payment
- No payment or write off
- Partial payment and partial write off
Written off debts (L lines) are just skipped, any kind of payment is
added and then refund as a single credit (CR). Amount outstanding is set
to 0. Offsets need to be generated for paper trail of this decisions.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/Circulation.t
=> FAIL: Tests fail because _FixAccountForLostAndReturned is not doing
the right thing :-D
Followed test plan, patch works as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch introduces this trivial method for returning the sum0 result
of the amount columns of a Koha::Account::Offsets set.
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Account/Offsets.t
- Sign off :-D
Followed test plan, patch worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
It seems that I have over complicated things here, now
t/db_dependent/Koha/Object.t is failing
Apparently everything is ok with this change.
Test plan:
prove -r prove -r t/db_dependent/Koha/Object.t t/Auth_with_shibboleth.t
t/db_dependent/Patrons.t t/db_dependent/Koha/Patron*
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To test:
1 - Bring up a patron
2 - Click 'Modification log'
3 - Note you get borrower info
4 - Move the object field into info field
5 - Submit
6 - You are now on the 'Log viewer' page with admin options, not
borrower circ options
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To recreate:
- View a patron detail page
- Click "Modification log"
- Notice that the url has "modules=MEMBERS&modules=circulation", but
the "Circulation" option is not selected in the "Modules" dropdown list.
This is because of uppercase vs lowercase.
The bug:
- Right after you click "Modification log" you will see all the
"members" and "circulation" logs
- Submit the form without changing any input/select from the form
=> Now you only see the "members" logs.
Test plan:
- Apply the patch
- Click "Modification log"
- Notice that the url now contains "modules=MEMBERS&modules=CIRCULATION"
- Submit the form without changing any input/select from the form
=> Now you have the "members" and "circulation" modules selected.
Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
See C4::Circulation::SendCirculationAlert
3355 my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_NO_TABLE_LOCKS};
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Some shells may not pass the program name in underscore, and cron also
does not like it here:
Use of uninitialized value $ENV{"_"} in pattern match (m//) at Koha/Database.pm line 79.
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: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch will turn the strict SQL modes on When tests are ran with prove.
Test plan:
- Apply the first patch
- prove t/db_dependent/Koha/Database.t
=> Will pass if you have <strict_sql_modes>1</strict_sql_modes> in your
koha-conf.xml
=> Will fail otherwise
- Apply the second patch
- prove t/db_dependent/Koha/Database.t
=> Will pass whatever the value of strict_sql_modes in your
koha-conf.xml
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The cardnumber can contain strange non-javascript-friendly
characters. This patch converts all the internal references
to borrowernumber, but displays the cardnumber when showing
list of matching users.
TEST PLAN
---------
1) Make sure Course Reserves is being used. Check system preferences.
2) Make sure there is a Course entered.
3) Make sure there is a patron with an cardnumber which breaks
the existing javascript (eg. Ka-boom:Ka-boom!KABOOM@#$!)
4) Go into the course list.
5) Click the course name link
6) In the "Instructor Search" box enter the patron name.
7) Select the patron which has the ugly cardnumber.
8) Click Save
-- The course listing should have the new teacher listed.
9) Click the course name link again.
10) Click 'Remove' beside the instructor's name.
-- nothing happens, and inspecting the error console gives
you the ugly error described in comment #0
11) apply this patch
12) restart_all
13) sudo service apache2 restart
14) refresh the page.
15) click 'Remove' beside the instructor's name.
-- name is removed.
16) Click Save
-- the course listing no longer lists the new teacher.
17) Actually confirm that adding and removing others normally
works as expected.
18) run koha qa test tools.
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch addresses a few more instances of this problem which I didn't
catch in my first pass.
To test, apply the patch and clear your browser cache if necessary.
- Go to Circulation -> Check in and check in an item which has a hold.
- When the 'Hold found' modal appears the checkin barcode field
behind it should be disabled.
- When the modal is dismissed the checkin field should be enabled
again and focus should be on that field.
- Go to Administration -> Library groups.
- Click the "Add group" button. The cursor focus should be on the
title field.
- Go to Tools -> MARC modification templates.
- Click the "New template" button. The cursor focus should be on the
name field.
Signed-off-by: Cori Lynn Arnold <carnold@dgiinc.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Modals in the staff client which load data via AJAX require an "on
close" action in order to clear the modal of the loaded data. The
problem presents itself the second time the modal is opened: The
"loading" message doesn't appear, and the data from the previous link
is shown until the new data loads.
To test, apply the patch and test the following pages. In each case,
click multiple instances of the modal trigger link/button to confirm
that the "loading" message always appears between re-displays of the
modal.
- Acquisitions -> Vendor -> Add to basket -> From a staged file -> Add orders
- Test the "MARC" and "Card" links
- Acquisitions -> EDIFACT messages
- Test the "View message" button
- Acquisitions -> Vendor -> Add to basket -> From an existing record
- Test "View MARC" link
- Acquisitions -> Vendor -> Invoices -> Invoice -> Receipt page
- Test the "Order," "MARC," and "Card" links.
- Catalog -> Bibliographic record details
- Test the MARC Preview link.
- Cataloging -> Cataloging search results
- Test the Actions -> MARC preview and Actions -> Card preview menu
items
- Cataloging -> Cataloging search results -> Merge
- Test the "View MARC" linkscataloguing/merge.tt
- Tools -> Patron lists -> Actions -> Print patron cards
- Tools -> Batch record modification -> Results -> Show MARC
- Tools -> Staged MARC record management -> View batch
- Test MARC preview shown when you click a staged title
Signed-off-by: Cori Lynn Arnold <carnold@dgiinc.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Fix:
'koha_kohadev.letter.name' isn't in GROUP BY
DBD::mysql::db selectall_arrayref failed: 'koha_kohadev.letter.name' isn't in GROUP BY [for Statement "SELECT branchcode, module, code, name, branchname
FROM letter
LEFT OUTER JOIN branches USING (branchcode)
GROUP BY branchcode,module,code ORDER BY module, code, branchcode"] at /home/vagrant/kohaclone/tools/letter.pl line 383.
Test plan:
- Copy some default notice templates for specific libraries
- List the different notice templates
- Confirm that notices appear only one, with "all libraries" you will see all
Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We must not encode & again and use the url filter, instead of uri.
To test:
- Search for a record in staff
- Go to holds
- Search for patrons, multiple should be displayed
- Pick a patron by clicking on the name
- Verify an error is shown and the URL is incorrect
- Apply patch
- Repeat test above, placing a hold should now work.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Based on the feedback of Jonathan, Katrin, Mark and Martin (thx), adding
the general remark that if lockout is enabled, an account may be locked
out after x attempts. This does not say that the account on hand is
locked or not.
Adding a <p> tag between 'Log in'-button and 'Forgot your password' for
presentation.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We should not expose more information than needed when someone tries
to login with invalid credentials. Saying that an account is locked
reveals that the account exists (or perhaps an email address).
Trivial fix. Keeping the var too_many_login_attempts for staff.
Note: We do not remove this distinction for the staff client here (in the
assumption that a library may well have additional security measures in
place for staff client). But it could be done too (on another report).
Test plan:
Enable lockout feature.
Enter invalid credentials until account locks out (on OPAC !!)
Note that message does no longer change to 'Account is locked'.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch substitutes C4::Accounts->getnextacctno for
C4::Accounts::getnextacctno since getnextacctno is only expecting to be
passed a borrowernumber
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch makes the same change as the main patch, just for an
additional occurance of getnextacctno
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
1. Fetch and checkout the master branch
2. Checkout an item with a rental charge associated with it to a user
and notice an error is thrown
3. Apply patch
4. Restart memcached and plack
5. Repeat steps 1 and 2 and notice the checkout happens successfully
Sponsored-By: Catalyst IT
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To Test:
1 - Find a record
2 - Place several holds some with differing pickup locations
3 - Check in one hold to confirm as waiting
4 - Check another in to set a hold in transit
5 - View the holds queue for the record:
/cgi-bin/koha/reserve/request.pl?biblionumber=##
6 - Confirm found holds are incorrect
7 - Apply patch
8 - Reload
9 - Confirm holds look correct
10 - Alter the values of HoldsSplitQueue and HoldsSplitQueueNumbering
11 - Confirm things still look right
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch removes the 'frameworkcode' template parameter. Now the
'framework' object is passed, and everytime the frameworkcode is needed
on the template, framework.frameworkcode is called.
To test:
- Apply this patch
- Run:
$ grep ' frameworkcode ' koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt
=> SUCCESS: There's no use of 'frameworkcode' in the template
- Sign off :-D
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch removes URL building from the controller
(admin/marctagstructure.pl) and does it on the templates, using the
right 'uri' filter for the query parameters.
It also removes:
- No longer used attributes that are passed to the template
- Duplicated code in the templates
To test:
- Go to Home > Administration > MARC bibliographic framework
- On any of the listed frameworks, pick the 'MARC structure' action in
the dropdown
- On any subfield, hover on the actions dropdown
=> FAIL: The URLs are wrongly formed
- Click on the links
=> FAIL: You get errors because of the bad URL
- Apply this patch
- Reload the 'MARC structure' page
- On any subfield, hover on the actions dropdown
=> SUCCESS: LInks look correct
- Click on any of them
=> SUCCESS: They work!
- Use the 'Display only used tags/subfields' checkbox
=> SUCCESS: No behaviour change!
- Sign off :-D
Sponsored-by: Theke Solutions
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch modifies several tools templates to use the
Bootstrap grid instead of YUI.
This patch also removes obsolete "text/javascript" attributes from
<script> tags and "text/css" attributes from <style> tags in the
modified templates.
To test, apply the patch and view the following pages, confirming that
they look correct at various browser widths:
- Tools -> MARC modification templates
- View and edit templates
- Tools -> Batch patron modification
- Test each step of the process
- Tools -> Overdue notice/status triggers
- Tools -> Upload patron images
- Test each step of the upload process
- Tools -> Quote editor
-> Quote uploader
- Import quotes
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This follow-up corrects CSS which was dependent on YUI-specific markup
context.
To test, apply the patch, regenerate the staff client CSS, go to Tools
-> Calendar.
- Click any date on the calendar
- In the entry form the checkbox and radio button options should be
styled correctly.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch modifies several tools templates to use the
Bootstrap grid instead of YUI.
This patch also removes obsolete "text/javascript" attributes from
<script> tags and "text/css" attributes from <style> tags in the
modified templates.
To test, apply the patch and view the following pages, confirming that
they look correct at various browser widths:
- Tools -> Export data
- Tools -> Calendar
- Tools -> Import patrons
- Tools -> Inventory
- Tools -> Staged MARC management
- View batches and batch details
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds two missing template filters as pointed out by the
template test.
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>
This patch adds a new collection column to the item table when selecting
a specific item for a hold. The column will only appear if at least
one item has a collection set.
To test, in OPAC:
- Place a specific hold on
- a record with one or more items with collections
- a record with one or more items without collections
- Verify the collections display correctly when they exist
- Verify the table still works as expected
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>
This patch adds a new collection column to the item table when selecting
a specific item for a hold. The column will only appear if at least
one item has a collection set.
To test, in staff:
- Place a specific hold on
- a record with one or more items with collections
- a record with one or more items without collections
- Verify the collections display correctly when they exist
- Verify the table still works as expected
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>
The column with serial enumeration/chronology information is moved
from the back to the right of itemcallnumber on the intranet detail
page.
To test:
- Apply patch
- Look at different detail pages in staff:
- 1 or more items have serial enumeration information
- no item has serial enumeration informatoin
- Verify
- itemcallnumber is now followed by serial information, if
an item with the field filled in exists for the record
- all other functionality, like sorting, works as expected
https://bugs.koha-community.org/show_bug.cgi?id=20896
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>
Changes were made to address QA issues from Comment 17
Tests:
1) Verify that a new entry for CONTROL_NUM_SEQUENCE is added to table authorised_value_categories
2) Edit a bib record using advanced editor and note that the 001 widget isn't there when adding/editing 001 field
3) Add a new row into authorised_values table with:
a) CONTROL_NUM_SEQUENCE in category column
b) authorised_value column has a string ending with a number i.e. sprLib0001
c) lib column has a short string indicating the type of control number i.e. "Springfield Library"
4) Edit a bib record using the advanced editor and note that the 001 widget is there when adding / editing 001 field
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Tests:
1) Verify that a new entry for CONTROL_NUM_SEQUENCE is added to table
authorised_value_categories
2) Edit a bib record using advanced editor and note that 001 widget
isn't there when adding/editing 001 field
3) Add a new row into authorised_values table with:
a) CONTROL_NUM_SEQUENCE in category column
b) authorised_value column has a string ending with a number i.e.
"faw0001"
c) lib column has a short string indicating type of control number
i.e. "FAW"
4) Edit a bib record using the advanced editor and note that the 001
widget is there when adding / editing 001 field
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
In advanced editor mode for cataloging, the 001 widget will
automatically assign the next control number if a format is defined
in authorised_values table.
To test you need access to the database.
1) Verify that a new entry for CONTROL_NUM_SEQUENCE is added to
table authorised_value_categories
2) Enter a new value for authorised_values with the authorised_value
column ending in a number
3) Edit a record using the advanced editor, specifically the 001
field
4) Note the new widget and option to increment or assign manually.
The buttons fit within the widget now, and there's a database update
to create the CONTROL_NUM_SEQUENCE.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To test:
1 - Define a new authorised valued category "CONTROL_NUM_SEQUENCE"
2 - Add a value/sequence
The authorised_value is the starting value - shoudl end in a number
that can be incremented e.g. "control_sequence_001"
The description field is the name for the seqeuence
Opac description is unused
3 - Edit a record in rancor
4 - Note the new widget and option to increment or assign manually
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>
