Commit Graph

602 Commits

Author SHA1 Message Date
Mason James 826f33b719 Revert "Bug 18329 - Batch record deletion broken"
This reverts commit f6cac05923.
2017-05-05 21:40:21 +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
Jonathan Druart e1a72e9d21 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: Mason James <mtj@kohaaloha.com>
2017-05-03 14:28:37 +12:00
Jonathan Druart b811451989 Bug 18312: Fix export unless a file is supplied
Bug 18087 breaks export unless a file is supplied.

Can't use an undefined value as a HASH reference at
/home/vagrant/kohaclone/tools/export.pl line 75.

Test plan:
Export records using a file of id that is not a valid file (not txt or
csv)
Export records using a valid file
Export records without supplying a file

=> The export should work or fail as expected.

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-04-30 23:20:03 +12:00
Jonathan Druart bb0a58494b Bug 18087: Handle invalid filetypes
If an invalid file is used as biblionumber list, we should display a
message.

Test plan:
1/ Use csv, plain text files
=> Should work
2/ Use invalid files (binary files like pdf, doc*, xsl*, etc.)
=> Should not work and see a warning message.

Amended patch after signoff: Remove one warn debug line

Signed-off-by: Joy Nelson <joy@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-04-30 23:20:02 +12:00
Jonathan Druart a09f17fa70 Bug 15451: Better error handling
1/ If a librarian edit (add_validate) a non-existing csv profile, we
explicitely die
2/ If you try to delete a non-existing csv profile, you will now get a
nice alert box
2017-04-30 23:20:02 +12:00
Jonathan Druart de90787db6 Bug 15451: Do not considered a Resultset as a Result
The previous calls were wrong, but there is something bad with the DB
structure: export_format.profile should be a unique key.
This patch fixes the previous calls and add a FIXME not to forget to fix
the DB structure.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Previous test where done with all patches applied,
including this one, and all worked.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-04-30 23:20:01 +12:00
Jonathan Druart df46724747 Bug 15451: Koha::CsvProfiles - Remove the residue
This patch erase all traces of C4::Csv since it's not used anymore.
All occurrences have been replaced by previous patches to use
Koha::CsvProfiles.

Note that GetMarcFieldsForCsv was not used prior this patch set.

Test plan:
  git grep 'C4::Csv'
should not return any result.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No more traces of the file.
This produces a koha-qa fail, due to the missing file.
No other errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2017-04-30 23:20:01 +12:00
Jonathan Druart 63802c7ec2 Bug 15451: Koha::CsvProfiles - Remove GetCsvProfileId
This subroutine returned the export_format_id for a given profile name.
This can be done easily with the Koha::CsvProfiles->search method.

Test plan:
Export records using the misc/export_records.pl script and the
export tool.
If you are exporting using the MARC format, the profile filled in the pref
ExportWithCsvProfile will be used (or the one passed in parameter of
misc/export_records.pl).
If you are exporting using the CSV format, you can choose a profile in
the dropdown list.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Exported using tool & cmd, marc & csv. Pref is used.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-04-30 23:20:00 +12:00
Jonathan Druart ad9bef2419 Bug 15451: Koha::CsvProfiles - Remove GetCsvProfiles
This subroutine did the same job as GetCsvProfilesLoop, so this patch
applies the same changes as the previous patch.

Test plan:
1/ Claim some serials, sql profiles should be listed
2/ Export records using the export tool. MARC profiles should be listed.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Listed sql & marc profiles
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-04-30 23:20:00 +12:00
Jonathan Druart daefab479c Bug 15451: Rewrite the csv profile tool script
This page was inconsistent with the other admin and tool pages.
The 2 tabs 'New profile' and 'Edit existing profile' were useless, the
ergonomic needs to be revisited.
This patch applies the same script/page structure as others: by default
a table containing all csv profiles is displayed with 2 action links:
edit and delete.

Test plan:
1/ Create 1+ CSV profiles, with different types (marc and sql)
2/ Update some values using the Edit link
3/ Delete a CSV profile

Note: When deleting a CSV profile, it would be great to warn the user if it is
used.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No problems on create, update and delete.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2017-04-30 23:20:00 +12:00
Fridolin Somers f6cac05923 Bug 18329 - Batch record deletion broken
Hie, Tools > Batch record deletion seems broken.
Any deletion returns error :
Bibliographic record YYY was not deleted. An error occurred. (The error was: {UNKNOWN}: DBD::mysql::db begin_work failed: Already in a transaction at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1560. at /home/koha/src/C4/Biblio.pm line 3468 , see the Koha log file for more information).

