Bug 17993 - Do not use modal authentication with CAS - tags

Bug 12046 corrected the fact that modal dialog does not allow to use the CAS authentication in main authentication link.
This must also be corrected in link for tags in detail page : "Log in to add tags"

Test plan :
- Enable syspref casAuthentication
- Go to OPAC
- Go to a record detail page opac-detail.pl
- Click on "Log in to add tags"
=> Without patch you get the modal login popup
=> With patch you go to the login page opac-user.pl

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17993 - Do not use modal authentication with CAS - lists

Bug 12046 corrected the fact that modal dialog does not allow to use the CAS authentication in main authentication link.
This must also be corrected in link of lists popup : "Log in to create your own lists"

Test plan :
- Enable syspref casAuthentication
- Go to OPAC
- Click on Lists > Log in to create your own lists
=> Without patch you get the modal login popup
=> With patch you go to the login page opac-user.pl

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 7550: [QA Follow-up] Resolve param warning from sco-patron-image

Resolve this warning:
  CGI::param called in list context from package C4::Service line 212, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436.

It comes from the require_params call in sco-patron-image.pl.

Git grepping on require_params tells me this:
  members/default_messageprefs.pl:my ($categorycode) = C4::Service->require_params('categorycode');
  opac/sco/sco-patron-image.pl:my ($borrowernumber) = C4::Service->require_params('borrowernumber');
  opac/sco/sco-patron-image.pl:my ($csrf_token) = C4::Service->require_params('csrf_token');
  svc/cataloguing/metasearch:my ( $query_string, $servers ) = C4::Service->require_params( 'q', 'servers' );

The only candidate for multi_param seems to be 'servers', but as we can see
this variable is a scalar. Additional servers returned by require_params are
lost. This should be solved on its own report.
So, we can safely add scalar to the params call, resolve the warning and
keep the same behavior.

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

Bug 7550: SCO - Restrict access of patron's image

With this patch if SelfCheckoutByLogin is set to 'username and
password', only the logged in user will be able to see the image linked
to his/her logged in account.
If set to "barcode" we generate a token but it can be easily generated.
You should add a warning in the about page if
SelfCheckoutByLogin="barcode" and ShowPatronImageInWebBasedSelfCheck="Show".

How I tested:
- Go to SCO
- Log - Enable self checkout, go to [Your
  Server]//cgi-bin/koha/sco/sco-main.pl
- Log in with a user 'A' who has a patron image
- Copy the address of the patron image into an other browser window
- Change the borrowernumber to on of an other user 'B' having a patron
  image
- Verify that the patron image is NOT displayed

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18152: Add tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18152 : fix unimarc label in SetMarcUnicodeFlag

The standard UNIMARC requires than the 9th character (starting from 0) in
labels must be blank (while it may be 'a' in marc21)

the problem is that C4::Charset::SetMarcUnicodeFlag (called in particular when
we import a record) always add 'a' char in the 9th label'pos whereas it should
do it just for MARC21 and NORMARC (not for UNIMARC) :

