Commit graph

69 commits

Author SHA1 Message Date
Julian Maurice
b168f4a2e9 Bug 21395: Make perlcritic happy
This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes
almost all perlcrictic violations according to this .perlcriticrc
The remaining violations are silenced out by appending a '## no critic'
to the offending lines. They can still be seen by using the --force
option of perlcritic
This patch also modify t/00-testcritic.t to check all Perl files using
the new .perlcriticrc.
I'm not sure if this test script is still useful as it is now equivalent
to `perlcritic --quiet .` and it looks like it is much slower
(approximatively 5 times slower on my machine)

Test plan:
1. Run `perlcritic --quiet .` from the root directory. It should output
   nothing
2. Run `perlcritic --quiet --force .`. It should output 7 errors (6
   StringyEval, 1 BarewordFileHandles)
3. Run `TEST_QA=1 prove t/00-testcritic.t`
4. Read the patch. Check that all changes make sense and do not
   introduce undesired behaviour

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:02 +02:00
481bc59de5
Bug 23473: (follow-up) check if overwrite_passwords is set
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-27 12:23:00 +00:00
6464ada6c6
Bug 23473: Allow overwrite of passwords during import
To test:
 1 - Have some patrons in your system
 2 - Export some of their info via reports
    SELECT cardnumber, userid, surname, firstname, password, branchcode, categorycode
 3 - Edit the file from above, changing all the password lines
 4 - Import the file with overwrite
 5 - Confirm passwords have not changed (run the report again and confirm the hashes are the same)
 6 - Apply patch
 7 - Restart all the things
 8 - Check the new box on import screen to overwrite passwrods
 9 - Import file again
10 - Confirm passwords have changed
11 - Signin using new password to verify the hash is the password as supplied
12 - Repeat via commandline import supplying --overwrite_passwords option
13 - Verify works as expected
14 - Prove -v t/db_dependent/Koha/Patrons/Import.t

Sponsored-by: ByWater Solutions
Signed-off-by: Ron Marion <ron.marion@goddard.edu>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-27 12:21:59 +00:00
f1d210019b
Bug 20443: Move C4::Members::AttributeTypes::GetAttributeTypes to Koha::Patron::Attribute::Types
We can then now start to move methods from C4::Members::AttributeTypes
as well.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-23 13:44:35 +00:00
109334102f
Bug 14570: Make it possible to add multiple guarantors to a record
This patch adds the ability to set an unlimited number of guarantors
for a given patron. As before, each guarantor may be linked to another
Koha patron, and all the behavior that applies to a given guarantor
remains the same.

Test Plan:
 1) Apply this patch
 2) Run updatedatabase.pl
 3) Find some patrons with guarantors, verify the still have their guarantor
 4) Test adding and removing guarantors on a patron record, both Koha users and not
 5) Verify the "Add child" button works
 6) Verify NoIssuesChargeGuarantees still works
 7) Verify tools/cleanborrowers.pl will not delete a guarantor
 8) Verify the guarantors are displayed on moremember.pl
 9) Verify the guarantor is removed by members/update-child.pl
10) Verify the guarantor is removed by misc/cronjobs/j2a.pl
11) Verify import patrons converts guarantor_id, relationship, contactfirstname,
  and contactsurname into a guarantor
12) prove t/Patron.t
13) prove t/db_dependent/Circulation.t
14) prove t/db_dependent/Circulation/NoIssuesChargeGuarantees.t
15) prove t/db_dependent/Items.t
16) prove t/db_dependent/Koha/Patrons.t
17) prove t/db_dependent/Members.t
18) prove t/db_dependent/Patron/Relationships.t

Signed-off-by: Kim Peine <kmpeine@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Agustin Moyano <agustinmoyano@theke.io>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-08-20 16:04:48 +01:00
b990b953b3 Bug 21993: Display a user-friendly message when the CSRF token is wrong
Instead of dying!

Test plan:
Assuming you have a patron with borrowernumber=51 and another one that
can be deleted with borrowernumber=42

- authorities-home.pl
 * Delete an authority record
 * hit /cgi-bin/koha/authorities/authorities-home.pl?op=delete

- basket/sendbasket.pl
 * Send a basket to someone
 * hit /cgi-bin/koha/basket/sendbasket.pl?email_add=1

