Bug 14424: Tools Help Files for 3.20

This patch updates and adds help files to 3.20+

To test:

* Visit batch record modification and note that there is a help file
 and confirm the text is right
* Visit export data, import borrowers, stage marc for import, and log viewer
  * Confirm updated text is right

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14424: Admin Help Files for 3.20

This patch updates some of the help files for Admin areas in 3.20+

To test:

* Visit
  * Frameworks, add field, add subfield
  * Column settings
  * Patron attributes
  * Circ rules
* Confirm help loads up and is right

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 11458: Improve confusing description of syspref 'gist'

The description of "gist" was:

"Default tax rates are ... (enter in numeric form, 0.12 for 12%.
First is the default. If you want more than 1 value, please
separate with |) "

The doubled use of "default" is confusing here.

With the patch it reads:

Tax rates are ... Enter in numeric form, 0.12 for 12%.
The first item in the list will be selected by default.
For more than one value, separate with | (pipe)

To test:
- Verify that the gist system preference description is
  correct.

The use of "default" is confusing here.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14412: SQL injection possible

There is a SQL Injection vulnerability in the
/cgi-bin/koha/opac-tags_subject.pl script.

By manipulating the variable 'number', the database can be accessed
via time-based blind injections.

The following string serves as an example:

/cgi-bin/koha/opac-tags_subject.pl?number=1+PROCEDURE+ANALYSE+(EXTRACTVALUE(9743,CONCAT(0x5c,(BENCHMARK(5000000,MD5('evil'))))),1)

To exploit the vulnerability, no authentication is needed

To test
1/ Turn on mysql query logging
2/ Hit /cgi-bin/koha/opac-tags_subject.pl?number=1+PROCEDURE+ANALYSE+(EXTRACTVALUE(9743,CONCAT(0x5c,(BENCHMARK(5000000,MD5('evil'))))),1)
3/ Check the logs notice something like
  SELECT entry,weight FROM tags ORDER BY weight DESC LIMIT 1
  PROCEDURE ANALYSE
  (EXTRACTVALUE(9743,CONCAT(0x5c,(BENCHMARK(5000000,MD5('evil'))))),1)
4/ Apply patch
5/ Hit the url again
6/ Notice the log now only has
   SELECT entry,weight FROM tags ORDER BY weight DESC LIMIT 1

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Confirmed the problem and the fix for it.
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14418: More XSS vulnerabilities in opac-shelves.pl

To test:
1/ Hit a url like
/cgi-bin/koha/opac-shelves.pl?viewshelf=7&op=modif&display="><script>alert('oh
noes')</script>  Where the id is a valid shelf id
2/ Notice the js is executed
3/ Apply patch
4/ Reload page
5/ Notice input is now escaped on display

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Tested in Debian, couldn't reproduce the alert in Iceweasel, but in
Chromium. Patch fixes it.
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14418: XSS flaw in opac-shelves.pl

To test:
1/ Create a list and add at least one item to it
2/ Hit a url like http://192.168.2.18/cgi-bin/koha/opac-shelves.pl?viewshelf=7&sort=author&direction=%22%3E%3Cscript%3Ealert%28%27oh%20noes%27%29%3C/script%3E
  Where the shelf id is the number of the list you created, notice the js is executed
3/ Apply the patch
4/ Reload the page notice the js is now escaped

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14418: XSS Vulnerabilities in OPAC search

Fix for /cgi-bin/koha/opac-search.pl

To test

1/ Hit /cgi-bin/koha/opac-search.pl?tag="><script
src='http://cst.sba-research.org/x.js'/>&q=a
2/ Notice the js is executed
3/ Apply patch
4/ Reload page, notice it is no longer executed
5/ Test the rss links work still

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Confirmed bug and that the patch fixes it.
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14416: Stored XSS vulnerability - add biblio to shelf (intranet)

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14416: (follow-up) opac addbybilionumber

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14416: Stored XSS vulnerability

opac-addbybiblionumber.pl is also vulnerable because it doesn't escape
list names.

To test
1/ Create a malicious list name
2/ Try to add a biblio to the lists
3/ Notice js is excuted
4/ Apply patch
5/ Test again

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14416: Stored XSS vulnerability

The affected page in the OPAC client is:
http://testbox:9001/cgi-bin/koha/opac-shelves.pl
the vulnerable parameter: addshelf

The affected page in the STAFF client is:
http://testbox:9002/cgi-bin/koha/virtualshelves/shelves.pl

To test:
1/ Create a shelf in the opac that contains some malicious js
 eg Bad stuff <script>alert('oh noes');</script> as the name
2/ Go to /cgi-bin/koha/virtualshelves/shelves.pl in the staff client
  Note the js is executed
3/ View
http://192.168.2.18:8080/cgi-bin/koha/svc/virtualshelves/search?template_path=virtualshelves/tables/shelves_results.tt&type=1
  Notice the html is not escaped
4/ Apply patch
5/ View
http://192.168.2.18:8080/cgi-bin/koha/svc/virtualshelves/search?template_path=virtualshelves/tables/shelves_results.tt&type=1
  Notice the html is now escaped
6/ View /cgi-bin/koha/virtualshelves/shelves.pl - no more exploit

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 13993: DBRev 3.21.00.009

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 13993: Clarify test messages

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 13993: (3) Transfer order leaves incorrect orderstatus

11) Apply patch (3)
12) Log in to staff client
13) Acquisitions
14) Create a basket for two different vendors
15) Place an order in one vendor's basket.
16) Transfer the order to the other vendor's basket.
17) prove -v t/db_dependent/Acquisition/TransferOrder.t
    -- This should succeed without intervention.
18) Run koha qa test tools for the last 3 commits.

Signed-off-by: Indranil Das Gupta <indradg@gmail.com>
Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 13993: (2) Correct poorly transferred orders

Added Atomic Update to fix poorly transferred orders

TEST PLAN
---------
8) Apply patch (2)
9) Run the database updates
   $ ./installer/data/mysql/updatedatabase.pl
   -- This should run without error
10) prove -v t/db_dependent/Acquisition/TransferOrder.t
    -- This should fail, because the transfer function is still
       not fixed.

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 13993: (1) Add tests to confirm 'new' and 'cancelled'

This adds 2 tests to t/db_dependent/Acquisition/TransferOrder.t
in order to confirm the order's status is properly marked.

