Commit Graph

717 Commits

Author SHA1 Message Date
Nick Clemens 9a79340b8f Bug 18469: QA Follow-up
Restore datepicker class
Use Koha.Preference
Copy changes to moremember

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit a66c0b1dd7)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit c26108afa7)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
2017-10-24 16:42:07 +13:00
Amit Gupta 2489a7e7ff Bug 19079 - XSS Flaws in Membership page
1. Hit /cgi-bin/koha/members/moremember.pl?borrowernumber=xx<script>alert('amit')</script>.
   xx - is a borrowernumber
2. Notice the java script is executed.
4. Apply patch.
5. Reload page, and hit the page again /cgi-bin/koha/members/moremember.pl?borrowernumber=xx<script>alert('amit')</script>.
   xx - is a borrowernumber.
6. Notice it is no longer executed.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-08-24 17:58:27 +12:00
Aleisha Amohia 9db0e7eca8 Bug 18858: Prevent warn when deleting a borrower debarment
To test:
1) Look at intranet log
2) Go to delete a debarment on a borrower
3) Notice warn
4) Apply patch
5) Add a new debarment
6) Delete this debarment
7) Notice warn is gone

Sponsored-by: Catalyst IT

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 7ed66a6c8a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(cherry picked from commit 7d2bd04820)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
2017-07-31 01:02:09 +12:00
Jonathan Druart 8aa1e40953 Bug 17146: Fix CSRF in picture-upload.pl
If an attacker can get an authenticated Koha user to visit their page
with the
url below, they can change or delete patrons' images
/tools/picture-upload.pl?op=Delete&borrowernumber=42

Test plan:
1/ Hit /tools/picture-upload.pl?op=Delete&borrowernumber=42
And confirm that you get a "Wrong CSRF token" error
2/ Go on the patron detail page with a patron's image
3/ Click on the Delete link (note the csrf_token param)
4/ The image will be deleted and you are redirected to the patron detail
page.

Regression tests:
Upload an image from the patron detail page and from the "upload patron
images" tool.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-05-03 14:35:48 +12:00
Mason James e1294b6d91 Bug 18124: add to members/deletemem.pl 2017-05-03 14:07:02 +12:00
Marcel de Rooy cf3b67032e Bug 17097: [QA Follow-up] Exit after redirect
Adds one exit statement, and some whitespace.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified deleting a patron again.
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-05-03 14:03:52 +12:00
Jonathan Druart 08ec3cd0d0 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: Mason James <mtj@kohaaloha.com>
2017-05-03 14:01:26 +12:00
Jonathan Druart a0b7acfae1 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: Mason James <mtj@kohaaloha.com>
2017-05-03 14:01:26 +12:00
Mason James e628d013cd Bug 18124: [16.05.x] remove HouseboundModule code
modified:   members/memberentry.pl
2017-05-03 12:43:21 +12:00
Jonathan Druart c6d7fd2dd3 Bug 16993: Fix CSRF in memberentry.pl
If an attacker can get an authenticated Koha user to visit their page
with the url below, they can change patrons' passwords or other
patrons'details

members/memberentry.pl?op=save&destination=circ&borrowernumber=3435&password=ZZZ&password2=ZZZ&nodouble=1

Test plan:

Trigger
members/memberentry.pl?op=save&destination=circ&borrowernumber=42&password=ZZZ&password2=ZZZ&nodouble=1

=> Without this patch, the password will be updated
=> With this patch applied you will get a crash "Wrong CSRF token" (no
need to stylish)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: removed the commented use Digest::MD5-line.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-05-03 12:43:12 +12:00
Nick Clemens 00b76faf0a Bug 18423 - Followup - enable Add child button for institutional borrowers
If they can have guarantors and we shwo the button on one page I think
we should in fact show it on all.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-04-30 23:19:58 +12:00
Nick Clemens c481c15a4f Bug 18423 - Add 'Add child' button to files, housebound, and deletemem pages
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-04-30 23:19:58 +12:00
Nick Clemens 63c72da948 Bug 18423 Follow-up - Add 'Add child' button to statistics tab
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-04-30 23:19:58 +12:00
Nick Clemens 4cc1f7304f Bug 18423 follow up - fix display off add child on members/notices.pl
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-04-30 23:19:58 +12:00
Nick Clemens 46fd998c85 Bug 18423 - Add child button not always appearing - problem in template variable
This patch removes the retrieval of the syspref borrowerRelationship
from the scripts and moves it to a check using the Template Toolkit
plugin