Looks like it is because of Bug 18242 which added a transaction  in C4::Biblio::_koha_delete_biblio_metadata : $schema->txn_do.
The script batch_delete_records created a transaction with $dbh->{AutoCommit} = 0;

This patch fixes by using also Koha::Schema in batch_delete_records to manage transaction.
It also removes "$dbh->{RaiseError} = 1", this behavior is managed in Koha::Database.

Test plan :
 - Go to Staff interface : Tools > Batch record deletion
 - Enter a few existing biblionumbers
 - Click on "Continue"
 - Click on "Delete selected records"
=> Without patch you get a DB error
=> With patch you get confirmation message
 - Try to get the biblios to confirm they are deleted : /cgi-bin/koha/catalogue/detail.pl?biblionumber=xxx
 - Test with and without Plack

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 0f32bd9999)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
2017-04-30 23:19:54 +12:00
Marcel de Rooy 6771c9bf4c Bug 12913: [QA Follow-up] Reword 'change item status' and add zero status
The zero status should be added in @notforloans. It is a legitimate status
for available books. The zero status description can be translated in
the template.
The 'Change item status' is confusing. We mean that we saw a notforloan
status that we do not expect (read: does not exist in the table).

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 664d653417)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
2017-04-24 11:07:54 +12:00
Fridolin Somers 6a713f7793 Bug 12913 - wrong inventory results when notforloan filter is not used
In inventory tool, there is a test on scanned items notforloan value : if the value is not one of authorized values, the inventory results displays the problem "Change item status".
The bug is that if notforloan filter is not used, the notforloan values array is not filled and so all scanned items are marked with this problem.

Test plan :
- Check you have in default framework an item subfield mapped with items.notforloan and with an authorized value catergory, for example 'NOTFORLOAN'
- Check the authorized value category has at least value '0'
- Create a new value in this category, for example '9'
- Create a biblio with default framework
- Create an item with barcode='000AAA1', callnumber='ZZZAAA1' and notforloan=0
- Create an item with barcode='000AAA2', callnumber='ZZZAAA2' and notforloan=9
- Delete new value created above, for example '9'
- Create a file 'barecodes.txt' containing 2 lines '000AAA1' and '000AAA2'
- Go to inventory tool : /cgi-bin/koha/tools/inventory.pl
- Choose file in 'Barcode file'
- Enter item callnumber between 'ZZZ' and 'ZZZZ'
- Check 'Compare barcodes list to results'
- Submit
=> without patch, you get 2 results with problem 'Change item status' even for item with notforloan=0
=> with patch, you get 1 result with 'Change item status' on item with notforloan=9

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 358fb744c7)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
2017-04-24 11:07:48 +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
Fridolin Somers c3dc361739 Bug 17420 - record export fails when itemtype on biblio
In Tools, Export data, you can export with a filter on item type.
When item type is on biblio record, you get the error :
export.pl: DBIx::Class::ResultSet::next(): Unknown column 'biblioitems.itemtype' in 'where clause' at /home/koha/src/Koha/Objects.pm line 150

Looks like its because this code use to be a SQL Select and is now a DBIx call :
                          C4::Context->preference('item-level_itypes')
                            ? ( 'items.itype' => $itemtype )
                            : ( 'me.itemtype' => $itemtype )

This patch corrects by using "me.itemtype"

Test plan :
- Use syspref item-level_itypes = biblio record
- Go to Tools > Export data : /cgi-bin/koha/tools/export.pl
- Select an item type
- Click on "Export biblio records"
=> Without patch you get an error 500
=> With patch you get a correct export file

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Fix the issue, no errors.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>
2016-12-12 14:54:08 +13:00
Jonathan Druart 5c324d8201 Bug 17411: Remove 3 other occurrences of exit 1
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit f1a8ca1302)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-10-22 15:19:23 +02:00
Jonathan Druart 4cf4751c08 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>
(cherry picked from commit 762415a2ff)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-09-08 21:55:45 +02:00
Jonathan Druart bb5adfa7f9 Bug 11490: Split MaxItemsForBatch into 2 prefs to clarify things
Prior to this patch, a single pref (MaxItemsForBatch) was used to
restrict the use of the batch item modification and deletion.

On the batch modification tool, the pref is used to limit the number of
items a librarian is allowed to modify in a batch, but on the batch
deletion tool the pref is used to limit the number of items to display.