TEST PLAN
---------
1) Log into staff client
2) Acquisitions
3) Create a basket for two differing vendors.
4) Place an order in one of the baskets.
5) Transfer the order from one vendor's basket to the others.
6) Apply this patch (1) only
7) prove -v t/db_dependent/Acquisition/TransferOrder.t
   -- should fail one test: not marked as 'cancelled'.

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14215: Change the 'delimiter' syspref description for its wider use

Patch changes 'report files' to 'CSV files' as there are more
options now for downloading and creating CSV files where this
preference is taken into account.

To test:
- Verify the changed system preference description for
  'delimiter' is correct.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 13904: Make unimarc_field_4XX displays usefull 200 subfield data

When searching for multivolumes titles, UNIMARC 4xx field plugin
displays the title of the biblios (200$a), without giving info about
volumes (200 $h $i). It neither doesn't display $e (subtitle) info which
could greatly help to disambiguate search result.

The displayed title is supposed to link to a biblio record view (MARC /
normal). It doesn't work.

TO TEST:

- On a UNIMARC Koha, add a new biblio record
- Call the 4XX plugin from 461/463 field
- Search for a biblio record which contains 200$e, and/or 200$h and/or
  200$i subfields.
- You get a result list, with two issues:
  1. $a, $h & $i aren't displayed
  2. Biblio title is not a link
- Apply the patch, and repeat previous steps.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
I got a link on fulltitle, but in 'ahie' order (not aehi)
Fixed some tabs.

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 4137: Fix the OPACViewOthersSuggestions behavior

This pref does not work at all, the interface let the user choose to
list all suggestions, but whatever he chooses the suggestion list is the
same.

This patch cleans a bit the suggestedby management.

There are a lot of cases to test, because linked to 2 prefs:
 AnonSuggestions and OPACViewOthersSuggestions.
1/ AnonSuggestions = 0 and OPACViewOthersSuggestions = 0
  - A non logged in user is not able to make a suggestion.
  - A logged in user is not able to see suggestions made by someone else.
2/ AnonSuggestions = 0 and OPACViewOthersSuggestions = 1
  - A non logged in user is not able to make a suggestion.
  - A logged in user is able to see suggestions made by someone else.
3/ AnonSuggestions = 1 and OPACViewOthersSuggestions = 0
  - A non logged in user is able to make a suggestion.
  The suggestedby field will be filled with the AnonymousPatron pref value.
  He is not able to see suggestions, even the ones made by AnonymousPatron.
  - A logged in user is not able to see suggestions made by someone else.
4/ AnonSuggestions = 1 and OPACViewOthersSuggestions = 1
  - A non logged in user is able to make a suggestion.
  He is able to see all suggestions.
  - A logged in user is able to see suggestions made by someone else.

In all cases a logged in user should be able to search for suggestions
(except if he is not able to see them).

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
All use cases tested, work as expected
No errors

Only comment is perhaps (in the future) a gracefull failure
when AnonymousPatron is not set, or has '0' value

Message is DBIx::Class::ResultSet::create(): Column 'suggestedby' cannot be null at ...

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 10866: Hide patron's history if intranetreadinghistory is set to not allow

If set to "not allow", the intranetreadinghistory pref prevent staff
members to access patron's checkout history.
But:
1/ The page is still accessible if you know the url
2/ The history can be consulted on the item history page

Test plan:
0/ Don't apply this patch
1/ Set the intranetreadinghistory to allow
2/ Go on a patron's checkout history page
3/ Open a new tab and go on a item's checkout history page
4/ Set the intranetreadinghistory to not allow
5/ Refresh both pages => no change
6/ Apply this patch
7/ Refresh both page.
On the first page, you should see a warning
On the other one, you should see that the patron column is not displayed
anymore.

Followed test plan, results were as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
http://bugs.koha-community.org/show_bug.cgi?id=10886
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Nice addition!
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14403: Remove warn in Koha::NorwegianPatronDB

Line 99 has an unconditional warn, left over from development:

warn "$combined_username => $combined_password";

This patch deletes the line i question.

To test:
No testing needed, just have a look at the diff and see that
it makes sense to delete the warn.

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 13427: jQuery Timepicker is not translated on returns page

The returns page was missing an include with the translated strings.

To test:
- Install an additional language, like de-DE
- Confirm the bug on the returns page
  - Make sure SpecifyReturnDate is activated
  - Open the datepicker, look at the time settings
- Apply the patch
- Reinstall the language, no update of the po files is needed
- Retest
- Verify, that now the time settings are translated

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

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 11467: Bug Untranslatable srings in opac-detail.tt (IDreamBooks*, OpacBrowseResults)

Patch marks several strings in the Javascript on the OPAC detail
and result page for translation.

1) IDreamBooks*
- Activate the 3 IDreamBooks* system preferences
- Check the 'cloud' and additional content shows up correctly on
  the detail and result pages
- Verify everything works as expected and the same as without the patch

2) OpacBrowseResults
- Activate OpacBrowseResults
- Do various searches
- Verify the nex, previous, browse result list features still
  work the same as without the patch

Bonus: Check new strings appear in the .po files by updating one
       language with the patch applied (perl translate update de-DE)

NOTE: Really should have read the test plan more closely.
      I couldn't find the 'Go to detail:' section, until I clicked
      'Browse results'.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 11011: Rephrasing 'in keyword' in OPAC authority search

Using 'in the complete record' rather than 'in keyword'. I think this fits well as it seems that this means the search looks anywhere in the record.

To test:

1) In the OPAC, click on Authority Search
2) Notice that in the drop-down menu for the 'Where:' field, there is an 'in keyword' option.
3) Apply patch
4) Now says 'in the complete record'

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 8686: Raise required version of URI::Escape to 3.31

Raises the minimum required version of URI::Escape from
1.36 to 3.31.

TEST PLAN
---------
1) git branch -b bug_8686 origin/master
2) ./koha_perl_deps.pl -a | grep URI
   -- it will list 1.36 required
3) git bz apply 8686
4) ./koha_perl_deps.pl -a | grep URI
   -- it will list 3.31 required
5) koha qa test tools

NOTE: Also default in Ubuntu 14.04 LTS,
      not just Wheezy as noted in comment #15.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signoff based on Nicole's comment (bug 9990 comment 6):
"This stops happening if you upgrade URI::Escape to
3.31.  We should make it clear in the Perl Modules page that an upgrade
is needed."
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14070: Prevent to save a letter if an error occurred