To test:
1 - Apply patch
2 - Ensure 'Add child' button displays appropriately on all member pages.

Tested 3 pateches together, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-04-30 23:19:58 +12:00
Jonathan Druart 39b1653348 Bug 18124: Change the calls to generate and check CSRF tokens
The parameter change in Koha::Token should be applied to the calling
scripts.

Test plan:
Confirm that the different forms of the scripts modified by this patch
still work correctly.

Test the problematic behavior:
Open 2 tabs with in same user's session, go on the edit patron page
(memberentry.pl).
Log out and log in from the other tab.
Submit the form
=> Wrong CSRF token should be raised

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
2017-04-24 11:00:08 +12:00
Jonathan Druart 9e74db7b51 Bug 18124: Restrict CSRF token to user's session
Currently the CSRF token generated is based on the borrowernumber, and
is valid across user's session.
We need to restrict the CSRF token to the current session.

With this patch the CSRF token is generated concatenating the id
(borrowernumber) and the CGISESSID cookie.

Test plan:
Run t/Token.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-04-24 11:00:07 +12:00
Jonathan Druart 99a2c87fc1 Bug 17905: FIX CSRF in member-flags
If an attacker can get an authenticated Koha user to visit their page
with the url below, privilege escalation is possible

The exploit can be simulated triggering
    /cgi-bin/koha/members/member-flags.pl?member=42&newflags=1&flag=superlibrarian

Test plan:
Trigger the url above
=> Without this patch, 42 is now superlibrarian
=> With this patch, you will get the "Wrong CSRF token" error.

This vulnerability has been reported by MDSec.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 0c3c162f76)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
2017-01-31 11:46:33 +13:00
Lari Taskula 8e72fc1dfa Bug 17419: Fix smsalertnumber and mobile confusion in moremember.pl
members/moremember.pl will set mobile number as smsalertnumber in template if
smsalertnumber is not defined. This will cause incorrect display for SMS number
in patron's Details-tab. This confusion between smsalertnumber and mobile is
already fixed in Bug 14683, but members/moremember.pl was not fixed yet.

This is a minor issue since it won't occur for manually added new patrons due to
fixes already pushed in Bug 14683, but in case patron's smsalertnumber is null
in database, this bug can be replicated:

To test:
1. Set EnhancedMessagingPreferences to "Allow" and make sure SMSSendDriver
   has been set.
2. Add a new patron, give it a mobile/other phone number.
3. Run a SQL query:
   update koha.borrowers set smsalertnumber=NULL where borrowernumber=XXX;
   (replace XXX with your new patron's borrowernumber)
4. Go to patron's details tab and observe that SMS number shows the mobile/
   other phone you provided earlier.
5. Apply patch.
6. Refresh patron's details tab.
7. Observe that smsalertnumber is now empty, as it should be.

Followed test plan, works as expected.
Signed-off-by: Marc <veron@veron.ch>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2016-12-12 14:54:15 +13:00
Jonathan Druart 0d9e1667bf Bug 17375: Search by dateofbirth - handle invalid dates
Prevent internal software error when searching patron with invalid birth date

To reproduce:

- Go to Home > Patron
- Expand patron search (click on + at the left of the search button)
- In drop down 'Search fields', select 'Date of birth'
- Enter a valid date (e.g. 11.02.1995 if syspref 'dateformat' is set to dmydot)
Result: Search works OK
- Enter an invalid date, e.g. 11.02 or abcd...
Result: Internal server error

- Do a patron search with many results
- Use filter on results screen, select 'Date of birth' as search field and
  enter an invalid date to search (e.g. 'a')
Result: Endless message 'Processing'

To test:
- Apply patch
- Repeat steps above
- In both cases, you should get "No results"

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Lucio Moraes <lmoraes@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 9b34b07d62)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-11-02 17:58:26 +00:00
Sophie Meynieux a8dbb0cae8 Bug 17154 : Note column is missing on account lines receipt
When displaying Fines > Account tab for a patron, you can see on screen a Note column that is missing if you click on Print

    Test plan :
    * Find a patron with accountlines or add them manually (Create manual invoice/credit).
    * Be sure some of them got a Note
    * Clik on Print fior those lines

    Without patch, the printed receipt does not show the Note column
    with the patch, the printed receipt shows a Note column and Note content is correctly printed for accountline with a note.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 734d08c2af)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-10-22 15:22:59 +02:00