To clarify things, this patch split the pref into 2 new prefs:
MaxItemsToDisplayForBatchDel and MaxItemsToProcessForBatchMod.

Later we could add a MaxItemsToProcessForBatchDel if needed.
Note that this patch should not introduce any changes in the current behaviors.

Test plan:
0/ Do not execute the update DB entry
1/ Set a value != 1000 for MaxItemsForBatch
2/ Execute the update DB entry and confirm that the 2 new prefs are
initially set with the value of MaxItemsForBatch and that
MaxItemsForBatch has been deleted
3/ Set different values for these 2 prefs (2 and 3 for instance to ease
the next steps)
4/ Try to delete less than MaxItemsToDisplayForBatchDel items
=> You must see the items details
5/ Try to delete more than MaxItemsToDisplayForBatchDel items
=> You must not see the items details but are allowed to delete them
6/ Try to modify less than MaxItemsToProcessForBatchMod items
=> You must see the items details
7/ Try to modify more than MaxItemsToProcessForBatchMod items
=> You must be restricted and not allowed to process

Signed-off-by: Laurence Rault <laurence.rault@biblibre.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 6b2c55ecdd)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-05 08:41:40 +02:00
Nick Clemens 4ba478759e Bug 16681 - Allow update of opacnote via batch patron modification tool
To test:
1 - Select a batch of patrons (via list, saved file, and cardnumber
        entry)
2 - Verify you cannot edit 'opacnotes' field, nor can you see
'borrowernotes' field in table, though you can edit it
3 - Verify editing works as expected
4 - Apply patch
5 - Select a batch via each method as above
6 - Note both note columns display in table
7 - Note that you can edit or clear the opacnotes field as expected
8 - Verify other feautres work as previously

Sponsored by:
   Lancaster Theological Seminary (https://lancasterseminary.edu/)

Followed test plan, 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: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 81a9e7c88e)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 13:31:22 +02:00
Aleisha 3e6e92ad66 Bug 9259: Ability to delete a staged file once it has been cleaned
To test:
1) Go to Tools -> Staged MARC Management and clean a file. If you have no files to clean, go to 'Stage MARC for import' and upload one to clean following the necessary steps.
2) Confirm that once the file has been cleaned, the Action column now shows a Delete button. Confirm this button only shows for cleaned files.
3) Click the Delete button.
4) Confirm that clicking Cancel exits the pop-up message and does not delete the file.
5) Confirm that clicking OK refreshes the list of staged records and the one you just deleted is no longer on it (has been deleted). You can confirm this by checking for the file in mysql (SELECT * FROM import_batches WHERE import_batch_id = X;)
6) Run prove -v t/db_dependent/ImportBatch.t (have written unit tests for CleanBatch and DeleteBatch)

Sponsored-by: Catalyst IT
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Catalyst sign off, so needs another one but YAY this is great.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 08e448ee4b)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 10:59:18 +02:00
Marcel de Rooy 58de933e03 Bug 16859: Fix wrong item field name in export.pl
Field callnumber should be itemcallnumber.
For completeness, prefixing the joined fields with items table prefix.

Note: You should expect Tools/Export to export only the items within a
given itemcallnumber range, but if the biblio has one item in that
range, the biblionumber is selected for export with ALL items. The script
is designed that way (first select biblio numbers based on criteria, and
run export with those biblio numbers).

Test plan:
[1] Select a biblio N with one item with say itemcallnumber X.
[2] Goto Tools/Export. Select biblio range N,N and no itemcallnumber
    range. The biblio should be exported.
[3] Select biblio range N,N and itemcallnumber range A,B. No output.
[4] Select biblio range N,N and itemcallnumber range A,<empty>. Output.
[5] Select biblio range N,N and itemcallnumber range <empty>,X. Output.

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 00960f1964)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 10:40:45 +02:00
Nick Clemens 0b40c96d2d Bug 16855 - Poor performance due to high overhead of SQL call in export.pl
This patch eliminates all columns retrieved in the biblioitems query
except for biblionumber.

To test:
1 - Go to tools->Export data
2 - Export using various filters and note you get expected records
3 - Apply patch
4 - Ensure exported results match results before patch

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit a44bb07b88)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
2016-08-01 10:40:14 +02:00
Jonathan Druart 908fdb572a Bug 16550: Clean the tests
- replace ok with is
- remove diag
- replace $dbh->prepare->execute->fetchrow with $dbh->selectrow_array