- members/apikeys.pl
  * Generate and delete an API key for a patron
  * hit /cgi-bin/koha/members/apikeys.pl?patron_id=51&op=delete

- members/deletemem.pl
  * Delete a patron
  * hit /cgi-bin/koha/members/deletemem.pl?member=42&op=delete_confirmed

- members/mancredit.pl
  * Add a manual credit
  * hit /cgi-bin/koha/members/mancredit.pl?borrowernumber=51&add=1

- members/maninvoice.pl
  * Add a manual invoice
  * hit /cgi-bin/koha/members/maninvoice.pl?borrowernumber=51&add=1

- members/member-flags.pl
  * Change permissions for a patron
  * hit /cgi-bin/koha/members/member-flags.pl?member=51&newflags=1

- members/member-password.pl
  * Change the password for a patron (from the staff interface)
  * hit /cgi-bin/koha/members/member-password.pl?member=51&newpassword=aA1

- members/memberentry.pl
  * Edit some patron's info
  * hit /cgi-bin/koha/members/memberentry.pl?borrowernumber=51&op=save

- members/paycollect.pl
  * Pay an individual fine
  * hit something like /cgi-bin/koha/members/paycollect.pl?borrowernumber=51&pay_individual=1&accounttype=L&amount=1.00&amountoutstanding=1.00&accountlines_id=157&paid=1
  You may need to edit some values

- tools/import_borrowers.pl
  * Import some patrons
  * hit /cgi-bin/koha/tools/import_borrowers.pl?uploadborrowers=1

- tools/picture-upload.pl
  * Upload an image for a patron
  * You will need to edit the html content
  hit Home › Tools › Upload patron images
  then locate the csrf_token input and modify its value

Note for QA:
- Opac is not done as blocking_errors.inc does not exist for this
interface
- ill/ill-requests.pl
I did not manage to replace this occurrence

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-01-25 20:38:32 +00:00
29e4291718 Bug 12598: Add use Koha::Patron::debarments statement
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-16 14:05:17 -03:00
2cd0f388fa Bug 12598: Re-add bug 18555: Create patron list from patron import
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-16 14:05:17 -03:00
c2066bcadc Bug 12598: New misc/import_borrowers.pl command line tool
Test Plan:
1) Apply this patch
2) Test importing patrons from command line,
   options are availble with --help.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Tested with minimal csv
(cardnumber,surname,firstname,categorycode,branchcode,password,userid)

Overwrite does not change category or branch.
Patrons are loaded, userid & password works

Updated license to GPLv3
No other koha-qa errors.

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

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

Bug 12598 - Tidy import_borrowers.pl

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

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

Bug 12598 - Move importing code to a subroutine

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

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

Bug 12598 - Update command line script to use patron import subroutine

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

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

Bug 12598 [QA Followup]

* Fix copyright on import_borrowers.pl
* Changes -c --csv to -f --file
* Adds -c --confirm option
* Renames misc/import_borrowers.pl to misc/import_patrons.pl
* Restore userid matchpoint option

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

Bug 12598 - Fix merge to master. Backport 3 updates from latest import_borrowers.pl

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

Bug 12598 - Started regression tests. Fix missing C4::Members::Attributes package

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

Bug 12598 - More refactoring and regression tests in Koha::Patrons::Import

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

Bug 12598 - Creating objects in misc/import_patrons.pl and tools/import_borrowers.pl

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

Bug 12598 - Refactoring Koha::Patrons::Import includes bug fixed for critical date types and header column parsing

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

Bug 12598 - Rebase + backport of 16426 plus fixing 16426

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

Bug 12598: catch warnings raised by import_patrons in tests

Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-16 13:57:57 -03:00
Jenny Way
cad2db6016 Bug 20017: use Modern::Perl in Tools perl scripts
Test plan:
Check that the following files don't contain use strict; use warnings;
and have use Modern::Perl instead.
background-job-progress.pl
batchMod.pl
copy-holidays.pl
exceptionHolidays.pl
holidays.pl
import_borrowers.pl
koha-news.pl
letter.pl
manage-marc-import.pl
newHolidays.pl
overduerules.pl
quotes-upload.pl
quotes.pl
quotes/quotes-upload_ajax.pl
quotes/quotes_ajax.pl
scheduler.pl
stage-marc-import.pl
upload-cover-image.pl