C4::Charset::SetMarcUnicodeFlag add 'a' char in the 9th label character for
MARC21 and NORMARC (it's normal), but just before doing this it call
"$marc_record->encoding('UTF-8')" which is a MARC::Record function which, when
called with 'UTF-8' parameter, do only one thing : add 'a' char in the 9th
label character

This patch only removes this incorrect function call, so, when we import a bib
record in UNIMARC : it no longer adds erroneous character (this does not change
anything for MARC21 and NORMARC because SetMarcUnicodeFlag explicitly adds 'a'
char in the 9th label for them)

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18442: Add a test

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

Bug 18442: Fix DB user loggin

Test plan:
1. Drop and recreate your db

2. Clear memcached

3. Go through the installer (to speed up this test plan install all
sample data so you dont have to create libraries, patron categories etc. later)

4. On the installer page login as the database user and notice that it
does not work on the first attempt ( you get 'Error: You do not have
permission to access this page')

5. Try logging in as database user for a second time and notice you are
logged in successfully this time

4. In staff interface create a patron account with superlibrarian permissions

5. Logout of the staff interface

6. Login as database user

7. Notice you cant log in. You get the 'Error:: You do not have permission to access this
page' error

8. Try a second attempt and notice you get the same error

9. Open the URL in a new tab and notice the staff interface appears
showing that you are logged in

10. log out and log back in as the superlibrarian user you created and
notice it works on first login attempt

11. Apply patch

12. Log out and try logging back in as database user and notice that you
can login successfully on first attempt

13. Repeat steps 1,2,3 and login as database user and notice the login
works on first attempt

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

Bug 18484 - opac-advsearch.tt missing closing div tag for .container-fluid

This patch corrects HTML validation errors by adding back a missing
</div> which was removed accidentally by Bug 9043 (2014!).

This patch also removes "border" attributes from <img> tags because the
attribute is obsolete.

To test, apply the patch and test the validity of the OPAC's advanced
search page. The only error should be one about 'Bad value
"api-server,"' which isn't really resolvable.

Signed-off-by: Barton Chittenden <barton@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 13835: Popup with searches: results hidden by language menu in footer

Language menus in pop up windows are not necessary and can hide the contents
(especially search results) on a narrow screen.

For an example, see screenshot in comment #3

This patch allows to mark pop p menus not to display the language footer.

To test:

- Reproduce issue from comment #3
- Apply patch
- Try to reproduce issue from comment #3
  -> language menu should no longer display
- Verify that language menu is suppresed in 'Add to ist' as well
  (from catalog search results, select an item, Add to:...)

Note: There will be more pop-ups with unwnated language selector.
      That can be resolved in follow up bugs.

Followed test plan which worked as intended
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18419 - Broken patron-blank image in viewlog.tt

Because of a variable name collision the blank patron image doesn't
display in viewlog.tt. This patch moves the image from the template to
the stylesheet to avoid this problem.

This patch also replaces the blank patron image PNG file with an SVG
file. SVG support is wide enough to begin using whereever possible.

This patch also removes some inline CSS from circ-menu.inc and puts it
in the global stylesheet.

To test, apply the patch and clear your browser cache if necessary.

- Enable the patronimages system preference.
- View a patron account which lacks a patron image. All views (details,
  fines, notices, etc) should show the "blank" patron image, including
  the modification log view.
- View a patron account which has a patron image and check that it still
  displays correctly in all views.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 15738: Show rental fees on OPAC summary page

This patch adds a few lines that check for a rental fee on an item. If
yes, it will show in brackets as a rental fee on the OPAC summary page.

To test:
1) Have a borrower with an overdue item accruing fines, a lost item and
an item with a rental fee. Confirm the Fines column on the OPAC summary
page now shows you what you may expect to see for each item.

Sponsored-by: Catalyst IT
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18452: Correcting 'url' to say 'URL' in catalog detail

To test:
1) Edit a record, put a URL in 856u and hit save
2) Confirm that url shows as URL in OPAC and staff client

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 15815: Reword confirm message when removing patrons from card batch

This patch rewords the confirm message when removing patrons from a card
batch.
From: "Are you sure you want to remove card number(s):1 from this
batch?"
To: "Are you sure you want to remove the selected patron(s) from this
batch?"

To test:
1) Go to Tools -> Patron Card Creator -> Manage batches
2) Edit a batch
3) Select one or more patrons and click 'Remove selected patrons' (not
individual Delete buttons)
4) Confirm the message is worded better and easier to understand
5) Click OK and confirm the patrons are deleted as expected

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 15702: Recommended Counter-patch

As per comment #7, this patch affects AddMember and ModMember.
The test plan should be the same as comment #6.
Secondary patch with tests still to come.

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 7fd248f3e9c80b36fb451eb90d2c34242c0cbb61)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 15702: Add test cases for modified code

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 31378adbe1add83afd2ac77520a295c18ba70b72)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18457: Add tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 41669b45a808550088146380d534e4f3629590d9)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Conflicts:
t/db_dependent/Letters.t

Bug 18457 - process_message_queue.pl will die if a patron has no sms_provider_id set but sms via email is enabled for that patron

If SMS via Email is enabled, and a patron has opted for SMS messages, but has not selected a service provider, the cronjob will die with the error
Can't call method "domain" on an undefined value at /usr/share/koha/lib/C4/Letters.pm line 1055.
This will cause all messages that come after the error to not be sent!