On saving a letter, if the title or the content of a template is not
defined, a JS alert is raised. But the form is submitted anyway.
This patch prevent the form to be submitted.

Test plan:
- Create or edit a letter
- Fill the title for a template, not the content
- Save
- Confirm you get the alert and that the form is not submitted

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

Signed-off-by: Indranil Das Gupta <indradg@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14351: Remove given-when from opac-search.pl

Reformats given-when to if-elsif-else in opac-search.pl
to remove the experimental feature and with it a lot
of warnings from the logs.

To test:
- Do several different advanced searches with and
  without expanded search options
- Verify the link back to the search appears above
  the results list and works correctly

See also: test plan on bug 13307

NOTE: Even installed firefox plug in to edit cookies to
      trigger else case. :)

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14350: Missing statement in kohastructure.sql - DROP TABLE IF EXISTS borrower_sync

Reported by Jonathan on bug 11401:

DROP TABLE IF EXISTS borrower_sync;
is missing in installer/data/mysql/kohastructure.sql

To test:
- Run the web installer and confirm all tables are
  created correctly

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14280: Add branches fields to discharges letters

Currently, when generating a discharge letter, branches fields are not translated in the letter (<<branches.branchname>>, <<branches.branchaddress1>>, etc.)
This patch fixes that.

How I tested:
- Set syspref 'useDischarge' to 'allow'
- Go to Home > Tools > Notices & Slips
- Edit DISCHARGE, add to 'Email message':
<p><<branches.branchname>><br />
<<branches.branchaddress1>><br />
<<branches.branchaddress2>><br />
<<branches.branchaddress3>><br />
<<branches.zip>> <<branches.city>><br />
</p>
- Go to detail page of a patron > discharge
- Click 'Generate discharge'
- Verify that the PDF contains the information above.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 14029: Provide 'clear' link to empty reports search filters

Patch adds a 'clear' link next to the 'Apply filters' button to
empty out the form fields.

To test:
- go to the reports module
- make sure you have some saved reports
- search your saved reports using the author, keyword and date filters
- verify searching works as expected
- verify the new 'clear' link works as expected

Tested couner patch, followed test plan, works as expected. QA tools ok.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 13336 - Add time to date column for patron circulation history

Some librarians have expressed that it would be very helpful to have the
hours displayed in the date column for a patron's circulation history.
The time an action took place can be vital to tracking down which
librarians were working at the time a given circulation action took
place.

Test Plan:
1) View a patron's circulation history
2) Note the Date column has no hour/minute
3) Apply this patch
4) Reload the page
5) Note the hour and minutes now display

Signed-off-by: David Roberts <david.roberts@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 7981: DBRev 3.21.00.008

Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 7981: Reintroduce dropboxdate

The parameter was lost in previous commit.

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 7981: (QA followup) Rename TT vars

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 7981: Follow-up - Fix qa script complaints

- Fix updatedatabase, moving the update entry into the right spot
- Fix some tabs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 7981: Use The Branches TT plugin instead of GetBranchName

Note that homebranchname is never used in the template.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 7981: Remove HomeOrHoldingBranchReturn syspref from C4::UsageStats

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 7981: Remove HomeOrHoldingBranchReturn syspref

This patch removes HomeOrHoldingBranchReturn syspref and makes circ/returns.pl respect branch
circulation rules from C4::Circulation::GetBranchItemRule. Also transfer slip notice should reflect this.

Default should always be to return item to home branch.

Test plan:
- make sure syspref 'AutomaticItemReturn' is set to 'false'
- unset 'Default checkout, hold and return policy' or set 'Return policy' to 'Item returns home'
- checkout an item and do a checkin from different branch than items homebranch
- verify that you're prompted with a transfer message to item's home branch and that print slip matches

- set 'Return policy' to 'Item returns to issuing library'
- do a checkout and a checkin from branch different than item's home branch
- verify that you're not prompted with a transfer message and that holding library is your current branch

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Follow-up:
- Added 3 tests in t/db_dependent/Circulation_Branches.t to test AddReturn
  policies
- Removed HomeOrHoldingBranchReturn from sysprefs.sql
- Added notice on removing syspref in updatedatabase

QA edits:
- removed trailing whitespace in tests
- moved branchname lookup from returns.pl to template

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>

Bug 11941: Add link to patron lists from the patron home page

The patron lists are only accessible from the tools module, which is not
easily accessible when you are in the patron module.

Test plan:
Go on the patron home page.
In the toolbar, you should see a link to the patron lists.

NOTE: Tweaked button to a to get the click to work.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14206: Adds test for getletter() call from overdue_notices.pl

Adds missing test for getletter() when called from overdue_notices.pl

Test plan
=========

1/ apply this patch
2/ run prove -v t/db_dependent/Letters.t
   all tests should pass, especially test #40 which tests call from
   overdue_notices.pl

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14206: Adds delete function for non email templates

C4::Letters::getletter() is called in tools/letter.pl by the function
delete_confirm() to display the selected notice for deletion. Due to
current implementation of getletter(), a notice that does not use
the 'email' template (but uses any/all of the other templates - sms,
print or phone) can't be deleted from the staff client.

This patch adds deletion capability for notices that do not use email
template, but uses any/all of the other templates i.e. sms, print or
phone. This also adds 2 tests to t/db_dependent/Letters.t for testing
both conditions - a) when message_transport_type is specified b) when
it is not.

Test plan
=========

1/ Go to Tools -> Notices & Slips. Add a new notice only for print,
   leave 'Library' and 'Koha module' options as default selections.
   Enter 'KOHA_14206' and 'Koha Test 14206' against Code and Name
   respectively, and 'Test' and 'Test Message' for subject and body.
   Leave the Email, Phone and SMS tabs blank. Save the notice.
2/ On the notices listing page the new notice will be listed. Try to
   delete it. It will load the 'Delete notice' dialog form, but the
   table will not show any data under <th>s - 'Library', 'Module',
   'Code' or 'Name'.
3/ Click the "Yes, delete" button. The page will be submitted and the
   Notices listing reloaded. The print-only KOHA_14206 notice should
   continue to exist. This is *wrong*.
4/ Apply this patch
5/ Reload the listings page and click on the 'Delete' link for Notice
   KOHA_14206. This time, it should show the data under 'Module',
   'Code' or 'Name' at least.