And remove unecessary force to scalar context in pl

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-23 17:37:48 +00:00
Marc Véron b5acde6513 Bug 16550: Can't set opac news expiration date to NULL, it reverts to today
This patch fixes an issue with the expiration dates for news always reverting to today if empty.

To test:
- Apply patch
- Go to Home > Tools > News
- Create a news item, do not set expiration date
- Verify that expiration date stays empty
- Edit this news item
- Do not set expiration date
- Verify that expiration date stays empty
- Do the same with expiration dates
- Verify that they are saved properly

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-23 17:37:48 +00:00
Jonathan Druart 7f3feacfe3 Bug 16259: Replace CGI->param with CGI->multi_param in list context - part 2
This patch is a follow-up of bug 16154.

It removes the warning "CGI::param called in list context" in the
following scripts:
admin/branches.pl
admin/categories.pl
admin/patron-attr-types.pl
admin/preferences.pl
catalogue/image.pl
circ/circulation.pl
patroncards/add_user_search.pl
serials/add_user_search.pl
tools/marc_modification_templates.pl
virtualshelves/shelves.pl

Note that the warning from catalogue/itemsearch.pl still exists (the
call to CGI->param is done from the template).

Test plan:
- Add/modify a library, patron category, patron attr type
- Update a syspref
- Set localcoverimage and call catalogue/image.pl?biblionumber=XXX
- Search for patrons in the patron cards or serials module
- Add a marc modification templates
- Add a list (shelves)

You should not get the warning in the log after all these actions.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-16 17:28:06 +00:00
Jonathan Druart 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
Marcel de Rooy 220a517c8e Bug 14686: [QA Follow-up] Move allows_add_by to Upload.pm
As requested by QA, the sub should be moved.

Test plan:
See previous patches.

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: Brendan Gallagher <bredan@bywatersolutions.com>
2016-04-27 16:14:18 +00:00
Marcel de Rooy 9eb80092e3 Bug 14686: Add Upload to Tools menu
This patch makes sure that the added granular permissions work as
advertised.

Note: The field owner was not included in the Koha::Upload->get response.
The code to verify if a user is allowed to delete an upload, is concentrated
in the template now. When get returns a Koha::Object, this check could be
relocated.

Test plan:
[1] Verify that the current user has permission for tools, or has
    at least upload_general_files.
[2] Do you see Upload in the Tools menu? Follow the link.
[3] Upload a permanent file (with a category).
[4] Do you see the Delete button in the results form?
[5] Make sure that another user has no permission to upload.
[6] Login as that user and check the Tools menu.
    Try the URL [yourserver]/cgi-bin/koha/tools/upload.pl
    You should have no access to the upload form.
[7] Enable upload_general_files for this user. Go to upload and search for
    the upload from step 3. You should not see a Delete button.
[8] Enable upload_manage for this user. Search for the upload again.
    Delete the upload.
[9] Go to upload via the Cataloguing editor (856$u plugin) or add
    parameter "plugin=1" to the URL. You should not see the Tools menu.

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

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

Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>
2016-04-27 16:14:17 +00:00
Marcel de Rooy b69402d14c Bug 14686: Add allows_add_by to check permissions for uploading
The three permissions in tools/upload-file.pl are moved to a custom
routine in DBIx's UploadedFile.pm. An additional granular permission
upload_general_files is added. (The dbrev patch will contain it too.)
At some point in time this could be moved to a Koha::Object class.

The routine is tested in Upload.t.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] If you only apply this patch, you can also test uploading with a
    user that has edit_catalogue but does not have tools or circulate.
    Upload status should say: Denied.

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

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

Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>
2016-04-27 16:14:16 +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 a159d2b067 Bug 16154: CGI->multi_param - Assign a list
This patch replaces the occurrences of
  @foo = $cgi->param('foo');
with
  @foo = $cgi->multi_param('foo');

perl -p -i -e
's/^(\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: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
Nick Clemens 9658085d1e Bug 10612: (QA followup)
Rename not_borrowered_since to not_borrowed_since

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-04-01 19:52:14 +00:00
Kyle M Hall 7f9d5b27ae Bug 10612 - Add ability to delete patrons with batch patron deletion tool
This patch adds the ability to select a patron list for deletetion
when using the Batch patron deletion/anonymization tool. It also adds
buttons to the the patron lists table to access both the batch deletion
and batch modification directly from the lists view.

This is a squash of previous patches but now adds a patron_list_id
parameter to C4::Members::GetBorrowersToExpunge and uses that routine to
fetch patrons from a list.