Test Plan:
1) Enable SMS via Email
2) Enable SMS for a patron, but don't set a provider
3) Perform an action that will trigger an sms message to go into
   the holds queue ( item due, item checkout, etc )
4) Run process_message_queue.pl, note the error
5) Apply the patch
4) Run process_message_queue.pl, no error this time!

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit c7541091741878d28f648df8681a691cf787334c)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

SCHEMA UPDATE for Bug 17260 - updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

Bug 18066: Hea V2 [squashed]

This patch is the Koha part of the Hea v2 project.
You can find the (testing) code for the server at
  hea-ws  - https://github.com/joubu/hea-ws/commits/v2
  hea-app - https://github.com/joubu/hea-app/commits/v2
They contain the different pull requests made over the last 6 months.

More information on Hea at https://wiki.koha-community.org/wiki/KohaUsageStat_RFC
The goal of this commit message is to provide an overview of what could
be a new version of Hea.

Prior to these changes, the Hea database was filled with 1 line per Koha
installation. System preferences were filled by the libraries and a
cronjob (share_usage_with_koha_community.pl) collected these values to send
them to a webservice (hea-ws/upload.pl).

With the need to collect more data we would want to collect data at the library
level (branch) and not at the installation level.
For instance the geolocation, the url or the country can be different from one
library to another, even if managed from the same Koha installation.
The Hea DB has been upgraded to reflect that change (see hea-app/sql/schema.sql).

The hidden goal of this patch is to make Hea sexier and explain
better to libraries how it can be useful to share their information
with the Koha community. I guess the main problem is the lack of
communication and explanations about what we are doing we these data.
To fill this gap I'd like to (TODO)
  1. Communicate on the ML about this new version of Hea (once it got
pushed and backported)
  2. Link the Privacy_Policy.md from the Hea interface
  3. Get help from a native English speaker to add
popup/help/info/whatever on "Home › Administration › Usage statistics",
to clearly explain what happens (and what will not happen!) when an option or
another is set.

You can find screenshot of this whole enhancement on bug 18066, comment 2.

What this patch does:
- Create a new branches.geolocation DB field
- Add 3 new sysprefs:
  * UsageStatsGeolocation
  * UsageStatsLibrariesInfo
  * UsageStatsPublicID
- Integrate the Leaflet JS library to get a fancy map to pick
geolocations

How does it works:
On the new administration page where statistics to share are configured,
there are several new things. It is now possible to share information either
per Koha installation or libraries. If UsageStatsLibrariesInfo is set,
the info at library level (url, name, country, geolocation) will be
sent to the Hea webservice. If it is not set, you can decide to fill
UsageStatsLibraryUrl, UsageStatsLibraryName, UsageStatsCountry,
UsageStatsGeolocation to share these information. Note that even if the
data are retrieved at installation level, it's better to fill the prefs
as well: On the Hea website the different libraries defined for a given
Koha installation could be displayed on the same page.
This page is a public page which will be attributed to every
installation (with the pref UsageStatsPublicID). On this page all the
info available publicly will be displayed.

TODO later:
- Add a button on the administration page to delete the info shared
publicly. It will be easy to show that the info are no longer displayed
on the public page.
- Add an icon per Koha installation to get a better "public page"
- Any suggestions?

Test plan:
We will need to test hea-ws, hea-app and the Koha-side code to test the
whole enhancement.
1/ To start, clone the hea-ws and hea-app project and checkout the
'master' branch (*not* 'v2')
2/ Create the hea database and user
  CREATE DATABASE hea
  CREATE USER 'hea'@'localhost' IDENTIFIED BY 'hea';
  GRANT ALL PRIVILEGES ON hea.* TO 'hea'@'localhost';
  FLUSH PRIVILEGES;
3/ Fill the DB with some data
  mysql hea < hea-app/sql/schema.sql
  mysql hea < hea-app/sql/sql/mock-data.sql
4/ Checkout the 'v2' branch for both hea-ws and hea-app
5/ Execute the upgrade DB script
  % cd hea-app
  % perl -p -i -e 's/REPLACE_ME/hea/' sql/upgrade.pl # Fill the DB info
  % perl sql/upgrade.pl