6/ Click on 'Yes, delete'. The page should submit and the listing page
   reload. This time KOHA_14206 will be gone.
7/ Run prove -v t/db_dependent/Letters.t
   All tests should PASS without any error.

Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13970: Remove category_type related code

Working on bug 13497 and bug 9314, I run into some Koha vestiges.
The category_type parameter should not be passed to memberentry.
On creating a new patron, the categorycode should be passed, and on
editing, it's useless. We can work with the borrowernumber and retrieve
these values.

Details of the changes:
- members-toolbar.inc: Remove the category_type parameter passed to
memberentry.pl
- memberentrygen.tt: Just remove the useless category_type parameter on
  editing a patron. Also remove the unused one passed to
  guarantor_search.pl.
- tables/members_results.tt: the borrowernumber is enough to edit a
  patron.
- memberentry.pl: check_categorytype is never used in the template, all
  the process to calculate/retrieve it is unnecessary.
- members/nl-search.tt: The borrowernumber is enough to edit a patron.

Test plan:
Try to create and edit patrons and verify that
- the guarantor search still work
- the form (memberentry) behave as before

Edit a patron from the nl-search.pl script (Magnus?)

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
On top of 9314 (13497 already pushed)

No evident regressions found, add/edit patron works,
search/set guarantor works.

Cant test nl-patron.pl save for exec it.
prove -v t/NorwegianPatronDB.t runs

No koha-qa errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 9314: Remove useless code related to the type_only parameter

Since the pref AddPatronLists has been removed in bug 13497, the code
related to type_only and category_type in memberentry.pl is useless.

Test plan:
Confirm you don't the information message.
You can also confirm that the message was wrong and nothing was saved.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Dead code removed, no errors

Think that bug description can be updated to commit message

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14338: Unable to delete patron images

The call to RmPatronImage is still passing cardnumber as its parameter
instead of borrowernumber.

Test Plan:
1) Upload a patron image
2) Ensure the card number is not the same as the borrower number
3) Attempt to delete patron image
   -- Image will remain
4) Apply this patch
5) Attempt to delete patron image
   -- Image will be removed
6) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13265: Use sessionStorage to store searches instead of cookies

This is a counter patch.
The idea is to provide a permanent solution for the cookie length issue
we occurred on storing the searches (intranet side).

Test plan:
Launch as many searches as you can (don't forget to sleep).
You should not get any error.
Confirm there is no regression using the results browser.

Tested with 6 parralel searches in different tabs (with alternatively browising up and down). No problems found.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14360: Unescaped variable causes alert pop-up

To test:

1) Create a list in the OPAC, name it: <script>alert('Hello');</script>
2) Delete the list
3) Confirm deletion
4) See the alert say 'Hello'
5) Apply patch
6) Recreate list with same name
7) Delete list
8) Confirm deletion and alert no longer pops up

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14360: Unescaped variable causes alert

Adding |html to [% resultsperpage %] to escape the variable and get rid of the alert.

To test:

1) Go to URL such as ...  /cgi-bin/koha/opac-authorities-home.pl?op=do_search&resultsperpage=1%22%3E%3Cscript%3Ealert%28document.cookie%29%3C/script%3E
2) Notice pop-up box with alert
3) Apply patch, refresh page
4) Notice alert is gone

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10625: Inventory/Stocktaking tool cannot handle windows file uploads

The current code uses
    $barcode = <fh>;
logic. This reads until \n, as far as I can tell.
EOL is indicated by \n, \r, and \r\n depending on OS and software.
So, to this end, rather than File::Slurp (which is a potential
memory hog, which is already an issue with no filters), a loop
to pre-read the barcodes was written.

This loop includes:
    $barcode =~ s/\r/\n/g;
    $barcode =~ s/\n\n/\n/g;
    my @data = split(/\n/, $barcode);
    push @uploadedbarcodes,@data;

So, that means that lines ending in \n would have it stripped
and pushed into the uploaded barcodes array.

Lines ending in \r would likely be read as one giant block,
have everything converted to single \n's and then using a split,
the set of barcodes are pushed into the uploaded barcodes array.

Lines ending in \r\n would get that stripped and pushed into the
uploaded barcodes array.

It is then the uploaded barcodes array that is looped over for
validating the barcodes.

TEST PLAN
---------
 1) Back up your database
 2) Download the three sample files (or create your own)
 3) Log in to staff client
 4) Create a branch with no inventory.
 5) Home -> Tools -> Inventory/Stocktaking
 6) Browse for your '\r' test file.
 7) Limit to just that branch
 8) Click 'Submit'
    -- Confirm expected errors
 9) Repeat steps 5-8 with the '\n' test file.
10) Repeat steps 5-8 with the '\r\n' test file.
    -- one of these repetitions should have problems.
11) Apply patch
12) Repeat steps 5-8 for each of the 3 test files.
    -- there should be no issues.
13) run koha qa test tools.

Note: This is a tweak based on Jonathan Druart's comment #16
      I have reset it to needs sign off again.

Followed test plan. Works as expected. qa OK.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13437: Replace javascript function parameter in builder

The builder contains js functions with a parameter like subfield_managed
or i or something similar. This parameter contains the html id of the
field corresponding with the plugin.
With the functionality of Koha::FrameworkPlugin in place, we can eliminate
one js function call and get the same id via the event passed in. Note
that this actually makes the function a 'real' event handler.

Also note that in many cases this parameter was not used but the id
was borrowed from a perl variable like $params->{id}. If the field is not
cloned, this is not a problem. But some fields can be cloned and should
not use the static perl value but should get it from the event.

Test plan:
Look for js errors when loading the marc editor.
Since the Focus or Click event code has been touched for most marc21
plugins, move your cursor into the field or click on the tag editor
button. Verify that the focus event updates the correct field or the
click event correctly launches the plugin AND the value comes back into
the right field.
Bonus: Attach a plugin with popup (like leader) to 040$d. Clone this field.
Verify that the two launch buttons operate on the correct value.
(This resolves a current bug.)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Checked all 15 plugins.
Plugin marc21_linking_section seems to work, but I could not get it to pass
back something useful into my field. (Same without this patch.)
Tested the clone button with leader on 040d.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
All seems to work, no errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13437: Conversion of marc21 cataloguing plugins to new style

This patch converts marc21 plugins to new style by making the following
modifications:

[1] Replace use strict with use Modern::Perl. This implies that we now
re-enabled warnings. There are no redefine warnings anymore, but note that
we need to silence some warnings from individual plugins that were covered
by disabling the warnings pragma until now. Silencing these individual
warnings is outside the scope of this report.
[2] Sub plugin_javascript is replaced by an anonymous subroutine $builder.
[3] The parameters of $builder are combined in a params hashref. In most
cases we only need $params->{id} for the function name.
[4] Javascript function Clicxxx is renamed to Clickxxx.
[5] The builder does no longer return function_name.
[6] Sub plugin is replaced by subroutine $launcher.
[7] The parameters of $launcher are combined in a params hashref. We only
use $params->{cgi}. Mostly we save that to $input. One exception: $query.
[8] The plugins returns a hash with $builder and/or $launcher.

Test plan:
[1] Run t/db_dependent/FrameworkPlugin.t -incl cataloguing/value_builder/
marc21*.pl. This should catch compile errors and general problems when
building or launching these plugins.
NOTE: You will see several initialize warnings from individual plugins that
were hidden until now by disabling warnings. This is fine; we will be
able to address these warnings now on new reports.
[2] Check behavior of several plugins in the marc editor.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Checked all marc21 plugins. Attached unused plugins to some field.
Some plugins (unused by default) may need some further attention, but
also outside the scope of this report.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
New warnigs, but all seems to work.
No errors.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13437: Trivial edits on marc21 plugins before conversion

marc21_field_003.pl: copy-pasta comment about date removed
marc21_field_005.pl: commented use removed
marc21_field_006.pl: old/irrelevant pod lines
marc21_field_007.pl: old/irrelevant pod lines
marc21_field_008.pl: old/irrelevant pod lines, move sub par line twice
marc21_field_008_authorities.pl: whitespace, old/irrelevant pod lines
marc21_field_040c.pl: two commented lines removed
marc21_field_040d.pl: whitespace, commented lines, old/irrelevant pod
marc21_field_245h.pl: whitespace

marc21_linking_section.pl: relocated some comment lines, and replaced a new
CGI object by the one passed in via the plugin launcher (agreed, this may
not be so trivial as the other changes)

Test plan:
These (trivial) changes are hard to test.
Pick a few plugins and verify that behavior is not changed in the marc
editor.
For the brave: Try marc21_linking_section.pl.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tried marc21_linking_section.pl :)
(ling plugin to 773$9, create new record, search for parent and check values inserted)
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13437: Preliminary changes for marc21 plugins field 008

Preliminary work on marc21_field_008.pl and 008_authorities.pl.
Moving $dateentered en $defaultval to lower scope level.
date_entered is a new sub (in a module); sub Field008 has been reduced to
one line.
Added a trivial unit test for date_entered.
Note: the format used in date_entered could be added in DateUtils, moving
this logic to a better place.

Test plan:
Use both plugins in the marc21 editor (biblios/authorities).
Run the adjusted unit test.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Both plugins works, no errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13437: Perltidy some plugin files before conversion

Preliminary work before converting to new plugin style. Several files still
contained tabs or had strange indentation.

Perltidied: marc21_linking_section.pl
Perltidied: marc21_field_007.pl
Perltidied: marc21_leader_authorities.pl
Perltidied: marc21_leader.pl
Perltidied: marc21_leader_book.pl
Perltidied: marc21_leader_computerfile.pl
Perltidied: marc21_leader_video.pl

Test plan:
Run perltidy -pro=xt/perltidyrc on marc21_linking_section and compare.
Try another one too :)

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Minor differences on comparison :)
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14256: (follow-up) Check for unique constraint to regenerate random data

There were some issues in the previous patch. This patch fixes the
following:
- rename $value with $original_value
- remove $at_least_one_constraint_failed and $values_ok which make the
  code unnecessarily complicated
- the constraints have to be checked only if no original value is passed
- _buildColumnValue created a key to the default value hashref, it broke
  the test:
    last BUILD_VALUE if exists( $default_value->{$source} );

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14256: Check for unique constraint to regenerate random data

Unique constraints should be checked when creating random data. Otherwise
we get failures when the generated data already exists on the DB.

This patch takes advantage of ->unique_constraints() to do the job,
looping through all the unique constraints defined for the source.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10355: paramater 'object' lost on the road

Test plan:
1) Go to any detail page in staff
2) Click on the modification log tab
3) Verify, that the object is prefilled with the records biblionumber
and you can also see it as parameter in the url
4) Click a second time on modification log to reset your search

Before this patch, the object parameter was empty.
It now contains the value of the biblionumber.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described, no koha-qa errors

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14344: uninitialized value warning C4/Utils/DataTables/Members.pm

The condition for the assignment depends on $searchtype to be defined
and equal to 'contains'. So this change doesn't change the semantics.

-            if $term !~ /^%/
-                and $searchtype eq "contain";
+            if (defined $searchtype) && $searchtype eq "contain"
+                && $term !~ /^%/;

To test:
- Home -> Circulation -> Checkout
- Search for a user that does not exist (I searched 'whywouldthisexist') on the intranet interface.
- Look at the intranet logs
=> FAIL: you get "Use of uninitialized value $searchtype in string eq at.,,"
- Apply the patch
- Repeat the search
=> SUCCESS: No warning
- Sign off :-D

NOTE: Other pages are more forgiving. Tweaked test plan.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14053: DBRev 3.21.00.007

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14053: Acquisition db tables are missing indexes

Acquisition db tables are missing some indexes to have
performance queries.

This patch adds an index on some columns very often
used in search queries, such as aqbooksellers.name and
aqbudgets.budget_code.

Also adds an index on aqorders.orderstatus, very often used
with hardcoded value like 'cancelled', in various queries.

Test plan :
1) Back up database
2) $ git reset --hard origin/master
3) $ git bz apply 14053
4) In your mysql client
   > DROP DATABASE koha_library;
   > CREATE DATABASE koha_library;
   > QUIT;
   -- Obviously you may need to vary koha_library :)
5) Navigate to staff client
   -- should be able to set up the DB just fine.
   -- this will catch the comma bug that keeps coming in.
6) $ git reset --hard origin/master
7) Repeat step 4
8) Navigate to staff client
   -- nothing tested, but we need the DB set up.
9) $ ./installer/data/mysql/updatedatabase.pl
   -- atomic updates run without issue.