Test Plan:
1) Apply this patch
2) Create a list of patrons with the new Patron Lists feature
3) Try using the batch edit link form the lists table
4) Try using the batch delete link from the lists table
5) Verify previous functionality has not changed

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

Patron batches are correctly passed to the edit and delete pages.

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-04-01 19:52:13 +00:00
Jonathan Druart 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
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 1307f26bd1 Bug 5404: Move the test to a new IsMarcStructureInternal sub
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:30:09 +00:00
Jonathan Druart 2237e0f871 Bug 5404: C4::Koha - remove subfield_is_koha_internal_p
The commit b5ecefd485
Date:   Mon Feb 3 18:46:00 2003 +0000

had a funny description:
Added function to check if a MARC subfield name is "koha-internal"
(instead of checking it for 'lib' and 'tag' everywhere); temporarily
added to Koha.pm

"Temporarily", since 2003, everything is relative, isn't it? :)

The thing is that GetMarcStructure returns hash like

field_200 => {
    subfield_a => {
        %attributes_of_subfield_a
    },
    %attributes_of_field_200
}

The attributes for field_200 can be 'repeatable', 'mandatory', 'tag', 'lib'.
We don't want to loop on these values when looping on subfields.
Since there are just { k => v } with v is a scalar (string), it's easier
to test if we are processing a subfield testing the reference.

At some places, we don't need to test that, we are looping on values
from MARC::Field->subfields which are always valid subfields.

Test plan:
1/ Edit items using the batch item mod tool
2/ display and edit items via the cataloguing module.

You should not see any changes between before and after the patch
applied.

Tech notes:
We need to check what we are processing when we loop on 'subfields' from
GetMarcStructure, not from MARC::Field->subfields.

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

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-07 17:30:09 +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 85633155f4 Bug 15635: Koha::Patron::Images - Remove RmPatronImage
The C4::Members::RmPatronImage just removed a row in patronimage.
This can be accomplished using the delete method of Koha::Patron::Image.

Test plan:
From the patron defail page, try to delete the image of a patron.

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:53:01 +00:00
Jonathan Druart d8980b60ee Bug 15635: Koha::Patron::Images - Remove PutPatronImage
The C4::Members::PutPatronImage inserted/updated the image of a patron.
This can be done easily with ->find->set->store or ->new->store

Test plan:
1/ Modify the image of a patron from the patron detail page
2/ Add an image to a new patron
3/ Use the "Upload patron images" tools (tools/picture-upload.pl) to add
or modify the image of a patron
4/ Use the "Upload patron images" tools (tools/picture-upload.pl) to add
or modify the image of several patrons, using a zip file.
Stress the script trying to get as many errors as possible (wrong
cardnumber, wrong mimetype, file does not exist, etc.)
With this patch, if the cardnumber does not exist, you will get a
specific error "Image not imported because this patron does not exist in
the database"

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:53:00 +00:00
Jonathan Druart b6aa77ac49 Bug 15635: Koha::Patron::Images - Add new classes
There are 3 subroutines in C4::Members to get, add and delete patron
images:
- GetPatronImage
- PutPatronImage
- RmPatronImage