Now the DB is using the v2 structure. That means we have 1 installation
row per library previously defined. 1 library row has also been created.
5/ Configure hea-ws
% echo '192.168.50.1 hea.koha-community.org' >> /etc/hosts
<VirtualHost *:80>
  DocumentRoot "/path/to/hea-ws"
  ServerName "hea.koha-community.org"
  <Directory "/">
    Options +ExecCGI
    Require all granted
    AddHandler cgi-script .pl
  </Directory>
</VirtualHost>

And enable it with a2ensite, then restart apache.
The copy the database.yml.sample to database.yml and edit it to fill the
DB info.

6/ Launch the hea-app
  % cd hea-app
  % edit README.md # to install the missing modules
  % cp environments/config.yml environments/development.yml
  % edit environments/development.yml # to fill the DB info
  % perl bin/app.pl
Then hit localhost:3000
You should see a local version of Hea with sample data

7/ Back to Koha side
A. We will test that the webservice still works with previous version of Koha (without v2)
a. Do not configure Hea
  % perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
Then hit localhost:3000
=> Nothing added
b. Configure Hea on admin/usage_statistics.pl
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> New library added
c. Modify the Hea configuration
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> Info are modified

B. Not we will test that it works with the new version (much more fun ;))
% git checkout hea-v2 # koha
a. Configure Hea using /admin/usage_statistics.pl
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> Check the result on localhost:3000
b. Share libraries's info
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
c. Continue to play a bit and share the info.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 14608 - HEA : add possibility of sharing usage statistics [squashed]

This patch set adds:
- a reference to Hea at the end of the installation process
- a link to the new page from the admin home page
- a new page to easily configure shared statistics

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Update release notes to 16.05.12 release.

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: Mason James <mtj@kohaaloha.com>

Bug 18242: 16.05 version - 16.11.x adaptation - Old::Checkouts vs OldIssues

Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18242: 16.05 version - [SOLUTION 2]Handle correctly move to old_issues

The table old_issues has a primary key defined on the issue_id column.
This issue_id comes from the issues table when an item is checked in.

In some case the value of issue_id already exists in the table

Basically this happens when an item is returned and mysqld is restarted:
The auto increment value for issues.issue_id will be reset to
MAX(issue_id)+1 (which is the value of the last entry of old_issues).
See also the description of bug 18003 for more informations.

In this solution the change is done at code level instead of DB
structure: If old_issues.issue_id already exists before moving from
the issues table, the issue_id is updated (not on cascade for
accountlines.issue_id, should it?) before the move.

Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18242: 16.05 version - [SOLUTION 2]Add tests

Signed-off-by: Mason James <mtj@kohaaloha.com>

Revert "Bug 18329 - Batch record deletion broken"

This reverts commit f6cac0592397ae829e34a576173fd0c86980d1bc.

Fix t/db_dependent/HoldsQueue.t tests

FIX CSRF for opac-memberentry.pl

Bug 18094: Only search in searchable patron attributes if searching in standard fields

Test plan:
- Add a new patron attrbute and mark it searchable
- Populate a new patron with 'potato' in that field
- Add/edit another patron to have email potato@invalidemail.com'
- Perform a patron search with query 'potato' (in standard fields)
=> Both patrons are returned
- Perform a patron search with filters 'Email' and query 'potato'
=> Only 1 patron is returned and you are redirected to the patron detail page.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18094: Add tests to highlight the problem

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

revert buggy CSRF in opac/opac-memberentry.pl

Revert "Bug 18307 - Branchname is no longer displayed in subscription tab view"

This reverts commit 719dc345f4d38b1e34ef2318f472e4757709a647.

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>

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>

Bug 17109: [QA Follow-up] Die when wrong token

Removes template var csrf_error and associated handling.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Restested with opac and intranet: Still sends or dies elegantly..

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

Bug 17109: Use Koha.Preference in sendbasket template

No need to send OPACBaseURL to the template, if you load the Koha TT
plugin inside the template.

Test plan:
Send a few items in your cart from OPAC and intranet.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 17109: Add CSRF token to [opac-]sendbasket

If you have no (valid) token, you will not be able to send the message.

Test plan:
[1] Verify if you can still send the cart from opac and intranet.
[2] While still being logged in, try to send the cart from opac by
    using the following URL:
    /cgi-bin/koha/opac-sendbasket.pl?email_add=you@somedomain.com&comment=csrf_test&bib_list=doesnotmatter&csrf_token=justsomeguess12345
    This should now result in a csrf error.

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>

