Bug 11703: (followup) svc/holds invoked with trailing .pl

On my followup I forgot to fix js/holds.js. This patch rectifies that.

Regards
To+

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10519: Suggestions: 'Organize by' and correct display of tab descriptions broken

The tabbed display in suggestions offers different options to organize
the tabs. The descriptions on the tabs and some of the search options were
not working correctly, displaying as "Unknown".

To test:
- Add several suggestions to your installation, make sure you have:
  - suggestions from different users
  - suggestions managed by different users
  - suggestions with different statuses
  - suggestions with different selected item types

Test all the 'organize by' options (except "Organize by: Library"
- see note below), make sure that the tabs and search options
have correct descriptions and do no longer display as "Unknown".

- Add 1 or 2 custom status to SUGGEST_STATUS authorized value.
  - Verify display is still correct and your new status are displayed.

Note: "Organize by: Library" option is currently severely broken
(and not easily fixable, especially for 'IndependentBranches'
enabled). But this turns out to be a separate issue, with a different
underlaying causes, and it's outside the scope of this patch.
This should be dealt with later, in it's own bug report.

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10519: (followup) Fix context for evalyating $borrowernumber

Follow-up to fix additional suggestion statuses, which are not working
properly due to

   416:  borrowernumber           => $input->param('borrowernumber'),

being evaluated in list context inside $template->param() call arguments.

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 10519: (followup) unit tests leave problematic cruft

On testing I've found that t/db_dependent/Suggestions.t leaves
cruft on the DB. Small followup for that.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12729 - Overdue items won't show as overdue in red in circulation

It seems that Firefox date parser doesn't like our dates which are
formatted in ISO format like "2014-08-06 00:00:00". This results in
missing red color in overdue dates.

So intead of munching different date formats and JavaScript (and having
to support different browers) this patch moves check for overdue dates
back to mysql and just transfers boolean value to JavaScript so it can
show correct class for date_due.

Test scenario:
1. find borrower with overdue checkouts
2. verify that all dates are black (and are in ISO format)
3. apply this patch
4. reload page and verify that overdue dates turned red

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested with different due dates (hourly and not) and different date formats.
Passes tests and QA script.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12899: Row grouping in checkouts table is alphabetical and depends on translation

The sort order of the "today's checkouts" and "previous checkouts" row
groupings depends on the label, so in English "today's checkouts" comes
first. However, in other languages the reverse alphabetical order is
incorrect resulting in "previous checkouts" coming first.

This patch adds a dummy column with numeric data on which the sorting
can be done. This should make it translation-agnostic.

To test, apply the patch and install or update a translation which will
demostrate the problem (sv-SE for instance).

- Clear your browser cache and switch to the English templates.
- Check out some items to a patron who has checkouts from a previous
  day.
- Confirm that the sorting of the "today's checkouts" and "previous
  checkouts" row groups is correct.
- Switch to the new/updated translation and reload the circulation page
  for that patron. Confirm that the sort remains correct.
- Confirm that the checkouts table looks correct and that other features
  (sorting, checkboxes) still work correctly.

Revision: Corrected the table footer include to correct the colspan
error causing column misalignment.

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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12963: Columns misplaced in moremember.tt

After applying Bug 12899, in moremember page, checkouts table has
misplaced columns.

Test plan:
1. Check out title for a user. Columns in Detail page at checkout tab
are misplaced; there's no Due date column.
2. Aplly patch, everything shuld look fine now.

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

My bad! Thanks for catching this.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12811 - Patron 'Details' and 'Check out' pages not working for staff users without renewal override permissions

In case when the staff user doesn't have (circulate) ->
(override_renewals) permission granted, this code part

   var AllowRenewalLimitOverride = [%
CAN_user_circulate_override_renewals && AllowRenewalLimitOverride %];

in circ/circulation.tt and members/moremember.tt leads to javascript
error, because TT statement evaluates to empty string.

To reproduce:

- set AllowRenewalLimitOverride syspref to "Don't allow" (this step is
  possibly redundant / not quite relevant),
- for testing purposes, log in as some (sample) staff user whitch does
  not have permission granted for "(override_renewals) Override blocked
  renewals",
- have a look at some patron accounts (preferably, ones with 1+
  check-out), observe that page layout is incorrect in "Check Out" and
  "Details" tab.

To test:

- apply patch,
- retest and ensure that this issue is no longer reproductible,
- make sure that there are no regressions of any kinds regarding renewal
  override permission for staff users (i.e, that it still works like
  intended).

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes QA script and tests.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12419: Not for loan items are not listed

On the cataloguing search (cataloguing/addbook.pl), if an item has a
notforloan value > 0, the item is not listed in the Location column.

It is quite confusing, the current behavior let patrons believe that
there is not item for the biblio (or less than the real count).