Fridolin Somers f620fbe0d9 Bug 17423 - patronimage.pl permission is too restrictive
Bug 14566 added the permission "borrowers" on patronimage.pl.
This perm is too restrictive because circulation pages also uses this page.

I propose to simply use "catalogue" perm.

Test plan
- Set an image to borrower xx
- Create a user with only catalogue permission
- Log with this user
- Go to page (replace xx by borrower number) : /cgi-bin/koha/members/patronimage.pl?borrowernumber=xx
=> Without patch you get the page saying you do not have the permission
=> With patch you get the image
- Log out and retest the page patronimage.pl
=> You get 403 error

Signed-off-by: remy <remy.gonzalves@iepg.fr>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit e78f81d615)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-10-22 14:59:53 +02:00
Marc ff2b5116e9 Bug 11217: The # in accountlines descriptions makes them un-writeoffable
This patch adds escaping to url params in mambers/pay.pl

To test:
- Apply patch
- Go to Home > Patrons > Manual invoice and create an invoice with a
  description containing a #
- Go to tab "Pay fines"
- Write off the fine
Expected result: Fine is written off and does no longer display in pay tab.

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 692014c817)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-10-22 14:49:35 +02:00
Marc 0e95348990 Bug 17403: Internal Server Error while deleting patron
This patch fixes a Internal Server Error while deleting patrons.

To reproduce:

- Go to a patron's detail page
- Toolbar : More : Delete
- Confirm "Are you sure you want to delete..."

Result: Internal Server Error
Plack error log:
exited nonzero: 1 at /home/marc/koha/members/deletemem.pl
Note: Patron is deleted

Additional tests: Try to delete yourself, to delete a staff member
without having superlibrarian permission etc.

To test:

- Apply patch
- Re-start plack
- Try to reproduce steps above (with other patron)
Expected result:
No Internal Server Error, Redirect to Home > Patrons

Amended to include all occurences of 'exit 1'

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Good catch Marc

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit b588babdb2)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-10-22 13:47:26 +02:00
Jonathan Druart ade3ffb5ac 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>
(cherry picked from commit 2cf1e97e5c)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-09-08 21:56:35 +02:00
Marc Véron 7602198e60 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>
(cherry picked from commit 492a64cef9)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-23 12:51:00 +02:00
Jonathan Druart 527cd879bb Bug 17052: Display patron category description in the sidebar of paycollect
The patron category description is not passed to the template in
paycollect.pl

Note: The Categories TT plugin should be used here...

Test plan:
Go on paycollect.pl and confirm that the patron category description is
now displayed in the sidebar.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Brendon Ford <brendon.ford@manhattan.edu>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit c3badb0c57)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-23 12:18:48 +02:00
Jonathan Druart 99ac3dd049 Bug 16847: Remove C4::Members::GetTitles
This subroutine just reads the content of a pref, split it, add an
empty string and returns.
It is not really useful and the code in the script (memberentry.pl) uses
the only occurrence of CGI::popup_menu
Let's remove it and build the dropdown list in the template.

Test plan:
1/ Empty BorrowersTitles, edit a patron and confirm that the "Salutation"
does not appear
2/ Fill BorrowersTitles with "Mr|Mrs|Miss|Ms", edit a patron and confirm
that the "Salutation" dropdown list is correctly filled.
The default option should be selected if you are editing a patron with a
title defined.
This should also be tested at the OPAC.

Followed test pan, works as expected in Staff and OPAC
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 131002e623)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 13:35:05 +02:00
Blou 744b035932 Bug 14605 - Corrects the individual fine's description
When paying a fine, the description is built out of the borrower's type and title, instead of the actual fine's description.

STEPS:
1) in the staff, go to a user with a fine to pay.
   a) if none, go to a user and Fines>>"Create manual invoice" with a
      distinctive description.
2) In Fines>>Pay fines, click the "Pay" button on the line of the fine.
3) This will take you a "Pay an individual fine" screen, where the description
   will look like "Staff Mrs", for the category and title of the borrower.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 63041c19c9)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-06-15 09:00:34 +02:00
Aleisha 853d3cfe4d Bug 15823: Can still access patron discharge slip without having the syspref on
EDIT: Fix for OPAC side
EDIT: Comment 10
EDIT: Merge conflicts