Also the credits have been added to newHolidays.pl

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-05 09:46:45 -03:00
6f595e4c95 Bug 17829: (follow-up) Fix import patron
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-08-22 13:42:43 -03:00
Marc Véron
1b63d3e041 Bug 18555: (QA-Followup) Remove duplicate code
Remove duplicate code as of comment #7

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-08-10 16:25:32 -03:00
Marc Véron
3b87ec988d Bug 18555: Create patron list from patron import
Create a patron list from imported patrons to be used for printing patron
cards and other patron list related actions.

To test:
- Apply patch
- Go to Home > Tools > Import patrons
- Select a file to import and check new checkbox 'Create patron list'
- Import file
- Verify that in import results you see the time stamped name of the
  patronlist containig the imported patrons (if any patrons were imported)
- Go to Home > Tools > Patron lists
- Verify that the list shows up and contains the imported patrons

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-08-10 16:25:32 -03:00
c650764e95 Bug 17829: Fix import patron
Can't call method "unblessed" on an undefined value at
/home/vagrant/kohaclone/tools/import_borrowers.pl line 230, <$fh> line 2.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-07-25 10:29:08 -03:00
2b90ea2cb0 Bug 17829: Move GetMember to Koha::Patron
GetMember returned a patron given a borrowernumber, cardnumber or
userid.
All of these 3 attributes are defined as a unique key at the DB level
and so we can use Koha::Patrons->find to replace this subroutine.
Additionaly GetMember set category_type and description.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-07-10 13:14:19 -03:00
574d48362d 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>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-03-30 09:07:09 +00:00
4e40339db3 Bug 17830: CSRF - Handle unicode characters in userid
If the userid of the logged in user contains unicode characters, the token
will not be generated correctly and Koha will crash with:
  Wide character in subroutine entry at /usr/share/perl5/Digest/HMAC.pm line 63.

Test plan:
- Edit a superlibrarian user and set his/her userid to '❤' or any other strings
with unicode characters.
- Login using this patron
- Search for patrons and click on a result.