10) run koha qa test tools
11) Restore DB
12) Try to compare performance after and before
    database update. I think query contained in
    C4::Acquistion::GetInvoices could be a good example

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14053: Acquisition db tables are missing indexes - atomicupdates

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14314: System Preferences: Better explanation for syspref 'ShowReviewerPhoto'

[PASSED QA] If syspref ShowReviewerPhoto is enabled, the reviewer's avatar is displayed beside comments in OPAC. The avatar will be searched on www.libravatar.org using the patron's email address.

This patch changes the text for 'ShowReviewerPhoto'.

To test:
Apply patch
Go to Home > Administration > System preferences
Search for ShowReviewerPhoto
Verify that the new explanation makes sense.

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

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Changed mail to e-mail.
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14330: Remove unused email_sender from sendbasket/sendshelf

The sendbasket/sendshelf scripts and templates do not use email_sender
as a cgi parameter or as a template var. Probably a leftover from previous
changes.
Let's make Koha cleaner :)

Test plan:
[1] Send your cart from opac or staff.
[2] Send a shelf from opac or staff.
[3] Git grep email_sender. No results.

Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 11790: Remove dependency C4::Context from C4::Charset

C4::Context is only used to retrieve a syspref value.
This patch moves the use of C4::Context to a require.

Test plan:
Try to reach the SetMarcUnicodeFlag subroutine (batchmod, add/update a biblio, etc.)

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested on French UNIMARC install
No errors adding/editing biblios
No koha-qa errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14329: Useless copy/pasta from Template::Plugin::HtmlToText

The synopsis of this TT plugin contains two example lines:
[% myhtml FILTER html2text(leftmargin => 0, rightmargin => 0) %]
[% myhtmltext | html2text %]

These lines have been copied (without too much thought :) to a few templates. Since we do no use the variables myhtml or myhtmltext in these templates, these lines are useless.

Test plan:
[1] Put some items in your cart. And send it.
[2] Send a shelf.
[3] Git grep on myhtml. Should not have results.

NOTE: Sent carts and lists in Intranet and OPAC successfully.
      Though, this does bring into question why the letters
      have HTML formatting if it is getting removed. That,
      however, is beyond the scope of this bug.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14334: t/db_dependent/Letters.t AutoCommit fix

TEST PLAN
----------
1) git reset --hard origin/master
2) prove t/db_dependent/Letters.t
   -- horrible failure about transaction
3) apply this patch
4) prove t/db_dependent/Letters.t
   -- no issues
5) koha qa test tools.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14334: t/db_dependent/Budgets.t AutoCommit fix

TEST PLAN
----------
1) git reset --hard origin/master
2) prove t/db_dependent/Budgets.t
   -- horrible failure about transaction
3) apply this patch
4) prove t/db_dependent/Budgets.t
   -- no issues
5) koha qa test tools.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14334: t/db_dependent/Bookseller.t AutoCommit fix

TEST PLAN
----------
1) git reset --hard origin/master
2) prove t/db_dependent/Bookseller.t
   -- horrible failure about transaction
3) apply this patch
4) prove t/db_dependent/Bookseller.t
   -- no issues
5) koha qa test tools.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14327: Fix js error "TypeError: events is null" in additem.js

If you have no item plugins, the events variable in BindPluginEvents
of additem.js will be null. So testing events.length will generate
the described error.
This patch adds a check to prevent that from happening again.

Test plan:
[1] Do not yet apply this patch !
[2] Temporarily remove framework plugins from your items (in ACQ or default
    framework). Probably you have to clear dateaccessioned.pl and
    barcode.pl.
[3] Open js console in your browser.
[4] Go to Acquisition. Open a basket and add an order from a new empty
    record.
[5] You should see js error: "TypeError: events is null" (additem.js:176)
[6] Apply this patch and reload the page (make sure that you refresh so
    that the new javascript code is read).
[7] The TypeError should be gone.
[8] Restore the framework plugins from step 2. Refresh the page again and
    verify that they still work as expected.

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14318: iDreamBooks doesn't work when Koha is using https

If a Koha server is configured to run over SSL, all iDreamBooks content
is blocked due to the fact that a secure page is requesting an insecure
endpoint. This is due to the fact that the urls for iDreamBooks use http
and not https. A simple fix would be to switch them to https since
browsers have to qualms about loading a secure data endpoint from an
insecure one.

Test Plan:
1) Enable iDreamBooks
2) Set up your OPAC to use https
3) Verify iDreamBooks content continues to work

Note: tested Chrome and IE, so that the IE change would be validated.
      Discovered isbn semi-colon issue that is beyond scope of this bug.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14342: Unit tests in t/db_dependent/Context.t failing due to Bug 13967

The introduction of system preference objects in bug 13967 has caused
the current testing regimen to fail do to the mixing of DBI mocking and
DBIx::Class.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Context.t

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14339: C4::Context->preference should return undef if DB is not populated

The current behaviour for C4::Context->preference when the DB is not still
populated with tables is to return undef. This is used by C4::Auth to identify
the need of running the installer.

This behaviour got broken by bug 13967, which lets DB errors to escalate and
thus Koha gets broken instead of prompting for install.

This patch wraps Koha::Config::Sysprefs->find inside an eval and sets undef if needed.

To test:
- In current master, drop the DB
- Load OPAC and Intranet
=> FAIL: notice an ugly software error.
- Apply the patch
- Load the OPAC
=> SUCCESS: Maintenance mode screen is shown
- Load Intranet
=> SUCCESS: You are prompted the DB credentials to run the web installer.
- Sign off :-D

Tomas

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14346: (folowup) Fix previously existing POD

This patch makes koha-qa.pl happy by fixing POD issues prior
to this bug.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 14346: t/Biblio.t fails because of new warning

Running

 $ prove t/Biblio.t

fails because of us now using DBIx to retrieve sysprefs. Then our mocked DBI is not "supported" by DBIx hence a warning that makes our test fail (there is one more warning now).

The cool thing about this, is that it actually helped spot a situation where GetMarcBiblio is doing wrong things because is not checking its parameters are undefined, so we have the chance to fix it.

This patch makes GetMarcBiblio return undef if no biblionumber is passed, and
also raises a conveniently carped warning. This change is tested in t/Biblio.t with new tests.

To test:
- In current master, run
  $ prove t/Biblio.t
=> FAIL: a test detects a wrong warning count and fails.
- Apply the patch and run
  $ prove t/Biblio.t