To test:
1) Ensure syspref useDischarge is disabled
2) Go to /cgi-bin/koha/members/discharge.pl?borrowernumber=X&discharge=1
3) Validate that you are still able to generate a discharge slip for this patron
4) Apply patch and refresh page
5) Confirm that you are redirected to the circulation.pl page for the user and that an error message is there.
OPAC SIDE
6) Go to the OPAC
7) Go to /cgi-bin/koha/opac-discharge.pl
8) Confirm you get a message saying discharges are disabled
9) Go to /cgi-bin/koha/opac-discharge.pl?op=request
10) Confirm you see same message

Sponsored-by: Catalyst IT

Followed test plan, works as expected (both staff client and OPAC).
Re-tested, works OK.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-05-06 04:20:48 +00:00
Kyle M Hall 640b9585f8 Bug 16429 - Fix root problem
The root of this issue is old code in circulation.pl that is no longer
needed and leaves the affected scripts open to future regressions.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Heather Braum <hbraum@nekls.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-05 10:20:44 +00:00
Kyle M Hall cf6bd4d982 Bug 14497 - Add warning to patron details page if patron's fines exceed noissuescharge
Some librarians have requested that the same warning on the check out
page be placed on the patron details page if a patron's fines exceed the
system preference noissuescharge.

Test Plan:
1) Apply this patch
2) Find or create a patron who's fines exceed noissuescharge
3) Browse to that patron's details ( moremember.pl )
4) You should see a warning near the top of the page with links to the
   patron's account page and the pay fines page

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Patch ammended post sign-off

Signed-off-by: Sally Healey <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 12:15:01 +00:00
Jonathan Druart c133bedf54 Bug 16154: Fix some other occurrences
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-04-26 23:16:44 +00:00
Jonathan Druart f3e4b5bbb6 Bug 16154: CGI->multi_param - Force scalar context
This patch replaces the occurrences of
  $template->param( foo => $cgi->param('foo') );
with
  $template->param( foo => scalar $cgi->param('foo') );