=> Without this patch, you will get a software error (with "Wide
character in subroutine entry" in the logs).
=> With this patch, everything will go fine

You can also test the other files modified by this patch.

Signed-off-by: Karam Qubsi <karamqubsi@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-12-30 17:47:18 +00:00
8edb7f6fb9 Bug 17720: CSRF - Handle unicode characters
From the pod of Digest::MD5:
"""
Since the MD5 algorithm is only defined for strings of bytes, it can not
be used on strings that contains chars with ordinal number above 255
(Unicode strings). The MD5 functions and methods will croak if you try
to feed them such input data.
What you can do is calculate the MD5 checksum of the UTF-8
representation of such strings.
"""

Test plan:
- Set a MySQL/MariaDB password with unicode characters:
  UPDATE user SET password=PASSWORD('❤') WHERE USER='koha_kohadev';
  FLUSH PRIVILEGES
- Update your $KOHA_CONF file
- Restart Memcached
- Hit the files modified by this patch

=> Without this patch, you will get a software error (with "Wide
character in subroutine entry" in the logs).
=> With this patch, everything will go fine

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Edit: removed debugging leftover

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-12-05 15:20:18 +00:00
92bfc28eb0 Bug 16911: Koha::Patron::Categories - Move GetExpiryDate to ->get_expiry_date
This patch moves the code from C4::Members::GetExpiryDate to
Koha::Patron::Categories.
Nothing much exiting, please follow test plan in next patches.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-09-09 09:45:31 +00:00
9b9803b69c Bug 15758: Koha::Libraries - Remove GetBranchesLoop
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-09-08 14:36:02 +00:00
19a977dc7b Bug 15758: Koha::Libraries - Remove GetBranchName
This is the fourth and last patch set to remove C4::Branch.
The real purpose of this patch is to standardise and refactor some code
which is related to the libraries selection/display.
Its unconfessed purpose is to remove the C4::Branch package.

Before this patch set, only 6 subroutines still existed in the C4::Branch
package:
- GetBranchName
- GetBranchesLoop
- mybranch
- onlymine
- GetBranches
- GetBranch

GetBranchName basically returns the branchname for a given branchcode.
The branchname is only used for a display purpose and we don't need to
retrieve it in package or pl scripts (unless for a few exceptions).
We have a `Branches` template plugin with a `GetName` method which does
exactly this job.
To achieve this removal, we will use this template plugin and delete the
GetBranchName from pl and pm files.
The `Branches.all()` will now select the library of the logged in user
if no `selected` parameter has been passed.
This new behavior could cause regressions, for instance there are some
places where we do not want an option preselected (batch item
modification for instance), keep that in mind when testing.

GetBranchesLoop took 3 parameters: $branch and $onlymine.
The first one was used to set a "selected" flag, for a display purpose:
select an option in the libraries dropdown lists.
The second one was useless: If not passed or set to 0, the
`C4::Branch::onlymine` subroutine was called.
This onlymine flag was use to know if the logged in user was able to see
other libraries infos.
A patron can see the infos from other libraries if IndependentBranches
is not set OR if he has the superlibrarian permission.
Prior to this patch set, the "onlymine test" was done on different
places (neworderempty.pl, additem.pl, holidays.pl, etc.), including the
Branches TT plugin. In this patch set, this test is only done on one
place (C4::Context::only_my_library, code moved from
C4::Branch::onlymine).
To accomplish the same job as this subroutine, we just need to call the
`Branches.all()` method from the `Branches` TT plugin. It already
accepts a `selected` parameter to set a flag on the option to select.
To avoid the repetitive
  [% IF selected %]<option selected="selected">[% ELSE %]<option>[% END %]
pattern, a new `html_helpers` TT include file has been created, it
defines an `options_for_libraries` block, which takes a `selected`
parameter. We could imagine to use this include file for other
selects.

The 'mybranch` and `onlymine` subroutines of the C4::Branch package have
been moved to C4::Context. onlymine has been renamed with
only_my_library. There are only 4 occurrences of it, against 11 before
this patch set.
There 2 subroutines are Context-centric and it makes sense to put them
in `C4::Context` (at least it's the least worst place!)

GetBranches is the tricky part of this patch set: It retrieves all the
libraries, independently of the value of IndependentBranches.
To keep the same way as the existing calls of `Branches.all()`, I have
added a `unfiltered` parameter. If set, the `Branches.all()` will call
a usual Koha::Libraries->search method, otherwise
Koha::Libraries->search_filtered will be called. This new method will
check if the logged in user is allowed to see other libraries or only
its library.
Note that this `GetBranches` subroutine also created a `category` key:
it allowed to get the list of groups (of libraries) where this library
existed. Thanks to a previous patch set (bug 15295), this value was
not used anymore (I may have missed something!).

Note that the only use of `GetBranch` was buggy (see bug 15746).

Test plan (for the whole patch set):
The best way to test this whole patch set is to test with 2 instances: 1
with the patch set applied, 1 using master, to be sure there is no
regression.
It would be good to test the same with `IndependentBranches` and the
without `IndependentBranches`.
No difference should be found.
The tester must focus on the library dropdowns on as many forms as
possible.
You will notice changes in the order of the options: the libraries will
now be ordered by branchname (instead of branchcode in some places).
A special attention will be given to the following page:
- acqui/neworderempty.pl
- catalogue/search.pl
- members/members-home.pl (header?)
- opac/opac-topissues.pl
- tools/holidays.pl
- admin/branch_transfer_limits.pl
- admin/item_circulation_alerts.pl
- rotating_collections/transferCollection.pl
- suggestion/suggestion.pl
- tools/export.pl

Notes for QA:
- There are 2 FIXMEs in the patch set, I have kept the existing behavior,
but I am not sure it's the good one. Feel free to open a bug report and
I will fill a patch if you think it's not correct. Otherwise, remove the
FIXME lines in a follow-up patch.
- The whole patch set is huge and makes a lot of changes.
But it finally will tremendously reduce the number of lines:
716 insertions for 1910 deletions

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-09-08 14:36:01 +00:00
6239bbd35f Bug 15407: Koha::Patron::Categories - replace GetBorrowercategoryList
C4::Members::GetBorrowercategoryList returned all the patron categories,
ordered by description.
This can be done using the search method of the Koha::Patron::Categories
module.

Test plan:
- Same prerequisite as before
For the following pages, you should not see patron categories limited to other
libraries. They should be ordered as before this patch, by description.
- On the "patron attribute types" admin page (admin/patron-attr-types.pl).
- On the "Batch patron deletion/anonymization"
  (tools/cleanborrowers.pl), the dropdown list should be filled
  correctly.
- On the import patron tool
- On the following report:
    reports/reserves_stats.pl
Confirm that the patron categories are displayed and that the report
still works as before.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-09-08 13:29:22 +00:00
a5a23bcebc Bug 15407: Koha::Patron::Categories - replace GetBorrowercategory
The purpose of C4::Members::GetBorrowercategory was to return the patron
category for a given categorycode. This can be done easily with the
Koha::Patron::Categories->find method.

Test plan:
- Same prerequisite as before
- Edit a guarantor and confirm the information will be updated for his/her
  guarantee(s).
- Update a child to adult patron (Note: Should not we hide the patron
  categories limited to others libraries? If yes, it must be done on another
  bug report).
  There is a special behavior if there is only 1 adult category, the
  user does not need to select a category. So the same as before with
  only 1 adult patron category.
- Import a patron with a category_code which does not exist. You should
  be warned.
- Modify some patron using the batch patron modification tool. The
  patron category descriptions should correctly be displayed.

Note that the overduerules page has already been tested in a previous
patch.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-09-08 13:29:17 +00:00
6f5e2f8a86 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>
2016-09-02 13:47:02 +00:00
c97e04e8e7 Bug 16889: Remove C4::Members::columns and use Koha::Patrons->columns instead
Test plan:
Import some patrons (tools/import_borrowers.pl) and make sure it imports
the patrons correctly.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-07-15 18:12:12 +00:00
7b76b24fad Bug 16426: follow-up of bug 15840 - correctly manage userid while inserting patrons
Bug 15840 tried to fix a bug but makes things more complicated than it
was before.
If an userid is not provided for 1 or more rows of the csv file, it
should not be updated. However, if a userid is provided and it already
used by an other patron, the import should fail for this row (but not
crash!).

Test plan:
0/ Create a patron with a userid=your_userid
1/ Use the import patron tool to update this userid
=> userid should have been updated
2/ Update another data and do not provide the userid
=> data should have been updated and not the userid
3/ Update another data and provide the userid, but set it to an empty
string, or '0'
=> data should have been updated and not the userid
4/ Update another patron, and set userid=your_userid
=> Update should fail and an error whouls be displayed ("already used by
another patron")

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

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

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-05-06 03:41:37 +00:00
f1e156453a Bug 15840: Catch errors if userid already exists when importing patrons
Import borrowers tool explodes if userid already exists. This error
should be caught to avoid the script to explodes.

TEST PLAN
---------
1) back up DB
2) drop DB, create empty DB
3) Apply patch 15842
4) web install all the sample data
5) login as system user and import patron data
   -- sample provided to trigger error.
   -- you will get a DBIx::class exception
      "Duplicate entry ' for key 'userid'"