Test plan:
1/ Create 2 biblio records A and B
2/ Create some items for A
3/ Create 1+ item(s) for B with a notforloan status > 0
4/ Reindex both records
5/ Launch a search on the cataloguing module and verify that the
notforloan items are not listed in the 'Location' column.
6/ Apply this patch and verify the not for loan items are listed ("Not
for loan (XXX)").

Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes tests and QA script, not for loan items now show up.
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12419: (QA followup) regression tests

Changes to searchResults deserve unit tests.

Edit: minor tweaks:
 - Remove warning in line 702
 - Modernize
 - License added
 - "This file is a stub" removed

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12151: Remove uses of smartmatch operator in Search.pm and opac-search.pl

This patch removes the use of smartmatch operators in the search code.

Regards
To+

Edit: this revision uses 'grep' instead of Lists::MoreUtils::any

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Tested search, no problems found.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12151: Remove uses of smartmatch operator in report scripts

This patch removes the use of smartmatch operators in report scripts.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Acquisition and Patron statistics wizard tested, no regressions found.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12151: Remove use of smartmatch operator in tools/batchMod.pl

The '~~' smartmatch operator is used to compare MARC::Field->subfield(code)
(i.e. a string) and the text element of each MARC::Field->subfields() which
is also plain text.

Substituting '~~' for 'eq' should be harmless then.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Tested batch modification of items, no problems found.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12151: Remove uses of smartmatch operator from Koha/Solr/QueryBuilder.pm

Just that.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Could only verify by reading the code.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12593: search facets die with regex error if biblio has square brackets in fields

It's quite common to have [something] within facet data, and it produces following error:

Unmatched [ in regex; marked by <-- HERE in m/^[ <-- HERE

This problem was intoduced in Bug 12151 but is trivial to fix.

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

To test:
- Created a bibliographic record, linked to an authority record (personal name). Did a search that returned the author as a facet.
- Added a [ symbol to the author name.
- Repeated the search
=> FAIL: "Unmatched [ in regex; marked by <-- HERE in m/^[ <-- HERE"
- Apply the patch
- Retry the search
=> SUCCESS: No error, bracket shows correctly.

Passes koha-qa.pl too.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12600: remove duplicate use statement from code

A use C4::Charset was added deep in the body of the code
we have already imported it at the top of the file
(the by convention normal place) As use is executed at compile time
specifying it in the code body does not serve a
useful purpose and detracts from the readability of an already
overly complex subroutine.
Remove the superfluous statement
also removed the tabs introduced to the surrounding lines
by the same commit

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

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bub 12788: (regression test) refactor facet extraction code to allow testing

This patch refactors the facet extraction loop into a proper function.
The loop is changed so the MARC::Record objects are created only once
instead of the old/current behaviour: once for each defined facet (in
C4::Koha::getFacets).

To test:
- Apply the patch
=> SUCCESS: verify facets functionality remains unchanged.
- Run:
  $ prove -v t/db_dependent/Search.t
=> SUCCESS: tests for _get_facets_data_from_record fail, because
  100$a is considered for fields with indicator 1=z (field added
  by IncludeSeeFromInSearches syspref).
- Sign off :-D

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12788: facets calculation should skip 100 if ind1=z

This patch adds a test for field 100, to skip it on facet calculation
if ind1=z.

To test:
- Have IncludeSeeFromInSearches set.
- Create a biblio record, when adding an author, create a new authority record
  that contains a 400$a field (see from).
- Rebuild zebra db.
- Search for the record making sure the search returns more than one record.
=> FAIL: the facets contain the 'see from' field.
- Run
  $ prove -v t/db_dependent/Search.t
=> FAIL: it fails
- Apply the patch
- Run
  $ prove -v t/db_dependent/Search.t
=> SUCCESS: it passes
- Re-run the search, notice the 'see from' doesn't show anymore on the facets.
- Sign off :-D

Edit: minor stylistic change

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12788: (followup) minor optimization with proper tests

This patch removes the $facets_info calculation from the _get_facets_data_from_record
sub so it is not done for each record. It introduces a new sub, _get_facets_info
that is called from the getRecords loop, that does the job only once.

To test:
- Apply on top of the previous patches
- Run
  $ prove -v t/db_dependent/Search.t
=> SUCCESS: _get_facets_info gets tested and it passes for both MARC21 and UNIMARC.
  Facets rendering should remain unchaged on the UI.
- Sign off :-D

Sponsored-by: Universidad Nacional de Cordoba

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Bug 12405 - Search links on callnumber fails on intranet results page

On intranet results page, the callnumber of items is a search like :
/cgi-bin/koha/catalogue/search.pl?idx=callnum&amp;q=[% result.itemcallnumber |url %]

The callnumber should be URI-escaped to avoid special URI characters like & , ? ...
If the callnumber contains some CCL words or parenthesis, the search will fail, it should be wrapped with double-quotes.

This patch adds this to catalogue/results.pl and catalogue/shelves.pl :
- uri TT filter instead of url
- adds double-quotes using there URI code %22 since its in a HTML attribute using double-quotes

Test plan :
- Edit an item callnumber with : & ABC 123
- Index zebraqueue
- Perform a search returning this item
- Click on the callnumber
=> Without this patch you get no result, because URL parameters are : idx=callnum&q=& ABC 123
=> With this patch you get results
- Set syspref QueryWeightFields off (because this is no bug if on)
- Edit an item callnumber with : AB(C) AND OR
- Index zebraqueue
- Perform a search returning this item
- Click on the callnumber
=> Without this patch you get no result, because the search contains CCL words "OR" and "AND"
=> With this patch you get results

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Tested all with and without queryweightfields -
all searches from clicked call numbers for given callnumbers fail without the patch, all are successful with the patch.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described - no problems found.
Passes tests and QA script.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>