perl -p -i -e 's/(\s*=>\s*)\$(cgi|input|query)\->param\(/$1scalar
\$$2\->param\(/xms' **/*.pl

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-04-26 23:16:43 +00:00
Jonathan Druart 66aacace08 Bug 16154: CGI->multi_param - Declare a list
This patch replaces the occurrences of
  my @foo = $cgi->param('foo');
with
  my @foo = $cgi->multi_param('foo');

perl -p -i -e
's/^(\s*my\s*@\w+\s*=\s*)\$(cgi|input|query)\->param\(/$1\$$2\->multi_param\(/xms'
**/*.pl

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-04-26 23:16:42 +00:00
Jonathan Druart 38094a260e Bug 12267: Remove borrower_attributes.password
When creating a patron attribute type, there is a "Allow password"
checkbox. If checked, the librarian will be able to enter a password for
this patron attribute when editing a patron.
The goal was to allow a patron to log in with a secondary password.
However, this feature has never been implemented.

"""
commit 6fc62bcd32
  CommitDate: Mon May 12 09:03:00 2008 -0500
  extended patron attributes tables & syspref (DB rev 081)

- password_allowed (if set, staff patron editor will
  allow a password to be associated with a value; this
  is mostly a hook for functionality to be implemented
  in the future.
"""

To decrease maintainability, this patch suggest to remove the 2 DB fields
borrower_attributes.password and
borrower_attribute_types.password_allowed
If they have not used by the library.

Test plan:
- Edit a patron attribute type and select "allow password"
- Edit a patron and defined a password for this attribute
- Execute the DB entry
- Note that you get a warning
- Empty the password field
- Execute the DB entry
- You do not get the warning and the 2 DB fields have been removed

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-04-22 23:08:32 +00:00
Jonathan Druart a8491dc156 Bug 15930: Make patron searches defaulting on 'contain'
The default patron search types has changed from 'contain' to
start_with. Users consider it as a bug.
This patch revert the previous changes to default on 'contain'.

Test plan:
Search for patrons in different places (guarantor, checkout, patron
module, acquisition module, etc.) and confirm that the default is always
'contain'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>
2016-04-22 00:31:50 +00:00
Jonathan Druart b64e6be1c4 Bug 16157: Move the selected flag from GetAuthorisedValues to the templates
From C4::Koha::GetAuthorisedValues

    # TODO: the "selected" feature should be replaced by a utility function
    # somewhere else, it doesn't belong in here. For starters it makes
    # caching much more complicated. Or just let the UI logic handle it, it's
    # what it's for.

Indeed, it's not a job for a subroutine, the template should take care of that.
Note that a perf gain could be won with this patch \o/

Test plan:
- Edit an itemtype and check the value of the "Search category" dropdown list
- Edit a patron attribute type and check the value of the "Class" dropdown list
- Detail for a catalogue record, the Status column should be correctly
  populated if items are damaged and/or lost
- Item details for a catalogue record, the lost, damaged and withdrawn
  value should be correctly displayed
- Edit a patron, the "street type" should be correctly selected
- Create a patron attribute type linked to an authorised value list.
- Edit a patron, set a value for this attribute, edit it again. The
  correct value should be selected.
- Search for subscriptions. The 'Location' dropdown list should behave
  correctly (select the entry you have choosen before, etc.)
- Edit a subscription, the location dropdown list should select the
  correct value.
- Edit and view a suggestion with a 'reason for suggestion' set (you
  should have at least 1 OPAC_SUG AV defined)

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-04-07 00:16:09 +00:00
Nick Clemens e4d0363bc7 Bug 15928 - Show unlinked guarantor
To test:
1 - Add guarantor data to patron account by typing it in but do not 'Set to patron'
2 - Note it is not displayed on patron details
3 - Apply patch
4 - Note the info is displayed
5 - Test that linked guarantors show as expected

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended-patch: Correct some indents/spaces

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-04-06 23:50:33 +00:00
Jonathan Druart 4a3404594f Bug 15163: Do not erase patron attributes if limited to another library
The patron attributes displayed on editing a patron are not displayed if
limited to another library.

C4::Members::Attributes::SetBorrowerAttributes will now only delete attributes
the librarian is editing.
SetBorrowerAttributes takes a new $no_branch_limit parameter. If set,
the branch limitations have not effect and all attributes are deleted
(same behavior as before this patch).

Test plan:
1/ Create 2 patron attributes, without branch limitations.
2/ Edit a patron and set a value for these attributes
3/ Limit a patron attributes to a library (one you are not logged in
with).
4/ Edit again the patron.
=> You should not see the limited attributes
5/ Edit the patron attributes and remove the branch limitation
=> Without this patch, it has been removed from the database and is not
displayed anymore.
=> With this patch, you should see it.

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-21 16:56:37 +00:00
Jonathan Druart da21ad0659 Bug 16054: Fix variable scope error in paycollect.pl
$borrower, $borrowernumber and $template and not visible in the
subroutine.
Using $b_ref and passing $template fixes the issue.

Test plan:
Under plack, use the pay selected button.
On the paycollect page, you should see the image of the patron

It certainly fixes some other bugs.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-15 07:12:12 +00:00
Jonathan Druart aa73c96aed Bug 15656: Move guarantor/guarantees code - GetMemberRelatives
Note:
QA question: Does the Koha::Patron->siblings method should return undef
if there is no guarantor?
It would avoid the weird  != undef, = $borrowernumber conditions.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-12 23:40:10 +00:00
Jonathan Druart bff0ca0b0b Bug 15656: Move guarantor/guarantees code - GetGuarantees
2 subroutines of C4::Members deal with guarantor/guarantees:
GetGuarantees and GetMemberRelatives.
Since we already have a Koha::Patron->guarantor method, it makes sense
to move these 2 subroutines to this module.

This first patch deals with GetGuarantees.

Test plan for the entire patch set:
1/ Create 5 patrons A (adult), B (child), C (child), D (child), E
(child), F (adult)
2/ Add relation between them: A is father of B, C and D.
E does not have a guarantor
F does not have guarantees
3/ Check some items out for all of these patrons
4/ On the "Check out" and "Details" tabs, you should not see any
differences with these patch applied : The "Relatives' checkouts" tabs
should list all of the guarantor/guarantee/siblings checkouts

Note:
$template->param('C' => 1);
I have not found any reference of this 'C' in the template.
It seems it's an old c/p from members/memberentrygen.tt

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-12 23:40:10 +00:00
Jonathan Druart 545b64f869 Bug 15635: Koha::Patron::Images - Remove GetPatronImage
To retrieve a patron image, we can call Koha::Patron::Images->find or
Koha::Patrons->find->image
Both will return a Koha::Patron::Image object.

Test plan:
1/ From the patron/member module, open all tabs on the left (Checkouts,
detail, fines, etc.)
The image should be correctly displayed.
2/ At the OPAC, on the patron details page (opac-memberentry.pl) the
image should be displayed as well.
3/ Same on the sco module.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-03-04 12:54:15 +00:00
Jonathan Druart 3691bd8419 Bug 15548: Move new patron related code to Patron*
The 'borrower' should not be used anymore, especially for new code.
This patch move files and rename variables newly pushed (i.e. in the Koha
namespace).

Test plan:
1/
  git grep Koha::Borrower
should not return code in use.

2/
Prove the different modified test files

3/ Do some clicks in the member^Wpatron module to be sure there is not
an obvious error.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as described. Tested with Circulation, Members/Patrons, Discharge,
Restrictions modules and the must common functionalities

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
2016-03-03 14:38:26 -07:00
Lari Taskula fbb7097b1d Bug 12426: Allow resend for sent messages
This patch allows to resend both sent and failed messages.

With messages in 'sent' status, we have to be careful not to accidentally send
sent messages again. With the previous patch using GET request, this was likely
to happen because of browser storing the GET parameters.

This patch changes request method from GET to POST. Instead of a simple link,
we now have a form element.

In notices.pl we redirect back to notices.pl, because with POST there is a risk
of resending the message accidentally by form resubmission at refresh.

To test, find/create a Patron that has sent or failed notices in message_queue:
1. Enable EnchancedMessagingPreferences system preference
2. Go to Patrons -> Notices
3. In the Notice column, click the title of the sent or failed message
4. Observe that there is nothing for resending the sent or failed message
5. Apply the patches.
6. Reload Notices page and repeat step 3
7. Observe that there is now a link "Resend" in the Status-column
8. Click Resend
9. Observe that the message gets into 'pending' status

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:16:07 +00:00
Jonathan Druart 0813c72b1f Bug 12426: Simplify the code adding a new subroutine GetMessage
The C4::Letters module does not have a GetMessage subroutine, which
could be quite useful.
This patch adds it and simplifies the code added by the previous patch.
It also adds a few tests and fixes POD typos.

Note that ResendNotice only resends failed messages. This will avoid to
resend already sent messages (using an url from the browser history for
instance).

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:16:07 +00:00
Lari Taskula 05ea6ba1a8 Bug 12426: Allow resending of messages from the notices tab in the patron account
This patch adds a link 'Resend' under a notice in 'failed' status
in the Patron's Notices tab.

By clicking the link, we will request notices.pl with parameter
"resendnotice=XXXXX" where XXXXX is message_id. In notices.pl,
we then check whether the given message is actually in 'failed'
status. If so, we use the C4::Letters::ResendMessage(123) to
change the status of the message into 'pending'. This way it
will be processed again by the cronjob process_message_queue.pl.

To test, find/create a Patron that has failed notices in message_queue:
1. Enable EnchancedMessagingPreferences system preference
2. Go to Patrons -> Notices
3. In the Notice column, click the title of the failed message
4. Observe that there is nothing for resending the failed message
5. Apply patch.
6. Reload Notices page and repeat step 3
7. Observe that there is now a link "Resend" in the Status-column
8. Click Resend
9. Observe that the message gets into 'pending' status

Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-03 20:16:06 +00:00
Kyle M Hall 3b8f35de79 Bug 15629 [QA Followup]
* Use ->id instead of ->branchcode when possible to eliminate use of that nomenclature
* Fix bad use of ->branchcode to ->{branchcode} for unblessed hashref  version of Koha::Library

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-02-24 03:55:07 +00:00
Jonathan Druart 4f5217314c Bug 15629: Koha::Libraries - Remove GetBranchDetail
C4::Branch::GetBranchDetail retrieved library infos, it could be easily
replaced with Koha::Libraries->find

When this change needs other big changes, the unblessed method is
called, to manipulate a hashref (as before) instead of a Koha::Library
object (for instance when $library is sent to GetPreparedLetter).

Test plan:
1/ Print a basket group, the library names should be correctly
displayed.
2/ Enable emailLibrarianWhenHoldIsPlaced and place a hold, a HOLDPLACED
notice will be generated (focus on the library name)
3/ Edit a patron and change his/her library
4/ Generate the advanced notices (misc/cronjobs/advance_notices.pl) and
have a look at the generated notices
5/ Same of overdues notices
6/ Set IndependentBranches and use a non superlibrarian user to place a
hold. The "pickup at" should be correctly filled.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-02-24 03:55:06 +00:00