6) Apply this patch
7) Repeat steps 2-5
   -- patrons won't be inserted but an error
      will be nicely displayed in the error list.
8) run koha qa test tools

Note that it would be easy to insert the patrons anyway and generate a
userid based on the surname and firstname.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-21 21:24:00 +00:00
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
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
314f3e22d9 Bug 15014: Returns 0 if everything went fine
Be kind and return 0 if the sample file has been generated correctly.
Otherwise you are a bad guy and you make Plack returns an internal error
(exited nonzero: 1 at /home/vagrant/kohaclone/tools/import_borrowers.pl line 390.)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Works as expected
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Works as expected here too.
2015-11-04 13:32:01 -03:00
fd12390365 Bug 15010: Import patron tool creates 'duplicate' restrictions ( debarments )
If a patron has restrictions and a CSV file of patrons to be overwritten
is uploaded but has no debarred / debarredcomment columns, the a new
restriction will be added that combines all the existing restrictions (
i.e. a new debarment based on the existing borrowers.debarred
and borrowers.debarredcomment fields ).

Test Plan:
1) Create a patron with one or more restrictions
2) Import a CSV file that updates that patron, but with no debarred columns
3) Note a new restrictions is created
4) Delete the new restriction
5) Apply this patch
6) Re-import the CSV file again
7) Note that no new restriction is created
8) Add debarred and debarredcomment columns to the CSV file with a test
   date and description