Bug 17109: Remove second authentication from (opac-)sendbasket

Patch deals with opac and intranet variant.
If we authenticated the first time, it is not necessary to do it
a second time rightaway.

Replaces a call to get_template_and_user (including checkauth) by
gettemplate.

Also removes duplicate use C4::Biblio statements.

Test plan:
[1] Put a few books in the cart.
[2] Send the cart from OPAC.
[3] Send the cart from intranet.

Tested 3 patches 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>

Bug 18124: add to members/deletemem.pl

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>

Bug 17097: here the var is 'member', not 'borrowernumber'

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

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>

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>

t/Token.t merge typo fix
modified:   t/Token.t

Bug 18124: [16.05.x] remove HouseboundModule code

modified:   members/memberentry.pl

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>

Bug 17110: Add unit test for MaxAge parameter in Token.t

Test plan:
Run t/Token.t

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>

Bug 17110: Lower CSRF expiry in Koha::Token

Default expiry in WWW:CSRF is one week.
This patch sets it to 8 hours by default in Koha, and allows to
change the expiry period individually by passing MaxAge.

Test plan:
[1] Put items in your cart.
[2] Apply the example patch too.
[3] Send the cart from opac within the allotted 10 seconds.
[4] Send again, but wait some 10 seconds before submitting. Too late!

Tested 3 patches 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>

Add release notes to 16.05.12 release

Translation updates for Koha 16.05.12

Revert "Bug 18094: Only search in searchable patron attributes if searching in standard fields"

This reverts commit b696300c3a2ddbd670775fe22a0cb95b61751abf.

Revert "Bug 18094: Add tests to highlight the problem"

This reverts commit dde4036df4fba9b8d3652a78c320c42f9e71c660.

fix t/db_dependent/HoldsQueue.t tests

Signed-off-by: Mason James <mtj@kohaaloha.com>

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>

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>

Bug 18087 - Clarification on File type when using file of biblionumbers to export data

Added a line to the screen  detailing the types of files that can be used to upload a list of biblionumbers.

Test plan:
1. Go to tools->export data
2. under File option,  (File types accepted: .csv and .txt)  should appear
3. Check both Bibliographic and Authority Export screens for this new string.

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
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>

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

Bug 15451: (followup) fix filename extension for csv file

This patch does the same as basket/downloadcart.pl
to set '.csv' as filename extension for downloadshelf.pl

To test:
1) Define a CSV MARC profile
2) On staff download a list, extension is '.NN'
with 'NN' the CSV profile id.
3) Apply the patch
4) Download again, check extension is now '.csv'

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

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>

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>

Bug 15451: Koha::CsvProfiles - Remove GetCsvProfile

This subroutine just returned a csv profile for a given id.
It is replaced in this patch by a call to Koha::CsvProfiles->find.

There is nothing to test here, these changes have been tested in
previous patches.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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>

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>

Bug 15451: Koha::CsvProfiles - Remove GetCsvProfilesLoop

This subroutine returned the csv profiles for a given type.
This could be done easily with the new Koha::CsvProfiles->search method.

Test plan:
To do at the OPAC and staff interface!
1/ Export a list using a CSV profile
2/ Export your CART using a CSV profile
Note that only MARC profiles should be available.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested on staff/opac & cart/list
Small problem on filename extension fixed in followup.
No errors

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

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>

Bug 15451: Add the 2 new modules Koha::CsvProfile[s]

There are based on Koha::Objets. Tests provided.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test pass, no errors.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18443: Get rid of warning 'uninitialized value $user' in C4/Auth.pm

When logging out from OPAC, plack-error.log log and/or opac-error.log
complain about 'uninitialized value $user' in C4/Auth.pm line 187. The
warning is not necessary, this patch removes it.

To test:
- try to trigger warning
- apply patch
- verify that warning no longer occurs
- prove t/db_dependent/Auth.t
- verifiy that SCO still behaves like before (especially if
  you break out from sco path)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18429 - Receiving an item should update the datelastseen

