Aleisha Amohia [Wed, 24 Aug 2016 00:16:43 +0000 (00:16 +0000)]
Bug 17175: Typo in patron card images error message
To test:
1) Go to Tools -> Patron Card Creator -> New Image
2) Click Upload without attaching anything
3) Notice typo
4) Apply patch and refresh page (resend information if prompted)
5) Notice typo fixed
Sponsored-by: Catalyst IT Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Andreas Roussos [Wed, 24 Aug 2016 14:10:29 +0000 (17:10 +0300)]
Bug 17185: Staff client shows "Lists that include this title:" even if item is not in a list
In the staff client, the text "Lists that include this title:"
is always shown, regardless of whether the item is in a list
or not. This patch fixes that.
Test plan:
1) Log in to staff client.
2) Go to biblio details view:
/cgi-bin/koha/catalogue/detail.pl?biblionumber=X
Confirm that "Lists that include this title:" is
shown even if the item is not in a list.
3) Apply the patch.
4) Repeat step 2. Confirm that the patch works, i.e.
"Lists that include this title:" is only shown
for biblios that are actually in a list.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Only applies to non-XSLT view.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Marcel de Rooy [Wed, 17 Aug 2016 13:14:12 +0000 (15:14 +0200)]
Bug 16809: Follow-up for scalarizing biblionumber
Still resolves another multi_param warning.
Test plan:
Look at your logs before and after this patch when saving a biblio
record (you may have to start plack again).
If your biblionumber is mapped to 999c, you should no longer have a warn
about line 2563 (disclaimer: line numbers are subject to change).
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: Kyle M Hall <kyle@bywatersolutions.com>
Most of the floodiness is caused by accessing the cgi parameters
in a context which is hard to determine. By purposefully saving
the value to a scalar variable and using the variable, the issue
disappears, and it will likely be a tiny tad faster as variable
access is faster than multiple function calls.
TEST PLAN
---------
1) Back up your intranet error log
-- for example:
cp ~/koha-dev/var/log/koha-error-log ~/koha-error-log.backup
2) Blank your intranet error log
-- for example:
echo > ~/koha-dev/var/log/koha-error-log
3) Log into your staff client
4) Click 'Authorities'
5) Click 'New from Z39.50'
5) Type 'Seuss' into 'Name (any):' and press enter
6) Click 'Import' beside the first link
7) Click 'Save'
8) Check your koha-error-log
-- floody!
9) Apply patch
10) repeat steps 2-8
-- blank!
11) restore your intranet error log
-- for example:
mv ~/koha-error-log.backup ~/koha-dev/var/log/koha-error-log
12) run koha qa test tools
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with addbiblio.pl. I would have preferred the scalar option in terms
of simpler code, but this works too.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Owen Leonard [Thu, 18 Aug 2016 17:08:33 +0000 (13:08 -0400)]
Bug 14612 - Overdue notice triggers should show branchname instead of branchcode
This patch adds the Branches template plugin to the overdue notice
triggers template so that the library name can be shown instead of the
branchcode.
Also changed: Updated page title to match the name used in tools menus.
To test, apply the patch and go to Tools -> Overdue notice/status
triggers.
- Select a library.
- When the page reloads, the 'Defining overdue actions for...' and
'Rules for overdue actions: ' headings should show the library name
instead of the branchcode.
Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Owen Leonard [Tue, 9 Aug 2016 13:49:33 +0000 (09:49 -0400)]
Bug 16464 - If a patron has been discharged, show a message in the OPAC
This patch adds a message to the patron home page in the OPAC to be
shown if the user is restricted because of a discharge.
To test, apply the patch and log into the OPAC as a patron who has been
discharged.
- You should see a message which says so, including a link to
the discharge notice.
- Log in to the OPAC as a patron with a manual restriction and confirm
that the correct notice is displayed.
Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Colin Campbell [Wed, 17 Aug 2016 16:30:55 +0000 (17:30 +0100)]
Bug 17141: Call config method to retrieve logdir
Incorrect method call is causing runtime error and not
retrieving the correct logdir value
Change retrieves the value correctly
To test:
1) Run edi_cron.pl, notice error
2) Apply patch and run edi_cron.pl again, should work as expected
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Note: I did not test but changes make sense.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Magnus Enger [Wed, 31 Aug 2016 08:47:43 +0000 (10:47 +0200)]
Bug 17228 - Make two versions of SIPconfig.xml identical
Several bugs have made changes to etc/SIPconfig.xml but not
updated debian/templates/SIPconfig.xml. This means that an admin
using the Debian packages who enables SIP2 for a site and looks at
/etc/koha/sites/<instance>/SIPconfig.xml will not see an up-to-date
version of that file, with the risk of missing possible config
opportunities.
Since debian/templates/SIPconfig.xml contains no placeholders or
other magic stuff related to the Debian packaging, this patch simply
copies etc/SIPconfig.xml to debian/templates/SIPconfig.xml
To test: diff etc/SIPconfig.xml debian/templates/SIPconfig.xml
There should be no difference between the files
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Magnus Enger [Wed, 31 Aug 2016 08:45:32 +0000 (10:45 +0200)]
Bug 17228 - Fix whitespace in etc/SIPconfig.xml
Remove trailing whitespace and replace tabs with 4 spaces.
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Holger Meißner [Thu, 11 Aug 2016 09:22:00 +0000 (11:22 +0200)]
Bug 14434: Display "Not renewable (on hold)" in OPAC
This patch makes the OPAC display "Not renewable (on hold)" when
a hold is placed.
Test plan:
1) Do not apply patch.
2) Issue an item with automatic renewal.
3) Issue an item with manual renewal.
4) Place a hold on both items.
5) Log in as patron and note that the column "Renew" says "Automatic
renewal (x of y renewals remaining)" for the auto renewed item
and "(On hold) for the other item.
6) Apply patch.
7) Refresh OPAC and note that now "Not renewable (on hold)" is displayed
for both items.
8) Cancel the holds, then log in as patron again and confirm that the
correct renewal conditions are displayed.
Sponsored-by: Hochschule für Gesundheit (hsg), Germany Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 19 Aug 2016 11:43:24 +0000 (12:43 +0100)]
Bug 16990: Display branch names instead of code in patron mod requests
To test:
- change your homebranch in the OPAC, submit
- change patron modification request in the staff client
- Verify that it shows the old and new branch name instead of the
code
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Mon, 15 Aug 2016 14:45:38 +0000 (15:45 +0100)]
Bug 17128: Make summary-print.pl plack safe
$borrowernumber is used in build_issue_data but not correctly defined
(Variable "$borrowernumber" is not available)
That may cause wrong charge displayed in the summary slip.
Test plan:
- Set rental charge for an item type
- Define a rental discount for that item type in the circ rules
- check in an item matching this rule
Without this patch the charge displayed in the summary slip won't be
calculated with the discount
With this patch applied, the warning in the logs will no longer appear
and the values will be correctly calculated.
Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Sat, 9 Jul 2016 12:13:35 +0000 (13:13 +0100)]
Bug 16886: Make the 'Upload patron images' tool plack safe
Some vars are accessed from subroutine, but defined with my.
It causes at least the 2 followings errors:
Variable "$filetype" is not available at
/home/koha/src/tools/picture-upload.pl line 240.
Variable "$uploadfilename" is not available at
/home/koha/src/tools/picture-upload.pl line 241.
To avoid that, they are now declared with our.
Test plan:
Upload image for a patron and confirm that you get a "Result" table and
the errors do not longer appear in the logs.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Tue, 16 Aug 2016 08:18:19 +0000 (09:18 +0100)]
Bug 7045: Update default value placeholders for existing installs
Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Tue, 16 Aug 2016 08:09:53 +0000 (09:09 +0100)]
Bug 7045: Use <<tag>> for default value placeholders
When adding a biblio, the default value of a MARC subfield defined in
the frameworks can be used as placeholders to set the current date or
the surname of the logged in user (use cases?).
The different placeholders are 'YYYY', 'MM', 'DD', 'user'.
When adding an item, same behavior except that 'user' is not replaced.
This patch makes behaviors consistent between the 2 editors and
surrounds placeholders with << >>
We will now have: <<YYYY>>, <<MM>>, <<DD>> and <<USER>>
Test plan:
Define default values for biblio and item subfields.
Create a bibliographic record and attach it an item.
The default values should be used and replaced if you used placeholders.
Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jacek Ablewicz [Wed, 17 Aug 2016 16:42:37 +0000 (18:42 +0200)]
Bug 17142 - Don't show library group selection in advanced search if groups are not defined
Even if library groups/search domains are not defined in the system,
(empty) select for "Groups of libraries" under "Location and
availability" section in OPAC advanced search is still visible; it
shouldn't. Side effect of Bug 15295 - 'searchdomainloop' variable
in the template is now an object, it needs a different kind of statement
(.count) for checking if it's empty or not.
To test:
1) apply patch
2) when there are no groups of libraries defined in Administration ->
Libraries and Groups, "Groups of libraries" selection should be
no longer visible in OPAC adavanced search page
3) add library group or two: "Groups of libraries" selection should
reappear.
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Owen Leonard [Thu, 25 Aug 2016 18:06:11 +0000 (14:06 -0400)]
Bug 17200 - Badly formatted "hold for" patron name on catalog detail page
This patch adds a space between first name and surname on the
bibliographic detail page when there is "hold for" information in the
status column of the holdings table.
To test, apply the patch locate a title in the staff client catalog
which has one or more confirmed holds on it. Verify that the patron's
name in the "status" column of the holdings table looks correct, with a
space between first and last name.
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Owen Leonard [Thu, 18 Aug 2016 15:58:42 +0000 (11:58 -0400)]
Bug 11019 - Require some fields when adding authorized value category
This patch modifies the form for adding an authorized value so that
the category is a required fields.
Previously a new authorized value category could be saved with no data.
To test, apply the patch and go to Administration -> Authorized values.
- Click the "New category" button.
- Click the save button without filling in the category.
You should be prevented from submitting the form.
- Verify that filling in the required field allows the form to be
submitted.
- Perform the same test when editing an existing authorized value.
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Follow-up for QA: Allow a blank authorised value to be created.
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Amended test plan.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Owen Leonard [Fri, 19 Aug 2016 14:08:38 +0000 (10:08 -0400)]
Bug 13921 - XSLT Literary Formats Not Showing
This patch adds some missing literary formats to the staff client and
OPAC's search results XSLT display.
To test you must have DisplayOPACiconsXSLT and DisplayIconsXSLT system
preferences enabled. XSLTResultsDisplay and OPACXSLTResultsDisplay
should be set to 'default.'
Perform searches in the staff client and the OPAC and confirm that the
following literary forms (defined in 008 position 33) display correctly:
Not fiction; Fiction; Dramas; Essays; Novels; Humor, satires, etc.;
Letters; Short stories; Mixed forms; Poetry; Speeches.
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Mon, 22 Aug 2016 12:48:59 +0000 (13:48 +0100)]
Bug 17157: Same for "More"
Here I decided to redirect to the mainpage.
Works as dexcribed. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Mon, 22 Aug 2016 12:36:39 +0000 (13:36 +0100)]
Bug 17157: Fix middle click on "Search" drowndown menu
If you save an item and right click on "Search" or "More" you will get
a software error:
Can't call method "fields" on an undefined value at
/usr/share/koha/intranet/cgi-bin/cataloguing/additem.pl line 742
You will now be redirected to the adv search form.
Wors as described. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Owen Leonard [Tue, 12 Jul 2016 13:24:59 +0000 (09:24 -0400)]
Bug 16903 - Multiple class attributes on catalog search tab
cat-search.inc contains an element with two class attributes, which is
invalid. This patch corrects it.
To test, apply the patch and view any page which uses the cat-search
include file to display the header search boxes. For instance, the
bibliographic detail page.
In the header, the "Search the catalog" box should be selected, and any
text you type in the box should be carried over when you switch to
different search tabs.
Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 19 Aug 2016 15:09:55 +0000 (16:09 +0100)]
Bug 17118: (follow-up 15381) Fix regression when clearing a linked authority
Changes made on bug 15381 assumed that the authid was always set.
But if the user wants to clear the field of the authority, the script is
called with authid=0 (FIXME...)
To fix this issue, it's better to move the new calls to
Koha::Authorities->find($authid)->authtypecode
and
Koha::Authority::Types->find($authtypecode);
at the correct place
Test plan:
1. Edit a record which has a field linked to an
authority record (100a for instance).
2. Click the link which triggers the tag editor.
A pop-up window should be displayed.
3. In the pop-up window, click the "Clear field"
button.
=> Without this patch A second pop-up window opens and displays an error:
Can't call method "authtypecode" on an undefined value at /authorities/blinddetail-biblio-search.pl line 61.
=> With this patch applied the field is correctly cleared.
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 26 Aug 2016 12:33:32 +0000 (12:33 +0000)]
Bug 17201: (bug 16431 follow-up2) Remove occurrence of marcfromkohafield
I really don't know how these tests passed before 16431, the mapping was not complete.
Test plan:
prove t/db_dependent/Search.t
should return green
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 26 Aug 2016 12:32:00 +0000 (13:32 +0100)]
Bug 17201: (bug 16431 follow-up) Remove occurrence of marcfromkohafield
I am not sure this code is called so I don't know how to test it.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Thu, 5 May 2016 14:25:50 +0000 (15:25 +0100)]
Bug 16449: Remove "no method selected" warning from unimarc_field_4XX
Because of bug 14828, the unimarc_field_4XX now raises a warning:
"No method selected!"
There are no need to select an item type on this page, the default (all)
is always selected.
Test plan:
Link the unimarc_field_4XX value builder with on of the subfield
Edit a record, click on the value builder icon
Note the warning without the patch and that it's gone with the patch
applied
NOTE: Code fix that I derived was identical. Ran
prove t/db_dependent/FrameworkPlugin.t
to confirm it works.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Sun, 3 Jul 2016 14:32:47 +0000 (15:32 +0100)]
Bug 16686: Fix "Item in transit from since" in Holds tab
POC, tests needed.
Alternative patch works for me. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Marc <veron@veron.ch> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Kyle M Hall [Fri, 24 Jun 2016 15:18:04 +0000 (15:18 +0000)]
Bug 11144 [QA Followup] - Let each script run in sequence even if one fails
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested the A;B;C variant here. If A fails, B will run. Since we can safely
assume that A (or B) will not fail on a daily basis, this seems to be better
than running them in the wrong order every day.
As the comments on Bugzilla show, several people support this improved
(reordered) scheme and look forward to improved error handling on another
report (obviously not that simple).
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Katrin Fischer [Tue, 9 Jun 2015 21:13:15 +0000 (23:13 +0200)]
Bug 11144: Fix sequence of cronjobs: automatic renewal - fines - overdue notices
The patch changes the sequence of cronjobs in the crontab example
file and in the cron.daily file of the packages.
This is why:
1) Renew automatically
... only when we can't renew, we want to
2) Calculate fines
... once the fine are calculated and charged
we can print the amount into the
3) Overdue notices
Before the change it could happen that you'd charge for an item,
that would then be renewed. Or that you'd try to print fine
amounts into the overdue notices, when they would only be
charged moments later.
To test:
- configure your system so you have items that should
- be charged with fines
- renew automatically
- configure your crontabs according to the example file
or switch the cron.daily in your package installation with
the new one
- configure your overdue notices so that one should be generated
<<items.fine>>
- Wait for the cronjobs or schedule them to run earlier
- Verify all is well and as it should be
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 19 Aug 2016 13:36:23 +0000 (14:36 +0100)]
Bug 17048: Fix pagination offset for authority searches
At the intranet, the pagination has been broken by bug 12478 (Elastic
Search). There was a confusion between the offset and the page number.
At the OPAC, it is broken since bug 2735 which has mimicked the intranet
script.
Test plan:
Search for a term which will return more than 1 page of results.
Click on the second page
=> Without this patch, the first result of the second page is the second
result of the first page
=> With this patch applied, the offset will be corrected
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 12 Aug 2016 10:36:06 +0000 (11:36 +0100)]
Bug 17116: Fix CSRF in import_borrowers.pl
If an attacker can get an authenticated Koha user to visit their page
with the url below, they can change patrons' information
The exploit can be simulated triggering
/tools/import_borrowers.pl?uploadborrowers=42
In that case it won't do anything wrong, but it you POST a valid file,
it could.
Test plan:
Trigger the url above
=> Without this patch, you will the result page
=> With this patch, you will get the "Wrong CSRF token" error.
Regression test:
Import a valid file from the import patron form, everything should go
fine.
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To make the QA scripts happy, the POD needed a fix, and also
keys applied to a hashref needs to be avoided.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 14868: (QA followup) Change permission check order
This patch changes the permission check order because haspermission
is the smaller check, and going through the patron/user and its guaranteed
before checking if it is (say) a staff member or even a superlibrarian doesn't
seem right.
Bonus: Remove unneeded C4::Auth import in Patron.pm
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Tue, 23 Aug 2016 12:38:44 +0000 (15:38 +0300)]
Bug 14868: Display required permissions in permission error response
When user does not have required permissions to use API operation, it would be
useful to let them know which permissions he is missing. Since they are now
defined in Swagger, we can easily render them into the response.
To test:
1. Use a patron without any permissions
2. Make GET request to http://yourlib/api/v1/patrons
3. Observe permission error and see that required_permissions are displayed.
4. Run t/db_dependent/api/v1/patrons.t
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Thu, 11 Aug 2016 11:45:16 +0000 (14:45 +0300)]
Bug 14868: Use x-koha-authorization in current routes
To test:
1. Run t/db_dependent/api/v1/holds.t
2. Run t/db_dependent/api/v1/patrons.t
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Fri, 17 Jun 2016 08:43:52 +0000 (11:43 +0300)]
Bug 14868: Give users possibility to request their own object
Allow access to user's own objects even if they do not have required permissions.
This will be very useful in many cases where an user wants to request their own
object, for example renewing their checkouts or placing a hold for themselves.
First, this patch renames "x-koha-permission" to "x-koha-authorization" in order
to describe the new functionality better.
Second, we can now add two extra parameters under "x-koha-authorization":
- "allow-owner"; Allows the owner of object to access it (without permission)
- "allow-guarantor"; Allows guarantor of the owner of object to access it
(without permission)
Third, since permission checking is outside of actual controller, we need a way
to find out ownership from different types of parameters, e.g. checkout_id from
/checkouts/{checkout_id}, borrowernumber from /patrons/{borrowernumber} etc.
A solution is to match the parameter with a subroutine that is designed to verify
the ownership for that object. See the new subroutines in Koha::REST::V1.
To use this functionality you will simply define it in Swagger:
"/patrons/{borrowernumber}": {
"get": {
...,
"x-koha-authorization": {
"allow-owner": true,
"permissions": {
"borrowers": "1"
}
}
}
}
If a parameter that is not yet defined in Koha::REST::V1::check_object_ownership,
you also need to define it and implement a subroutine that determines ownership.
Tests are provided in a following patch that adds this functionality for current
API operations.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Define 'x-koha-permission' for the Swagger2 Operation Object, to automatically
authorize against the required permissions.
This way we immediately tell the API consumer in the Swagger2-definition, which
permissions are needed to access defined resources.
Also we don't need to maintain permissions in multiple locations and we can build
a smart testing framework to help a lot in creating tests for the new REST API.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Kyle M Hall [Tue, 26 Jul 2016 15:00:58 +0000 (15:00 +0000)]
Bug 16942 [QA Followup] - Add unit test
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Kyle M Hall [Tue, 19 Jul 2016 16:57:25 +0000 (16:57 +0000)]
Bug 16942 - Confirm hold results in ugly error
Confirming a hold to set it to waiting will result in an DBIC error in
master.
Test Plan:
1) Attempt to check in an item on hold and confirm the hold
2) Note the error
3) Apply this patch
4) Repeat step 1
5) Note there is no error!
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 16699: (QA followup) Move minified swagger file into the swagger/ dir
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Thu, 11 Aug 2016 12:51:38 +0000 (15:51 +0300)]
Bug 16699: Remove requirement from borrowernumberQueryParam
borrowernumberQueryParam shouldn't be required as also changed in Bug 16271.
To test:
1. Don't apply the patch yet, but first minify Swagger and run
t/db_dependent/api/v1/holds.t
2. Observe that some tests fail with response code 400 when expecting 200.
3. Apply patch and minify Swagger
4. Run t/db_dependent/api/v1/holds.t
5. Observe that tests pass.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Tue, 14 Jun 2016 14:27:36 +0000 (17:27 +0300)]
Bug 16699: Move Swagger-related files to api/v1/swagger
This patch separates Swagger-specifications and the minifySwagger.pl from other
api-files by moving specifications & minifier into api/v1/swagger.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Tue, 14 Jun 2016 11:24:42 +0000 (14:24 +0300)]
Bug 16699: Reference new x-primitives in currently defined objects
Since we have defined some basic x-primitives in x-primitives.json, we can now
start to reuse them in our currently defined objects.
To test:
1. Apply patch
2. Run minifySwagger.pl
3. Validate your Swagger specifications
4. Observe that validation passes
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Tue, 14 Jun 2016 09:53:58 +0000 (12:53 +0300)]
Bug 16699: Support multiple types in primitive definitions
Currently it is not possible to define multiple types for primitive definitions
in /definitions/*. If you try to use the following
"firstname": {
"type": ["string", "null"],
"description": "patron's first name"
}
in definitions.json, online.swagger.io validator will not validate it:
{"messages":["attribute definitions.firstname.type is not of type `string`"]}
One way to get around this issue is to extend definitions with custom
"x-primitives" object, where we will define all reusable primitive definitions.
To test:
1. Add the "firstname" example above to definitions.json
2. Run minifySwagger.pl
3. Validate your specification
4. Observe that error with description mentioned above is given
5. Apply patch
6. Repeat step 2 and 3
7. Observe that validation passes
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Fri, 10 Jun 2016 14:25:40 +0000 (17:25 +0300)]
Bug 16699: Fix mixed-up indentation from 2-4 spaces to 2 spaces
These definitions had indentation of 4 spaces, while rest of the specification
uses 2 spaces. This patch simply maintains the consistency in indentations and
provides no other modifications to code.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Fri, 10 Jun 2016 13:52:54 +0000 (16:52 +0300)]
Bug 16699: Add borrowernumberQueryParam for reusability
The borrowernumber as a query parameter should be defined in parameters.json
in order to allow its reusability.
To test:
1. Apply patch
2. Run minifySwagger.pl
3. Validate swagger.min.json in online.swagger.io/validator/debug?url=url_to+
_your_swagger_min_json or your local swagger-api/validator-badge validator
4. Observe that validation passes
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Thu, 9 Jun 2016 13:13:53 +0000 (16:13 +0300)]
Bug 16699: Split parameters and paths in Swagger
Parameters and paths should be split in our Swagger specification, because
otherwise swagger.json would become messy with all the paths and their
further specification in the same file. Also parameters should be split
for the same reason.
Instead of using index.json for definitions, parameters and paths, we define
new files "definitions.json", "parameters.json" and "paths.json" in order to
simplify the references. If we kept using index.json and try to reference
"/definitions/error.json" from "/paths/holds.json", reference would be
"../definitions/index.json#/error" instead of now simplified version,
"../definitions.json#/error".
The swagger.json paths, definitions and parameters will look as follows:
...
"paths": {
"$ref": "paths.json"
},
"definitions": {
"$ref": "definitions.json"
},
"parameters": {
"$ref": "parameters.json"
}
...
A problem with splitting specification into multiple files directly from
swagger.json (e.g. "paths": { "$ref": "paths.json" }) is that it is not
following the Swagger specification and an error will be thrown by the
Swagger-UI default validator (online.swagger.io/validator).
To overcome this problem, we use the minifySwagger.pl script from Buug 16212.
This allows the developers to work with the structure introduced in this patch
thus allowing developers to split the specification nicely, and still have a
valid Swagger specification in the minified swagger.min.json.
To test:
-2: Apply the minifier-patch in Buug 16212.
-1: Make sure you can validate your specification with Swagger2 validator at
online.swagger.io/validator/debug?url=url_to_swaggerjson, or install it
locally from https://github.com/swagger-api/validator-badge.
1. Don't apply this patch yet, but first validate swagger.json
with swagger.io-validator (or your local version, if you installed it)
2. Observe that validation errors are given
3. Run minifySwagger.pl
4. Validate swagger.min.json with the validator you used in step 1
5. Observe that validation passes and we overcame the invalid specification
problem in swagger.min.json
6. Apply this patch
7. Run minifySwagger.pl
8. Repeat step 4
9. Observe that validation passes with new structure
10. Run REST tests at t/db_dependents/api/v1
(11. Study the new structure of our Swagger specifications :))
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch re-introduces the transaction into t/db_dependent/Items_DelItemCheck.t
and does some cleaning on the touched tests so they raise less warnings.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
* Fix POD warning.
* Remove redundant 'use stric' and 'use warnings'
* Remove $VERSION and --version option.
* Remove --dry-run option
* Split test for --help and check for @criteria into two separate pod2usage calls,
enabling -msg on the latter.
* Fix 'target_tiems' typo.
* Test for holds on items to be deleted.
* Fix whitespace
* Fix test for holds.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Heather Braum <hbraum@nekls.org> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lari Taskula [Tue, 14 Jun 2016 13:01:24 +0000 (16:01 +0300)]
Bug 16212: Use swagger.min.json insted of swagger.json
Our current approach with splitting the Swagger specification brings problems.
For example, in swagger.json, if we split the whole "definitions" object with
$ref, validations will fail with online.swagger.io validator.
(See http://online.swagger.io/validator/debug?url=url_to_your_swagger_json)
The problem also occurs with "paths" (Paths Object), because simply $ref to all
paths for example in /paths/index.json does not meet the Swagger2 specification.
The problem is solved by using the minification script and the minified version
of swagger.json after which the swagger.min.json is valid Swagger2 specification,
because the minifier resolves the problematic $refs of swagger.json file in the
minified version.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Julian Maurice [Wed, 6 Apr 2016 11:16:50 +0000 (13:16 +0200)]
Bug 16212: Add script minifySwagger.pl
This patch introduces the misc/devel/minifySwagger.pl script that
loads the Swagger files, follows references and produces a compact
("minified") version of the swagger file which is suitable for
distribution.
The wiki page should be updated with instructions on how to regenerate
it so the Release Manager does it on each spec upgrade.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Marcel de Rooy [Tue, 23 Aug 2016 14:33:10 +0000 (16:33 +0200)]
Bug 14390: [Follow-up] Only update FU record in UpdateFine
Exclude O, F and M when outstanding == 0.
Check if the issue_id points to a FU record.
Note: We only warn now when we see a second FU record with this issue id.
That should be a rare exception. As before, we are just counting it in
our total. Added a FIXME.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested fine on overdue. Renewed and backdated for a second fine. The F
and FU can be seen on the Fines tab and are totaled on Check out.
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Marcel de Rooy [Fri, 19 Aug 2016 07:57:13 +0000 (09:57 +0200)]
Bug 14390: [QA Follow-up] UpdateFine should be passed a hash
Renewing an overdue would not work.
Log shows:
renew: Can't use string ("2144746608") as a HASH ref while "strict refs" in use at C4/Overdues.pm line 508., referer: /cgi-bin/koha/circ/circulation.pl?borrowernumber=1
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Kyle M Hall [Fri, 6 Nov 2015 18:20:56 +0000 (13:20 -0500)]
Bug 14390 - Fine not updated from 'FU' to 'F' on renewal
Test Plan:
1) Find an overdue checkout with a fine
2) Renew item, note fine is not closed out (Account type F)
3) Apply this patch
4) Find another overdue checkout with a fine
5) Renew item, note fine is now correctly closed out
6) Backdate a checkout to be already overdue ( but not have a fine since
fines.pl hasn't run yet )
7) Renew item, note a closed out fine is created
Signed-off-by: Sean Minkel <sminkel@rcplib.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
By setting 'use Modern::Perl' some previously hidden warnings arised.
This patch removes them by testing the variable $balance for undef
before using it in a comparisson.
Sponsored-by: VOKAL Signed-off-by: Liz Rea <liz@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 15023: Allow patron anonymize/bulk delete tool to be limited by branch
This patch makes the bulk patron delete/anonymize functionality be limited
by branch. It does so by adding a branch selection dropdown and using the
already defined APIs for filtering by branch.
It makes use of C4::Branches::onlymine for the IndependentBranches use case
and it adds a way to call it from the Branches template plugin.
To test:
- Apply the patch
1) Have a superlibrarian user
- Go to Tools > Batch patron deletion/anonymization
=> SUCCESS: Verify you can pick a branch (or all of them)
- Try doing some operations
=> SUCCESS: Verify the selection is respected, and carried around all steps
2) Have a user with tools/delete_anonymize_patrons permissions
- Set IndependentBranches on
- Go to Tools > Batch patron deletion/anonymization
=> SUCCESS: It picks the librarian's branch and doesn't let us choose another one
- Try doing some operations
=> SUCCESS: Verify the user's branch is respected, and carried around all steps
- Sign off :-D
Sponsored-by: VOKAL Signed-off-by: Liz Rea <liz@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 17050: (QA followup) Use Mojo::Transaction to get the remote address
While the original patch fixes the issue, reading at Mojolicious source code, revealed
that Mojo::Transaction already has proper processing for detecting the remote address:
sub remote_address {
my $self = shift;
return $self->original_remote_address(@_) if @_;
return $self->original_remote_address unless $self->req->reverse_proxy;
Without this patch, if there's a request without HTTP_X_FORWARDED_FOR, then the remote
address would be empty. Such would be the case of a dev/standard setup without Plack.
This patch makes Koha::REST::V1::startup use tx->remote_address instead of dealing with the
headers on its own.
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> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 5 Aug 2016 15:03:28 +0000 (15:03 +0000)]
Bug 17050: Do not kick the session out when accessing the REST API
Mojolicious does not set $ENV{REMOTE_ADDR} (neither $ENV{HTTP_*}) as
it may share ENV between different requests.
Fortunately for us, Plack does not!
This is a dirty patch to fix this issue but it seems that there is not
lot of solutions. It adds a remote_addr parameter to
C4::Auth::check_cookie_authin order to send it from
Koha::Rest::V1::startup reading the headers sent by Mojolicious.
Test plan:
Hit /cgi-bin/koha/mainpage.pl
Hit /api/v1/patrons/42
Hit /cgi-bin/koha/mainpage.pl
With this patch applied, everything will be fine and you won't be
logged out.
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> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 19 Aug 2016 12:39:35 +0000 (13:39 +0100)]
Bug 16960: Delete previous patron modifications
If a patron edit his/her details a second time, we need to delete the
first ones to avoid a "duplicate entry for key PRIMARY" error.
Test plan:
Log in at the OPAC
Edit your details
Edit them again
=> Without this patch, Koha will crash
=> With the patch applied, everything should work as expected
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Mon, 8 Aug 2016 12:46:43 +0000 (13:46 +0100)]
Bug 16960: Fix error on validating the registration
Followed steps in comment #14, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Fri, 5 Aug 2016 10:43:52 +0000 (11:43 +0100)]
Bug 16960: Update 1 missing occurrence of GetModifications
Signed-off-by: Bob Birchall <bob@calyx.net.au> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Kyle M Hall [Fri, 22 Jul 2016 16:50:50 +0000 (16:50 +0000)]
Bug 16960 - Patron::Modifications should be fixed
The changes from opac-memberentry do not reach the table, since the
Patron::Modifications object does not work well.
Test Plan:
1) Apply this patch
2) Create some patron modification requests
3) Ensure you can approve and deny modifications
4) Ensure patron self registration works
Signed-off-by: Bob Birchall <bob@calyx.net.au> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 15930 modified default value for DataTables patron search.
The doc text should also be modified :
"Can be 'contain' or 'start_with' (default value). Used for the
searchmember parameter."
Test plan :
- install Koha with patch
- look at man page man/man3/C4::Utils::DataTables::Members.3pm
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Marc Véron [Wed, 10 Aug 2016 08:12:55 +0000 (10:12 +0200)]
Bug 16741 - Remove dead code "sub itemissues" from C4/Circulation.pm
To verify:
- git grep itemissues
Result: Appears in C4/Circulation.pm only
To test:
- apply patch
- git grep itemissues
Expected result: No occurences
- prove t/db_dependent/Circulation.t
Expected result: Pass OK
Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Marc Véron [Wed, 10 Aug 2016 09:42:28 +0000 (11:42 +0200)]
Bug 17100: Do not display payments if patron has nothing to pay
This alternative patch moves logic and formatting to the template file.
To test:
* without patch
1/ find a patron with no lines in accountlines table : print summary shows no "account fines and payments" => OK
2/ find a patron with some lines in accountlines table and the total amount > 0 : print summary shows a table "account fines and payments" with fines to recover => OK
3/ find a patron with some lines in accountlines table but the total amount = 0 : print summary shows a table "account fines and payments" with nothing in it => NOK
* with the patch, same cases as before :
1/ same as without patch
2/ same as without patch
3/ print summary does not show "account fines and payments"
- Additionally, verify that formatting follows syspref 'CurrencyFormat'
- Verify that amount column is right-aligned
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 17001: fix due date filter on the overdue report
When the TimeFormat system preference is set to "12 hour",
setting a filter on the due date can result in:
- no overdue loans being reported, even if there are some
that meet the criteria
OR
- overdue loans being omitted from the report if they
are due on the "until" date in the filter
This patch fixes this by replacing output_pref() with
DateTime::Format::MySQL to format the date filter values
to pass to the SQL query.
To test
-------
[1] Run the overdue report (circ/overdue.pl) and set a filter
on due date, using values that should bring up one or
more overdue loans.
[2] Note that zero overdue loans are returned (if using MySQL
5.5, 5.6, or 5.7 or MariaDB 5) or that loans due on the
"until" date are omitted (if using MarioDB 10).
[3] Apply the patch and repeat step 1. This time, the correct
set of overdue loans should be reported.
Signed-off-by: Galen Charlton <gmcharlt@gmail.com> Signed-off-by: Jason Robb <jrobb@sekls.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Fridolin Somers [Thu, 11 Aug 2016 08:58:40 +0000 (10:58 +0200)]
Bug 17107 - Add ident and Identifier-standard to known indexes
Add ident and Identifier-standard to known indexes in C4::Search::getIndexes().
Those indexes can be very useful, for example for IdRef feature.
Test plan :
- Make sure some records have a field indexed with Identifier-standard, ISBN=1234 for example
- Perform a search /cgi-bin/koha/opac-search.pl?idx=ident,phr&q=1234
=> you find the record
- Perform a search /cgi-bin/koha/opac-search.pl?q=ident:1234
=> Without patch : you get no results
=> With patch : you find the record
Idem for 'Identifier-standard'
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Tue, 9 Aug 2016 19:08:54 +0000 (20:08 +0100)]
Bug 17095: (bug 16849 follow-up) Fix regression if patron does not exist
Bug 16082, 'Display a friendly "patron does not exist" message if that's the
case - circulation.pl' changed the behavior of the checkout page so that if
you hit that page with a borrowernumber which doesn't exist a nice message will
tell you so.
Now when you do that you get an error message:
Can't call method "is_debarred" on an undefined value at /circ/circulation.pl line 300.
Test plan:
confirm that the friendly message is back :)
Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jacek Ablewicz [Fri, 12 Aug 2016 14:19:52 +0000 (16:19 +0200)]
Bug 17117: Patron personal details not displayed unless branch update request is enabled
In patron acount ("your personal details" tab, which serves as a form
for submitting update requests as well) it's not possible to display
some account details like expiration date etc., without enabling
the field for branch / library update requests too.
To reproduce:
1) set OPACPatronDetails to "Allow"
2) clear PatronSelfModificationBorrowerUnwantedField preference
- all possible fields in the update requests form are visible
in patron account, including some extra details (card number,
expiration date, category) in the "Library" section on top
3) put 'branchcode' in PatronSelfModificationBorrowerUnwantedField preference
4) the whole 'Library' section disappears
To test:
1) apply patch
2) put 'branchcode' in PatronSelfModificationBorrowerUnwantedField,
'Library' section should remain visible (sans branch selection option)
3) put 'branchcode|cardnumber|datexpiry|categorycode' in the same
preference, 'Library' section should now disappear
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Mon, 15 Aug 2016 08:26:16 +0000 (09:26 +0100)]
Bug 17124: Fix tests in DecreaseLoanHighHolds.t
One test of DecreaseLoanHighHolds.t does not pass,
A hashref representing a patron has a 'borrower' key defined, instead of
'borrowernumber'.
Test plan:
prove t/db_dependent/DecreaseLoanHighHolds.t
must return green
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Tue, 9 Aug 2016 21:29:25 +0000 (22:29 +0100)]
Bug 17097: Fix CSRF in deletemem.pl
If an attacker can get an authenticated Koha user to visit their page
with the url below, they can delete patrons details.
/members/deletemem.pl?member=42
Test plan:
0/ Do not apply any patches
1/ Adapt and hit the url above
=> The patron will be deleted without confirmation
2/ Apply first patch
3/ Hit the url
=> you will get a confirmation page
4/ Hit /members/deletemem.pl?member=42&delete_confirmed=1
=> The patron will be deleted without confirmation
5/ Apply the second patch (this one)
6/ Hit /members/deletemem.pl?member=42&delete_confirmed=1
=> you will get a crash "Wrong CSRF token" (no need to stylish)
7/ Delete a patron from the detail page and confirm the deletion
=> you will be redirected to the patron module home page and the patron
has been deleted
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Jonathan Druart [Tue, 9 Aug 2016 21:18:14 +0000 (22:18 +0100)]
Bug 17097: Add a confirmation page when deleting a patron
It won't hurt to have a confirmation page when deleting a patron.
Moreover it's the more easy way to protect against CSRF attacks :)
Test plan:
Make sure you get a confirmation page when deleting a patron
Confirm that approving or denying the confirmation work as expected
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
projects / koha.git / log