9) Re-import the CSV file again
10) Note the new restriction is added to the patron

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Daniel Grobani <dgrobani@samuelmerritt.edu>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-10-30 10:00:20 -03:00
Marc Véron
33e679b905 Bug 14997: Remove C4::Dates from tools (import / export)
This patch removes C4::Dates from:

- tools/export.pl
- tools/import_borrowers.pl

Note: For testing, both need preparation without patch, see below.

To test export:
- Without patch applied, go to
  Home > Tools > Export data > Export bibliographic records
- Define Start date / End date for Accession dates
- Export bibliographic records as 'without-patch.mrc'

- Do the same with patch, export as 'with-patch.mrc
- Compare the files, they should be the same

To test Import patrons:
- Without patch
- Go to Home > Tools > Import patrons
- Create a patron category like 'TEST' (useful for filtering...)
- Prepare a file with some patrons with category TEST to import.
  Fill date of birth, enrolment date, expiry date with values
  formatted in syspref format, in iso format and garbage
- Import
- Review the imported patrons (search for category TEST)

- With patch: Change cardnumber and names in import file
- Import
- Review again and compare with results from previous import.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-10-27 10:10:18 -03:00
b3f215bd2a Bug 14981: Remove ethnicity occurrences from sample patrons file
Bug 10020 has removed these columns, the should be removed from the
patron sample file.

Test plan:
Insert the files updated by this patch into a fresh DB, you shouldn't
get any error.

Signed-off-by: Frederic Demians <f.demians@tamil.fr>
  Before applying the patch, I was able to notice the error when loading the
  patron sample SQL file to a DB. With the patch, it's solved.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-10-23 10:10:46 -03:00
02492bd6ea Bug 15005: Replace $ENV{SCRIPT_NAME} with the hardcoded script paths
Since I don't manage to make SCRIPT_NAME works directly with RequestHeader and/or
ReverseProxyPath, the easier way it to fix all the different occurrences.

Test plan:
On the import patrons page, confirm that the "Import" button does not redirect to a 'Not found' page.
You should stay on the import patrons page.

Note that if this change works, all others should work too.
Have a look at the diff of this patch and confirm there is no typo.

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

Tested in patron import and item batch modification

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-10-19 09:36:43 -03:00
Stefan Weil
0cac7bc140 Bug 14383: Fix some typos in comments and documentation
Most of them were found and fixed using codespell.

Signed-off-by: Stefan Weil <sw@weilnetz.de>

http://bugs.koha-community.org/show_bug.cgi?id=14383
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2015-06-22 17:34:46 -03:00
Jonathan Druart
a6c9bd0eb5 Bug 9978: Replace license header with the correct license (GPLv3+)
Signed-off-by: Chris Nighswonger <cnighswonger@foundations.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

http://bugs.koha-community.org/show_bug.cgi?id=9987

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-04-20 09:59:38 -03:00
011d8e9855 Bug 12543 - Add userid as matchpoint for "Import patrons" tool
The a patron's userid should be a matchpoint in the same manner as
cardnumber. Though not enforced as a unique key by the database yet
( pending bug 1861 ), this field is effectively unique as uniqueness
is enforced by Koha itself.

Test Plan:
1) Apply this patch
2) Browse to tools/import_borrowers.pl
3) Download the starter CSV file
4) Edit the csv file to include 1 or more patrons
   * Make sure to leave the borrowernumber field empty
   * Make sure the userid field matches the patrons you wish to overwrite
5) From import_borrowers.pl, upload your file
   * Set "Field to use for record matching" to "Username"
   * Set "If matching record is already in the borrowers table" to "Overwrite the existing one with this"
6) Click "Import"
7) Verify the patrons in your file have been updated in Koha

Signed-off-by: Frederic Demians <f.demians@tamil.fr>