To test:
0 - Ensure AcqCreateItem is set to 'placing an order'
1 - Order some items, note entrydate and datelastseen match
2 - Alter those dates to be earlier than today (or wait some days)
3 - Recieve the item and note datelast seen not updated
4 - Apply patch
5 - Repeat 1-3
6 - Date last seen should be updated.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Works as expected.
Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18335 - Check in: Make patron info in hold messages obey syspref AddressFormat

When checking in, information about patrons with a hold always display
in an US style. Address information should opey the system preference
AddressFormat

To reproduce:
- Set syspref AddressFormat to German style
- Check out an item to a patron A
- Put a hold on this item for patron B
- Check in item using the Check in field in page header
- Result: In message 'Hold found', address does not display in German
  style

To test:
- Apply patch
- Repeat steps above
- Verify that address displays as expected

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18460: Fix undefined itemtype warning in Serials.t

This patch makes the test create an itemtype, and use it for the created item so there's no warning.

To test:
- Run:
  $ prove t/db_dependent/Serials.t
=> FAIL: item-level_itypes set but no itemtype set... warning raised
- Apply the patch
- Run:
  $ prove t/db_dependent/Serials.t
=> SUCCESS: Tests pass and no warning is raised
- Sign off :-D

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>

Bug 12021 - SIP2 checkin should alert on transfer and use CT for return branch

This small patch corrects the SIP2 checkin response if branch transfer is needed.
It depends on bug 7981 and also removes an obsolete alert check in SIP2.

Test plan:
 - apply bug 7981
 - make sure syspref AutomaticItemReturn is false
 - check 'Return policy' in 'Default checkout, hold and return policy'
 - make a SIP2 checkout and checkin from a branch different than item's home branch.
 - verify that SIP2 response gives alert CV04 (=send to different branch)
 - also verify that response field CT gives correct branch according to 'Return policy'

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

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>

Bug 18423 - Add 'Add child' button to files, housebound, and deletemem pages

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

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>

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>

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>

Bug 17911: Message and timeout mismatch

The installation step 3 has a meta refresh of 10 seconds,
but the message says 5 seconds. People could get impatient.

TEST PLAN
---------
1) Fresh install
   -- notice the discrepancy on the web installation.
2) drop and create the DB
3) apply the patch
4) run the web install
   -- notice it refreshes faster (5 seconds vs 10 seconds)
5) run koha qa test tools

Works as intended and passes qa test tool
 OK koha-tmpl/intranet-tmpl/prog/en/modules/installer/step3.tt
  OK    forbidden patterns
  OK     git manipulation
  OK      spelling
  OK   tt_valid
  OK    valid_template

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17814: koha-plack --stop should make sure that Plack really stop

koha-plack --stop doesn't ensure that Plack was really stopped before
returning

So in case that koha-plack --stop was quickly follows by koha-plack
--start (e.g. logrotate), it could leave Plack in stop state. This is
due to koha-plack --start think that Plack was already started, while it
actually is being stopped.

For me I think this is the cause why I got random Plack stop when
logrotate is run.

It should be similar to the case of Zebra in Bug #16885, and the
solution might be the same: adding "--retry=TERM/30/KILL/5;" to
start-stop-daemon command.

TEST PLAN
---------
1) Login to staff client and do something that will hold connection for
a long time, e.g. a batch import or a slow report.
2) sudo koha-plack --stop mykoha
3) ps aux | grep plack   <-- a Plack process will still running work in
1)
4) wait for 1) to finish and all Plack processes exit
5) sudo koha-plack --start mykoha
6) apply the patch
7) repeat step 1)-2)
8) ps aux | grep plack   <-- There should be no Plack process running
now
9) Note that work in step 1) might get terminated midway. Since we force
Plack to stop after some wait.

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>

Bug 18364: [Follow-up] Also add an environment variable to prevent locking

The test in SendCirculationAlert is extended by adding an env var
called KOHA_NO_TABLE_LOCKS. If this var is set to a true value,
the table locking is skipped too.

This is useful when running a test without prove. The variable could be
set in a shell profile.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18364: Do not LOCK/UNLOCK tables from tests

From the MySQL doc:
"LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables."
If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed.
To avoid that we need to guess if this code is execute from testsa or not (yes it is a bit hacky)

Better ideas are welcome!

Another fix would have been to revert
  commit be156d9ad9e5bcfadab34d44f90e04fd61e256ad
  Bug 15854: Use a READ and WRITE LOCK on message_queue