=> SUCCESS: Tests now pass, and there are 2 new ones.
- Sign off :-D

Regards

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 10938: Item columns displayed in random order - OPAC

Same as before for the OPAC.

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

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10938: Item columns displayed in random order

On the MARC detail page, the columns are displayed in a random order.
This means that you can open 2 different records and see the columns
displayed in a different order.

Test plan:
Go on different MARC detail view and confirm that 1/ all fields are
present and 2/ they always are displayed in the same order

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
1/ & 2/ correct
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 7976: DBRev 3.21.00.006

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 7976: Update borrowers.flags

The borrowers.flags values need to be updated to prevent side effects.

Test plan:
Set the borrow permission flags for some borrowers
Execute the updatedb entry and verify the flags has been updated (-128)
only for borrowers with this permission set.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Moved updatedatabase entry (was in wrong position) to atomicupdate
with commented copy of orignal

No koha-qa errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 7976: Remove the borrow permission

The borrow permission was used but uselessly.
For instance, at the opac, the flagsrequired parameter was set to
'borrow' but the 'authnotrequired' was set also (which means no auth
required).
At the end, this permission was used at only 1 place: for the basket,
intranet side.
This can be replaced with the catalogue permission (which is used to
search).

Test plan:
1/ Confirm that you are able to show/download/sent the cart (intranet side)
with the catalogue permission.
2/ At the OPAC, you should be able to access the same pages as before
with any other permissions.

Concretely it is quite difficult to test this patch, you should have a
look at the code.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14276: Keep highlight on the active item in item editor

The highlight only works on even items.
This patch should resolve it.

Test plan:
Edit biblio with multiple items.
Verify that the highlight is visible on the selected item you edit.
And that there is no highlight for a new item.

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14173: Paging on 'recent comments' page in OPAC is not displaying correctly

This patch corrects the display of current page on
a multipage recent comments.

To test:
1) Enable OpacShowRecentComments

2) Add multiple comments to multiple records
I used a script to add multiple lines like
"insert into reviews values ($i, 51, $i, 'Comment $i', 1, '2015-06-01 00:00:00')"
to table reviews

3) On OPAC, go to 'Recent comments', verify the bug

4) Apply the patch

5) Reload and check correct display

Can't found missing space near 'by' from description.
Display is correct for me.

Followed test plan, displays as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 10824: OAI-PMH repository/server not handling time in 'until' and 'from'

This patch removes the DATE() function from a query on timestamp, and adds a
sub that strips the UTC designators "T" and "Z" from incoming "from" and
"until" arguments in OAI-PMH requests so that they're more compliant with
MySQL (and probably other databases as well). This means that the date
and time for the 'from' and 'until' arguments will be matched correctly
in the database.

This patch also adds 'T00:00:00Z' to 'from' arguments and 'T23:59:59Z' to
until arguments, when only dates are provided via the OAI parameters.

The zero time isn't necessary, since MySQL treats '2013-09-30' as
'2013-09-30 00:00:00' by default. However, the near midnight time
is needed for 'until'. Otherwise, you'll never be able to retrieve
a record with a date/time matching the 'until' argument.

In summary, this patch adds handling for times as well as dates, which
is necessary so that Koha is closer to meeting the actual OAI-PMH spec.

TEST PLAN:

0) Note down a selection of timestamps from your biblio table

1) Enable your OAI-PMH server through the global system preferences
Web services tab.

2) Craft and submit a similar request to the following in your browser:
KOHAINSTANCE/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc&
from=2013-09-02T13:44:33Z&until=2013-09-05T13:44:33Z

Change the exact dates to accord with your timestamps, but keep the
YYYY-MM-DDTHH:MM:SSZ format.

3) Note the unexpected behaviour. A "from" argument with the timestamp
2013-09-02T13:44:33Z will show records from 2013-09-03 but not records
from 2013-09-02 even though the timestamp in the database will say
"2013-09-02 13:44:33".

Also note that records with a timestamp later than 13:44:33 will show
up for the day 2013-09-05, even though they shouldn't.

4) APPLY THE PATCH

5) Resubmit the links you tried above

6) Note that the applicable records now appear (or do not appear) in
accordance with the precise date/time ranges!

--

Developer Note: We could've not stripped the UTC designators and used
DATE() around the parameters in the SQL queries, but that would have
lost the whole purpose of using times in the "from" arguments, since
they would've been generalized to just the dates.

I think this is probably the best solution. Admittedly, creating
"form_arg" and "until_arg" hashrefs in the ResumptionToken object
might not be ideal, but I preferred that to copying the
_strip_UTC_designator subroutine into two other objects. Perhaps this
sub could go somewhere else and be imported into those other two objects
but this seemed to be the most sensible decision. I'm open to other
opinions though.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works, find results with correct timestamp
No koha-qa errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13619: Acq home: ensure 'manage suggestions' goes to pending suggestions

The link on the start page of Koha goes to the pending suggestions tab
explicitly. The link on the acquisition start page doesn't do that so
when you have your own status it might show another tab first.
Patch links to be the same and explicitly target the pending tab.

To test:
- create a new suggestion
- verify link from the acq start page leads to the pending tab
- verify link from the Koha start page does the same

Signed-off-by: Marjorie <marjorie.barry-vila@ccsr.qc.ca>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13656: "Change"/"Set to patron" button for linking a member to an organisation (or child to guarantor) not translatable

Test plan:
1. install and activate an additional language
2. create patron in organization category
3. create professional patron
4. try to add this patron to an organization (Guarantor information section)
5. note that the left button text changed to "Change" - untranslated english string
6. push the "Delete" button, the guarantor patron field is cleared and the left button text changed to "Set to patron" - again original english text
7. apply the patch
7.1. update translation (koha-translate -u language_code)
8. repeat 4-6, note, that button text are still translated in all sitations
9. sign off ;)

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described, no errors
Fixed message capitalization

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13662: Fix the serials.receive_serials permissions

There are some issues with serial permissions.
For instance it's not possible to receive serials if the
edit_subscription is not set.
Also the toolbar is empty.

Test plan:
1/ Set the serials => receive_serials permissions to a patron (and only
this one for the serials module).
2/ Verify you cannot create a new subscription, you can search
subscriptions but cannot edit them.
3/ On the serial result list, receive a serial (action > Serial
receive).
You can now change the status and receive it.
4/ On the serial collection, you can edit 1+ serials to reveice it.
5/ Set the serials => edit_subscription permission and confirm there is
no regression.