Do exactly what's announced. This may help.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-02-09 13:45:06 -03:00
Jonathan Druart
e20270fec4 Bug 11944: use CGI( -utf8 ) everywhere
Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-01-13 13:07:21 -03:00
c3c473b1b9 Bug 12929 [QA Followup] - Hide borrownumber from sample csv file
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Works as described, hides the borrowernumber from the list
of fields to edit and also brings back the list of fields
for the sample file in the documentation.

Passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-10-14 11:25:31 -03:00
Jonathan Druart
61af4786ee Bug 12929: Restore behavior before bug 12716
This patch remove borrowernumber from the borrower list fields and
display the field list in the notes.

Test plan:
- verify that the list of fields is displayed on the right (Notes).
- verify the borrowernumber is not displayed anymore in the field list.

Signed-off-by: Petter Goksoyr Asen <boutrosboutrosboutros@gmail.com>

Confirmed that this patch removes borrowernumber from list of default
values, and reintroduces missing list of CSV columns in the Notes field.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-10-14 11:25:19 -03:00
Jacek Ablewicz
09c4417216 Bug 12859 - Patron import tool doesn't import extened attributes values any longer
To reproduce:

1/ define/set up extended pattron attribute in administration module,
2/ using "Tools -> Import patrons", try to import some sample patron
record[s], having some arbitrary (non-empty) value assigned in
'pattron_attributes' CSV column for attribute defined in step 1/,
3/ observe that extended attribute defined in step 1/ was not imported
(not added to the database), without any warnings.

This patch restores a small code chunk which went AWOL somewhere around
commit b4158f2d24.

To test:

1/ apply patch & retest,
3/ ensure that this issue is no longer reproductible, and there are no
apparent regressions of any kind,
4/ make sure that the ability to specify default value for extended
attributes added in bug Bug 12716 is still working as intended
(= follow up the test plan for bug 12716).

Signed-off-by: Petter Goksoyr Asen <boutrosboutrosboutros@gmail.com>
Patch fixes import of extended patron attributes.
Step 1-3 in test plan confirmed.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-09-16 15:09:35 -03:00
b4158f2d24 Bug 12716: Allow the import patrons form have drop-downs and datepickers
To reproduce:
- Go to Tools > Import patrons
- Notice branchcode and categorycode don't have dropdown menus with
  valid options.
- Notice dateofbirth, dateenrolled and dateexpiry don't have date-pickers
- Notice each input field is labeled with the column name instead of a
  proper description.

To test:
- Apply the patch, go to Tools > Import patrons
- Notice branchcode and categorycode have dropdown menus with valid options,
  and by default the empty value is selected.
- Notice that if you go into the dateofbirth, dateenrolled and dateexpiry
  fields, a nice date picker widget appears.
- Notice each field is labeled with the field description, and that the
  column name is conveniently shown at the right of the input field.

Important: test switching the ExtendedPatronAttributes syspref, and verify that
if enabled, the patron_attributes field appears, and also the checkbox selector
controlling the extended patron attributes import behaviour shows too.

Bonus points: Verify that on a different language, the descriptions get translated,
and the column names show correctly

Regards
To+

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes tests and QA script.
Works as described, no regressions found.
Dates are output in ISO/database format, this is ok.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-08-15 15:26:44 -03:00
afd2418d73 Bug 11349: Change .tmpl -> .tt in scripts using templates
Since we switched to Template Toolkit we don't need to stick with the
sufix we used for HTML::Template::Pro.

This patch changes the occurences of '.tmpl' in favour of '.tt'.

To test:
- Apply the patch
- Install koha, and verify that every page can be accesed

Regards
To+

P.S. a followup will remove the glue code.

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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2014-07-17 11:05:49 -03:00
b35665ba9d Bug 11287: make patron import tool take new restrictions system into account
The import patrons tool will add restrictions in the previous style (
directly to the borrowers table ). This will actually work, but
will result in the restriction being un-removable, and will be
overwritten by any new restrictions.

Test Plan:
1) Apply this patch
2) Import new patrons with debarrments, note the debarment is created
   correctly