but theorically a race is still possible.

Existing tests seem to be safe, to test this patch you will need new
tests from bug 17964.
Test plan:
  prove t/db_dependent/Letters/TemplateToolkit.t
twice, and notice that changes have been comitted.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18439 - Resend button for notices being hidden by CSS and never unhidden

Current jQuery is not unhiding resend button, this patch adds an id
and a toggle command

To test:
1 - Enable EnhancedMessagingPreferences
2 - Find a patron with sent or failed messages
3 - Note you can expand message, but not resend
4 - Apply patch
5 - Note that on expanding message a 'Resend' button appears
6 - Ensure resend button operates as expected

Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18001 - LocalHoldsPriority can cause multiple holds queue lines for same hold request

If LocalHoldsPriority is enabled, and a record level request has more
than one item that could fill that hold, there is a possibility that a
holds queue row will be generated for the request for each of the
available items!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18001 - Unit Test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18372 - transits are not created at check in despite user responsing Yes to the prompt

If the AutomaticItemReturn system preference is set to "Don't" and you
check in an item which belongs to another library, a dialog will ask you
if you want to transfer. This patch corrects a bug to enable the "Yes"
buttons to work, triggering the transfer.

To test, apply the patch and check in an item which belongs to another
library. In the dialog, click the 'Yes' button and confirm that the item
has been made 'in transit' to the correct library.

Perform the same test with the 'Yes, print slip' and 'No' buttons and
confirm they are working correctly.

Since this patch changes the behavior of many other popups triggered
from this template, test other operations which trigger a 'print slip'
option like:

- Checking in an item which is part of a rotating collection
- Checking in an item which is on hold for a patron

Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17821 - due date in intranet search results should use TT date plugin

Intranet search results displays due date from item onloan.
This should use the TT date plugin.

Test plan :
- set syspref dateformat not on yyyy-mm-dd, for example dd/mm/yyyy
- checkout an item
- at intranet, perform a search where you see the item
=> You must see : "date due : dd/mm/yyyy"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 12930: Web installer does not show login errors

Implemented invalid_username_or_password template
variable being handed to Auth.tt. Removed indentation
changes and modification to return value of checkpw

Test plan:
See comment #2

Alternative test plan:
- Clear session cookies or close / open your browser
- Go to  [MY SERVER]/cgi-bin/koha/installer/install.pl
- Verify that message appears if wrong username/pw provided
- Verify that you can log in with databas administrator account

Tested with alternative test plan, works as expected. Title and
description in commit message adapted.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 87eb80136d49451d0a9cc25e68db452a33f1771a)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18373: Re-add UpgradeBackup.pm

Bug 18028 removed the install_misc directory but
install_misc/UpgradeBackup.pm was still used by the 'upgrade' rule of
make.

Other files from install_misc were useless to it may be better not to
reintroduce this directory with only 1 file.

Test plan:
`make`
`sudo make install`
`make upgrade`

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 17872: Fix small error in GetBudgetHierarchy and one of its calls

In aqbudgetperiods.pl a commented line is removed that contains a
wrong userenv hash key. Should be branch instead of branchcode.

In aqbudgets GetBudgetHierarchy is called with the same wrong userenv
hash key. Should be userenv->{branch}.
This made another bug visible: if you call GetBudgetHierarchy with a
branch and without owner, the where clause should take into account
that the branchcode can be empty (not null).

Test plan:
[1] Run Budgets.t
[2] Run aqbudgets.pl from Administration
[3] Add the show_mine=1 parameter in the URL
[4] Change owner of one of the funds and repeat step 2 and 3.

t/Budgets.t returns green. 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: Mason James <mtj@kohaaloha.com>

Bug 14932: Do not call can_edit_subscription with an empty value

ref($sub) eq 'ARRAY' should always be true, what we want is to call
can_edit_subscription if there is at least 1 serial for this
subscription.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 14932 - serials/serials-collection.pl-page is very slow. GetFullSubscription* checks permission for each serial!

Currently we have subscriptions with 300+ serials received.
It takes a lot of time to show serials-collection.pl (~20s).
This is especially troublesome when receiving serials, since after receival we get redirected to that page.

We no longer can receive daily serials in the allotted timeframe.