QA note: I think we should introduce a C4::Serials::can_receive_serials
subroutine, to test the IndependentBranches pref, but I don't want to
add to much processing to check permissions.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14299: Today's checkouts not always sorting correctly

Sometimes the today's checkouts do not sort correctly. This is due to a
simple typo in the comparison line where the bad key 'timstamp' is
compared against the correct key 'timestamp'.

Test Plan:
1) Check out a decent number of items in a row ( 5+ )
2) Hopefully you will see they are sorted incorrectly
3) Apply this patch
4) Reload the page
5) Note they are now sorted correctly

Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13967: (RM followup) fix merge error

When I merged 13967 into master (with 5010 already on it) I inadvertedly
removed a few lines that add a check for bad OPACBaseURL values.

While I don't agree with that check it should be discussed on a separate
bug anyway.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13967: (QA followup) Make DBIx control transactions on tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14142 - Holds queue viewer only displays first subtitle from marc keyword mappings

Despite the point of the Keyword to MARC Mappings being to simplify the
handling and display of repeated values from multiple subfields, the
holds queue viewer will only display the first value found. What it
should be doing instead is displaying all fields that match the subtitle
keyword.

Test Plan:
1) Apply this patch
2) Define multiple Keyword to MARC mappings for the subtitle keyword
3) Place a hold on a record using those subtitle fields
4) View the hold in the holds queue viewer
5) Note that all the subtitles now appear

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

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13815 - plack loose CGI qw(-utf8) flag creating incorrect utf-8 encoding everywhere

This is major problem for plack installations with utf-8 encoding.

In this case, we are overriding CGI->new to setup utf-8 flag and
get correctly decoded $cgi->params, and reset syspref cache using
C4::Context->clear_syspref_cache

Test scenario:
1. under plack try to search with utf-8 charactes
2. try to find patron with utf-8 characters

Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14267: How active is active?

git grep on function active in additem.tt:
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt:function active(n
koha-tmpl/intranet-tmpl/prog/en/modules/serials/serial-issues-full.tt:active([%
koha-tmpl/intranet-tmpl/prog/en/modules/serials/serial-issues-full.tt:function a
koha-tmpl/intranet-tmpl/prog/en/modules/serials/serial-issues-full.tt:
t/Cache.t:      unless ( $cache->is_cache_active() && defined $cache );
t/Cache.t:          unless ( $cache->is_cache_active() );
Conclusion: active in additem seems to be quite inactive.

Test plan:
Add, edit or delete items and verify that you did not miss active :)

NOTE: The active function has a loop which is always run.
      Inside that loop 'ong' would always be defined as some number
       concatenated with XX.
      Both sides of the if/else reference document.getElementById(ong),
       but there is only one occurence of XX in the file: the concatenation!
      Similarly, the 'link' logic does not correspond to any of the
       id= or name= strings in the file.
      koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_subfields_structure.tt
       is the only file with "id=\"link" that matches the logic.
      This is likely a cut-and-paste remnant made useless by datatable upgrades
       and HTML/CSS class changes.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14269: OPAC: Some template improvements for the full serial history page

- Fix filter labels:
  Library : -> Library:
  Subscription : -> Subscription:
- Make '(All)' entry in filter pull downs translatable
- Show branch name instead of branchcode in table and filter

To test:
- Verify changes as described above
- Verify filters still work as expected

Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14024: DBRev 3.21.00.005

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14024 - add reports to action logs

See QA comment #5

- new reports log their Id instead of 0
- format info: <name> | <sql>

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14024 - add reports to action logs

This patch sets adds the possibility to log new, update and delete actions for saved reports.

To test:
-Apply patch
-Run updatedatabase.pl
-Enable system preference ReportsLog
-Create, duplicate, edit and delete saved reports
-Go to Home > Tools > Logs
-Verify that you can select "Reports" in Modules list
-Verify that your actions were logged

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14313: OPAC - Adding a comment makes result browser disappear

To reproduce:

- Allow commenting in OPAC (Syspref reviewson)
- Log in to OPAC
- Do a search with many results
- Click on a biblio in result list
- Verify that you can browse the results in detail view ("Browse results")
- Repeat teh search above
- Click on the same biblio as above
- Add a comment (Tab "Comments")
- Close commenting window
- Click on "Next" in result browser

Result: The next biblio is displayed, but result browser has disappeared.

To test:

- Apply patch
- Try to reproduce issue above, verify that result browser does no longer disappear

AMended to remove whitespace chars. / MV

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Bug & solution checked, works well. No koha-qa errors

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13967: Add a couple of tests for SysPref and Object

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

Bug 13967 - System preferences need a package

System preferences should have a package based on Koha::Object to remove
the need for direct manipulation via SQL.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/sysprefs.t

Signed-off-by: Chris Nighswonger <cnighswonger@foundations.edu>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14111 - More t/Auth_with_shibboleth.t silencing

TEST PLAN
---------
1) Apply first patch
2) prove t/Auth_with_shibboleth.t
   -- failed?!
3) apply this patch
4) prove t/Auth_with_shibboleth.t
   -- success. *whew*
5) koha-qa test tools.

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Test remarks: Expected outcome as per test plan. No anomaly noted.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 14111: Silence warnings in t/Auth_with_shibboleth.t

Poorly capturing warnings_exist test cases generated noise.
This patch fixes that.

TEST PLAN
---------
1) $ prove t/Auth_with_shibboleth.t
   -- There are messages about lines 132 and 133. Noise.
2) Apply patch
3) $ prove t/Auth_with_shibboleth.t
   -- No messages. YAY!
4) koha qa test tools

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Test remarks: Expected outcome as per test plan. No anomaly noted.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Bug 13382 - RDA: 700/710/711 display in XSLT

Test Plan:
1) Apply this patch
2) Ensure you are using the default XSLT setting for the staff and opac record details
3) Find or create a record with MARC tags 700,710,711
4) Perform an opac search that would show the record in the search results.
5) Click title to review record.
6) Note the fields updates 700,710,711 to show subfields a, b, c, d, e, f, g, h, i, k, l, m, n, o, p, r, s, t, u, x. Multiple fields are separated by span class=separator |. Adds Related and Contained Works as new headings.
7) Repeat steps 4 - 6 for the staff interface

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>