By creating these 2 Koha::Patron::Image[s] classes, we could remove them easily.

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:53:00 +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
Jonathan Druart c631fa0924 Bug 11023: Automatic item modification by age (Was Toggle "new" status")
This patch adds:
- a new DB field items.new.
- a new page to configure this new status
  (tools/toggle_new_status.pl).
- a new cronjob script (misc/cronjobs/automatic_item_modification_by_age.pl
was misc/cronjob/toggle_new_status.pl)

Why this status is useful for some libraries ?
The use cases are:
- to know easily what are the new items (with a simple sql query).
- to display an icon in the search results.
- issuing rules can be adapt for new items. Automatically (using the
  cronjob script), the status change (depending the configuration) and
  the item can be issued, for example.
- a RSS/Atom feeds can be created on these new items.

Test plan:
- log in with a librarian having the tools > items_batchmod permission.
- navigate to Home > Tools > Automatic item modifications by age (was: Toggle new status)
- click on the edit button
- there are 3 "blocks":
  * duration: the duration during an item is considered as new.
  * conditions: the status will change only if the conditions are meet.
  * substitutions: if there is no substitution, no action will be done.
    You can add some change to apply to the matching items.
    E.g. ccode=3
         new=''
    If the value is an empty string (in other words, the input does not
    contain anything), the field will be deleted.
  You can create as many rules as you want.
- test the interface : add/remove rule, conditions, substitutions,
  submit the form, edit, etc.
  (There is a looot of JS everywhere, so certainly a looot of bugs...).
- when you have your rules defined, you can now launch the cronjob
  script without any parameter.
  A report will be displayed with the matching itemnumber and the
  substitutions to apply. Verify results are consistent.
- launch the script with the -c argument and verify values have been
  modified depending the substitution rules.

Signed-off-by: juliette et remy <juliette.levast@iepg.fr>

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

Bug 11023: Add the ability to specify fields from biblioitems table.

Test plan:
Same as before but try with fields from the biblioitems table.

Signed-off-by: juliette et remy <juliette.levast@iepg.fr>

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

Bug 11023: Add UT for C4::Items::ToggleNewStatus

Test plan:
prove t/db_dependent/Items/ToggleNewStatus.t

Signed-off-by: juliette et remy <juliette.levast@iepg.fr>

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

Bug 11023: FIX - condition on biblioitems table does not work

If a rule contains a condition on the biblioitems table, the match won't
work. This patch fixes this issue.

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

Bug 11023: Use DBIx-Class to retrieve column names

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

Bug 11023: Don't use the biblioitems fields for the subtitution

It's dangerous to allow a change on the biblioitems fields with this
feature.

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

Bug 11023: Rename the duration parameter with 'age'

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

Bug 11023: The age parameter should be a number

The template should check if the age parameter is correctly filled
(should be a number).

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

Bug 11023: Change the name of the feature

Originaly this feature only permits to update the "new" field.
Now all item fields can be updated.
The name of the feature is now "Automatic item modifications by age".

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

Bug 11023 [QA Followup]

* Update DB version
* Fix capitalization error
* Rename misc/cronjobs/toggle_new_status.pl to misc/cronjobs/automatic_item_modification_by_age.pl

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

Bug 11023 [QA Followup] - Complete the renaming of "toggle new status" to "automatic item modification by age"

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

Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
2016-03-02 22:56:31 +00:00
Jonathan Druart 24e502e4a9 Bug 15923: Take the id list file into account when exporting records
When exporting records (tools/export.pl or misc/export_records.pl), a
file of ids (authid or biblionumber) can be passed to filter the
results.
Bug 14722 has broken this behavior.

Test plan:
Export records and specify a list of records to filter the results.
Prior to this patch, the record with the id 1 was exported.

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

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-02 00:43:11 +00:00
Marc Véron 59fc325310 Bug 15684: Fix encoding issues with quote upload
To verify:
- Download attachment 'Goethe with umlaut' from Bug 15684
- Go to Tools > Quote Editor
- Click "Import quotes" and select the file
- The quotes display in a table. Click 'Sav quotes'
- Result: Nothing happens

To test:
- Apply patch
- Repeat steps above
- Verify that you get a message '2 quotes saved' and that the quotes
  are added in to the quotes table
- Test with attachment 'sample quotes' as well

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

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-02-04 00:14:43 +00:00
Jonathan Druart b7b94b7012 Bug 12636: Do not display unique patron attributes in the batch patron modification
Batch patron modification should not offer to update patron attributes
with values which are designated as unique since such attributes are
required to be unique to a single patron.

Test plan:
Create some patron attributes, some should be unique.
Use the batch patron modification tool to modify patrons.
With this patch, the patron attributes marked as unique won't be
display anymore.

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-01-27 20:52:36 +00:00
Jonathan Druart bde685fa92 Bug 15542: Always display the patron's info the same way.
The patron's information displayed in the member module
(includes/circ-menu.inc and includes/member-display-address-style-*.inc)
are not always displayed the same way.
Sometimes the streetnumber is missing, sometimes it's the streettype.
Sometimes the streettype is after the address, sometimes before...

Test plan:
Go on a patron detail page, and open all the tabs on the left (Check
out, Fines, Notices, etc.)
Without this patch, the patron's info displayed will differ from one page to
another.
With this patch, they will be displayed the same everywhere.

Followed test plan, works as expected. (Tested both patches together.)
Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-01-23 19:15:08 +00:00
Marc Véron e4715453e3 Bug 15579: Fix permission for batch record modification
To test:

- For a patron, permissions 'catalogue' and 'records_batchmod' only
- Log in as this patron
- Go to /cgi-bin/koha/tools/batch_record_modification.pl

Without patch: You have no permisson to acces the page.
With patch: Page 'Batch record modification' displays as expected.

(Amended to fix wrong test plan)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-01-23 18:23:04 +00:00