This quick and dirty fix prevents checking the subscription editing for each serial, but instead checks it for the first serial only.

This reduced page load time by ~18s

TEST PLAN:
1. Receive ~300 serials (or just a bunch :)  )
2. Observe the gradual slowing of the receival action.

AFTER THIS PATCH:
1. Receive ~300 serials more (or just a bunch)
2. Observe a significant performance improvement.

Signed-off-by: Paul POULAIN <paul.poulain@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: [QA Follow-up] Add four missing error messages

The error codes come from CanBookBeIssued.
The warnings speak for themselves. Note that the GNA message is similar
to the one used in opac-user.tt (gonenoaddress is translated to the patron
as contact information not up-to-date).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: Remove useless info in alert message

Empty tags should be removed, and message id
Note that not all error codes are covered here.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: If a confirmation is required, consider as blocker

Only RENEW_ISSUE should not be considered as blocker, others should.
Note that this code is not robust and the whole script as well as the
return of CanBookBeIssued should be rewrittent completely.
We need a small patch to easy backport to stable releases

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: $borrower is a hashref

I guess this code was never called before...

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>

Bug 18349: SCO - Do not trust the confirmed flag

The "confirmed" flag is used to know if that user confirmed a situation
that needs a confirmation.
But if the issue/renew is impossible the CanBookBeIssued and the
'impossible flags' should be checked.

Otherwise a patron can checkout and renew bypassing the circulation
rules (Understand 'no limit' here...)

Test plan:
Want to renew?
Checkin $barcode, then
/cgi-bin/koha/sco/sco-main.pl?patronid=$cardnumber&barcode=$barcode&confirmed=1&op=checkout

Want to bypass the checkin?
Same url...

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(cherry picked from commit 85bd15a83ffdab0c2e28eae54e50ce4dee9e608b)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Conflicts:
opac/sco/sco-main.pl

Bug 17309 - Renewing and HomeOrHoldingBranch syspref

The AddRenewal subroutine currently uses the circulation rules for the branch
stored in the Issues table (which is the holding branch) when calculating the new due date.

This patch replaces using the branch from the Issues table with the branch
specified by the HomeOrHoldingBranch syspref.

To test:
1. Set up 2 branches, Branch1 and Branch2
2. Set up a loan rule in Branch1 for DVDs with a 21 day loan period and a
   21 day renewal period.
3. Set up a loan rule in Branch2 for DVDs with a 14 day loan period and a
   14 day renewal period.
4. Checkout a DVD belonging to Branch1 while logged into Branch2. It will
   receive the correct 21 day loan period.
5. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 14 day due date, rather than 21 days.
6. Checkout a DVD belonging to Branch2 while logged into Branch1. It will
   receive the correct 14 day loan period.
7. Renewing the same DVD while logged into either Branch1 or Branch2
   will give a 21 day due date, rather than 14 days.
8. Apply the patch and repeat steps 4-7. The correct due date should be given
   when the item is renewed, regardless of where it is checked out or renewed.

This update removes reassignment of $branch variable.

Signed-off-by: Cédric Vita <cedric.vita@dracenie.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 7d35bdf58a0bcebab20df47b50b249cafe12ddb6)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

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 0f32bd99997c82cf7b70eef32818dc817cbff64b)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Bug 18266: Fix internal error when paying fine for lost item without.. item

If a fine is created for a lost item but the itemnumber is not supplied,
the system will return it.
The item should not be mark as returned if there is no item linked to
the fine.

Test plan:
1. Turn StoreLastBorrower on
2. Create a manual invoice for a lost item, do not supply a barcode
3. Pay the fines 'Pay fines > Pay'

=> Without this patch applied you get
Can't call method "last_returned_by" on an undefined value at
/home/marc/koha/C4/Circulation.pm line 2188.

=> With this patch applied, you must not get the error.

Bug 17346: Make checkin column hidable

To test:
1) Go to columns_settings.pl --> Circulation --> issues-table and note that you cannot change the settings for checkin.
2) Go to columns_settings.pl --> Patrons --> issues-table and note that you cannot change the settings for checkin.
3) Apply the patch.
4) Go to both tables again and note that you can now change the settings.

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 9098a4d3fa7268bf11ab108693192070c44c9eaf)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>