3) Import existing patrons with overwrite enabled, note that any new
   debarrment is added, and any existing debarrment is ignored

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-05-05 22:37:12 +00:00
Jonathan Druart
834abff631 Bug 10861: Add a check on cardnumber length
Some libraries would like to add a check on the cardnumber length.
This patch adds the ability to restrict the cardnumber to a specific
length (strictly equal to XX, or length > XX or min < length < max).
This restriction is checked on inserting/updating a patron or on importing
patrons.

This patch adds:
- 1 new syspref CardnumberLength. 2 formats: a number or a range
  (xx,yy).

- 1 new unit test file t/Members/checkcardnumber.t for the
C4::Members::checkcardnumber routine.

Test plan:
1/ Fill the pref CardnumberLength with '5,8'
2/ Create a new patron with an invalid cardnumber (123456789)
3/ Check that you cannot save
4/ With Firebug, replace the pattern attribute value (for the cardnumber
input) with ".{5,10}"
5/ You are allowed to save but an error occurred.
6/ Try the same steps for update.
7/ Go to the import borrowers tool.
8/ Play with the import borrowers tool. We must test add/update patrons
and the "record matching" field (cardnumber or a uniq patron attribute)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested adding, updating; importing and ran unit test.
Preliminary QA comments on Bugzilla

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-03-12 02:16:18 +00:00
Galen Charlton
a36b3ad43a Bug 7785: (follow-up) standardize POD
This makes the POD for the columns() function consistent
with the rest of C4/Members.pm.  It also removes a note
that can be relegated to the bug report and the Git
history.

Also, since C4::Members::columns() is not actually a
class method, this patch changes the invocation to
not call it that way.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2013-08-13 16:15:30 +00:00
Jonathan Druart
acbfe26c9e Bug 7919 : Display of values depending on the connexion library
In a librairies network, we would like to declare specific values just
for one (or more) library.
Here we implement the ability to associate categories, patron attributes
types and/or authorised_values with librairies (branches).

This patch adds 3 new association tables:
- categories_branches ( association table between categories and branches )
- authorised_values_branches ( association table between
  authorised_values and branches )
- borrower_attribute_types_branches (association table between
  borrower_attribute_types and branches )

Plan test:
  - Create (or modify) categories, patron attributes and
    authorised_values and link it with one (or more) library.
  - Set one of these librairies
  - Go to one of the multiple pages where this specific value must be displayed
    and check that it does appear.
  - Set a library not concerned.
  - Check on the same pages this value is doest not appear.

A page list:
cataloguing/addbiblio.pl
cataloguing/additems.pl
members/members-home.pl
members/memberentry.pl
acqui/neworderempty.pl
tools/modborrowers.pl
and others :)

Please say me if filters don't work on some pages.

Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com>
Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Melia Meggs <melia@bywatersolutions.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
2012-12-05 10:10:57 -05:00
Robin Sheat
5dd94487ac Bug 5373 - allow 'cardnumber' to appear on the patron import field list
For some reason, it was explicitly removed, however you need it if you
want to match up on cardnumber.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Easy to test and works nicely.
Before patch cardnumber is not shown in the list of fields on the
patron import page. After applying the patch it is.

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
2012-02-21 17:04:18 +01:00
Frédérick Capovilla
906ef4b28c Bug 6854: Correct a double password encryption problem in import_borrowers.pl
When the CSV file has no password column and no default value was set for
the password :
If we try to modify an existing user, the password from the BD is sent
to ModMember, which then encrypts the already-encrypted password a
second time.

http://bugs.koha-community.org/show_bug.cgi?id=6854

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Confirmed bug on current master.
1) Import new patron with password provided
> OPAC login works
2) Overwrite patron record, password provided
> OPAC login works
3) Overwrite patron record, password not provided
Before patch: OPAC login broken, password in database changed
After patch: OPC login works with same password as before

Signed-off-by: Ian Walls <ian.walls@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
2011-10-21 04:47:05 +13:00
Chris Nighswonger
ae48ebbcaa Bug 5379 - import_borrowers.pl fails with db insert/update errors
Some spreadsheet programs use smart quotes which causes the db to throw
an error when an insert/update is attempted due to improper processing
of the CSV file. This patch adds code to check for smart quotes and change
them to "dumb" quotes.

This patch also adds more logging of errors and a notice to the user to check
the logs for errors when they occur.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
2011-07-05 11:15:00 +12:00