Commit graph

4711 commits

Author SHA1 Message Date
Jonathan Druart
7ca9168385 Bug 11243: update vendor list to also display canceled bib counts
Same as previous patch but for the biblios count.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-20 15:18:51 +00:00
Jonathan Druart
b62a793580 Bug 11243: make vendor list distinguish between active and canceled items
On the vendor result list, the "Item count" columns contain the sum of
all items ordered for a basket. But if an order is canceled, the item
count is not really meaningful.

This patch just adds, in parenthesis, the number of items canceled.

Test plan:
- create a basket and 3 orders with different number of items
- cancel 1 order
- verify on the supplier list that the number of items is correct and
  the number of canceled items is correct too.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.

Note: In case the biblio was deleted when the order was cancelled,
the number of biblios will be off.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-20 15:17:31 +00:00
Fridolyn SOMERS
cac06afeb1 Bug 11254: make reservoir search normalize ISBNs
When importing records, the ISBN is normalized and stored
into database (see C4::ImportBatch::_add_biblio_fields).  But when
searching with ISBN into reservoir, it is not normalized
(see C4::Breeding::BreedingSearch).  So search does not match.

This patch adds the normalisation to reservoir search.  Also, it
replaces call private method _isbn_cleanup by GetNormalizedISBN,
the correct public method.  Also allows the reservoir search
on ISBN with hyphens.

This is intended to fix only reservoir searches.

Revised Test plan
-----------------
 1) Back up DB
 2) Save copy of attached example somewhere findable
 2) Home -> Tools -> Stage MARC records for import
 3) Click Browse and select the example MARC file
 4) Click Upload file
 5) Tweak as desired then click Stage for import
 6) Click Manage staged records
 7) Click Import this batch into the catalog
 8) Home -> Cataloging
 9) In the Cataloging search text box type 978-0-691-14289-0 and
     click Submit
    -- ISBN13 with hypens not found in reservoir
10) In the Cataloging search text box type 9780691142890 and
     click Submit
    -- ISBN13 without hypens not found in reservoir
11) In the Cataloging search text box type 0-691-14289-0 and
     click Submit
    -- ISBN10 with hypens not found in reservoir
12) In the Cataloging search text box type 0691142890 and
     click Submit
    -- ISBN10 without hypens found in reservoir
13) Apply patch
14) Repeat steps 9-12, this time it is always found! :)

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 21:44:30 +00:00
774483772b Bug 11912: (refactoring followup) make GetMarcISBN implement its advertised API
The current implementation of GetMarcISBN contradicts the documented API.
It currently returns an array of hashes with only one key (marcisbn)
which doesn't add any value to it.

I chose to fix GetMarcISBN to honour the API instead of changing thex
 docs, because it seems a really silly change.

To test:
- Run:
  prove t/db_dependent/Biblio.t
=> SUCCESS
- catalogue/detail.pl should correctly show ISBNs.
- opac/opac-detail.pl should correctly show ISBNs in both prog and bootstrap.
- opac-opac-sendshelf.pl should correctly show ISBNs in the email.

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 18:23:15 +00:00
c4900dc448 Bug 11912: fix problem where GetMarcISBN wrongly prepends a space to ISBNs
This patch makes the logic inside GetMarcISBN simpler and
fixes the issue.

To test:
- Run the regression tests:
  prove -v t/db_dependent/Biblio.t
=> FAIL
- Apply the patch
- Run:
  prove -v t/db_dependent/Biblio.t
=> SUCCESS
- Verify that opac-detail.pl and catalogue/detail.pl look as usual regarding ISBN
- Sign off

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 18:21:15 +00:00
Galen Charlton
97bf5f258a Bug 10986: (follow-up) improve POD
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 17:40:35 +00:00
Jonathan Druart
3950a9c9eb Bug 10986: QA followup - fix various issues
This patch fixes some qa issues:

 FAIL   C4/Languages.pm
   FAIL   critic
        # Variables::ProhibitConditionalDeclarations: Got 1 violation(s).
 FAIL   installer/data/mysql/updatedatabase.pl
   FAIL   forbidden patterns
        forbidden pattern: merge marker (<<<<<<<) (line 7157)

 FAIL   installer/data/mysql/sysprefs.sql
   FAIL   sysprefs_order
        Not blocker: Sysprefs AdvancedSearchLanguages is bad placed (see bug 10610)

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 17:20:38 +00:00
blou
92d021f989 Bug 10986: add system preference to limit the list of languages in advanced search
In Advanced Search, the list of available language is long and will only
get longer.  For a library offering books in 2-3 languages, that is
offering too many options to the user (most of the small libraries we
deal with only offer documents in two languages).

Code changes:
Languages.pm: Extract getAllLanguages to make a more customizable
getLanguages (have getAllLanguage call it, so rest of codebase is
oblivious to the change).  Build array returned based on system pref if
corresponding argument is set.

search.pl and opac-search.pl: call getLanguages instead of
getAllLanguages.

TESTING
0) All language codes are iso 639-2 (three characters)
1) in OPAC, Advanced search, open Language box, acknowledge 30+ items.
2) in Intranet, go to system preferences AdvancedSearchLanguages,
   enter "ita|eng"
3) back in OPAC, refresh screen, acknowledge only Italian and English
   are listed.
4) in Intranet, click Search then click "More options" to make the
   Language box appear.  Acknowledge limited options.
5) Regression Test: Back to the preference, empty the field then save.
   Go back to the OPAC and Intranet search, refresh the page, then the
   Language drop-box will now contain 30+ items.

Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 17:18:32 +00:00
Galen Charlton
08fe85950a Bug 10951: (follow-up) use Koha.Preference() template function
This patch uses the TT helper function Koha.Preference() to
retrieve the value of NoLoginInstructions rather than passing
it to all templates as a template variable.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 16:53:26 +00:00
blou
c67b397236 Bug 10951 - Adding NoLoginInstructions to system preferences
On a failed login, the default message is harcorded into opac-auth.tt.

     It would be preferable to allow for a preference to override that message (for example: ...Please bring an ID to t
     The changes modify
         -opac-auth.tt to allow for custom value
         -admin/preferences/opac.pref to add it to the preferences with a description
         -C4/Auth.pm for the loading of the preference
         -sysprefs.sql
         -updatedatabase.pl

     TESTING
         1) in OPAC, logged out, try login in by entering no or wrong credentials.  Acknowledge the "Don't have a p
         2) Apply the patch
         3) Regression Test: Redo step 1.  Same (default) message should appear.
         4) Log in to intranet,
             - select NoLoginInstructions in system preferences.
             - Enter new (xml) message.  Possible:
             <h5>Welcome to Koha, please bring your passport to the front office</h5>
            - and save
         5) refresh the OPAC, try login again with invalid credentials.  The new message should appear.

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

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
2014-04-19 16:26:50 +00:00
Galen Charlton
58b5b384f6 Bug 9416: (follow-up) reconcile with work done on bug 11699
This patch teaches the ordering receiving process how to
set vendor and internal order notes.

One observation: I'm not sure it's entirely useful to set
a note to communicate to the vendor during receiving --
how is it to be sent to them, and why?

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 15:55:38 +00:00
Mathieu Saby
36074fba65 Bug 9416: add new order vendor note field
Currently, there is a single note field in each order. It would be
useful to have 2 notes fields:

- one for the staff (ex: "catalog this book as soon as possible")
- one for the vendor (ex: "urgent", "only the 2d volume"...), which
  could later be printed in basketgroup pdf for example

This patch adds a new note made for vendor in each order. The existing
note is renamed "internal note".

The behavior of the 2 notes are the same

Changes in database structure:
- new column aqorders.order_vendornote
- column aqorders.notes renamed aqorders.order_internalnote

To test :
[1] Make a complete acquisiton process (creating the order > looking at
    the basket > looking the order > receiving); and try to use the 2
    notes (internal note / vendor note)
[2] Check the changes made on one page (eg detail of the order) are
    saved and visible on an other page (eg receipt page)

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Testing notes on last patch.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 15:55:36 +00:00
Galen Charlton
5629f4bc25 Bug 11699: (follow-up) fix errors in the POD for ModReceiveOrder
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 15:12:08 +00:00
Jonathan Druart
36fc9a3e64 Bug 11699: change ModReceiveOrder to used named parameters
Test plan:
prove t/db_dependent/Acquisition.t
prove t/db_dependent/Acquisition/Invoices.t
prove t/db_dependent/Acquisition/OrderFromSubscription.t

all should return green.

NOTE: Any error messages are the same between master and this
      patch, and are unrelated to the added/revised tests.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 15:08:21 +00:00
Jonathan Druart
ab1a74897e Bug 11699: fixed saving notes entered when receiving orders
Revised test plan:
1/ Create an order with 2 items
2/ Receive 1 item and enter a note for the order
3/ Verify the note is not saved
    The note should be visible on the Mod Order Details screen,
    but it isn't there.
4/ Apply patch
5/ Receive the second item and enter a note for the order
6/ Verify the note is correctly saved
    The note is visible on the Mod Order Details screen.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Works as described. The note now saves correctly and also remains when
you undo a receipt.

Note: it would be nice to show the note on the receive page as well.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 14:59:17 +00:00
Fridolyn SOMERS
2c658d882a Bug 11069: increase title ranking in relevance when using QueryWeightFields
When using QueryWeightFields to add ranking on a search without index,
the search actually uses:

 - rank 1 : Title-cover,ext :  exact title-cover
 - rank 2 : ti,ext : exact title
 - rank 3 : Title-cover,phr : phrase title-cover
 - rank >7 : queries without index

This relevance sets title as phrase in priority and then any index.

This patch adds title as words list before search on any index, so
that records with all searched terms in title, even not well ordered,
are more relevant.

Test plan :
- Enable QueryWeightFields syspref
- Perform a search, with sort by relevance, with two words ofen
  contained in title, but never one near the other.
  For example: 'History France'
=> Records with both words in title are first. For example:
   "Histoire de France" and "La France : 100 ans d'histoire"

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>

Relevance ranking and field weighting are hard to test,
as many MARC fields are indexed into the used indexes.
If we had an index that only indexed 245$a/200$a the
effect might be more visible.
I found no regressions by this patch, change reads
logical.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 14:45:51 +00:00
Jonathan Druart
bece83f8ed Bug 9216: make columns.def file translatable
The SQL column headers is stored into the columns.def file.
This file is not managed by the translation script.

This patch makes possible the headers translation.
Note: The translation xml tags were added to avoid all lines being put
on a single line.

Test plan:
1/ update your po file
cd misc/translate;
perl translate -f columns update LANG # Replace by another language here
2/ translate header columns (search "columns.def" in your po file).
3/ install the translated columns.def
perl translate -f columns install LANG # Replace by another language here
4/ go on the report module > create a new report > next > next
5/ change the language
on the 3rd step, you should see the column header translated.

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

[on es-ES about a third of the strings translated!! :-) ]

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described and fixes a long standing translation
problem.
Passes all tests and QA script.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-19 13:34:33 +00:00
Fridolyn SOMERS
3b402d04e1 Bug 9578: avoid a search crash when attempting to sort results of invalid query
When searching with a sort (means not by relevance) and there is an error
in Zebra connexion (server is down or query is wrong), you get the message :

  Error : Can't call method "sort" on an undefined value at /home/kohaadmin/src/C4/Search.pm line 405.

This patch corrects by not performing sort if there are no no results.

Steps to reproduce the error without patch:

In OPAC go to Advanced Search
Choose "Title" in first "Search for:" end enter "ccl=( and )"
Display "More options"
Set "Sort by" to "Title (A-Z)"
Click "Search" at bootom of page

Result:
Error:
Can't call method "sort" on an undefined value at /usr/share/kohaclone/C4/Search.pm line 430.

After applying the patch, try that search again.  This time,
it should report not results found with out the error message.

Alternative Test plan :
- Set OPACdefaultSortField on something else than relevance
- Perform a simple search with a wrong CCL query. For example : ccl=( and )
=> You get the messge : No results found ...

Patch behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Adds another check to prevent a bad Zebra error message.
Works as described, passes all tests and QA script.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 22:34:02 +00:00
f140207808 Bug 11416: fix case where serials item editor was incorrectly hiding fields
In serials/serials-edit.pl, if an item field is hidden from the OPAC,
it will not display in the editor, even if the field is marked as
visible in the staff intranet and editor. However, the field is still
displayed correctly in the items editor ( additem.pl ).:

Test Plan:
1) Select an item-level field ( e.g. non-public note )
2) Create a serial using the default framework ( or one of your choice )
3) For that framework, mark the chosen field as visible from the
intranet and editor, but not the opac.
4) Receive an item for this serial, note your field does not display
5) Use the biblio item editor to add an item ( additem.pl ), not the
field displayes
6) Apply this patch
7) Repeat step 4, not the field displayes

Signed-off-by: Kim Schwant <kim.schwant@courts.in.gov>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
PrepareItemrecordDisplay is only used for editor (-4 < hidden < 4)

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 21:38:07 +00:00
Galen Charlton
9064395892 Bug 11689: (follow-up) fix another warning when running Serials.t
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 21:12:31 +00:00
Jonathan Druart
d2c424eda2 Bug 11689: (follow-up) fix warnings generated when running Serials.t
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 21:10:39 +00:00
Jonathan Druart
3285e34dc2 Bug 11689: Take new serial missing statuses into account in more places
Bug 10851 introduced new missing status (codes 41,42,43,44), but in
GetSerials and _update_missinglist, they are not taken into account.

This patch corrects the issue.

To reproduce:
1/ Create a serial with 10 issues.
2/ Set different statuses on each one, with at least 6 missing statuses
(not only "Missing").
3/ Go on the subscription detail page, tab "Summary", the issues with a
new missing status are not listed in the missing issues list.
4/ On the "Issues" tab, all missing are listed (normally only 5 should
be listed.
5/ Apply the patch.
6/ Edit serial (to rewrite the missing list).
6/ Verify that steps 3 and 4 have now correct behavior.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes QA script and tests.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 21:07:59 +00:00
Julian Maurice
47a9afcb7e Bug 12003: Do not calculate next pubdate for irregular subscriptions
Show 'Unknown' when planneddate and publisheddate cannot be calculated

Also fixes SQL query in misc/cronjobs/serialsUpdate.pl that was still
using "periodicity != 32" to exclude irregular subscriptions from
results

Test plan:

1) Create a subscription in the serials module. Make sure to choose:
   Frequency = Irregular
2) Test the prediction pattern, first publication date is set to
   "First issue publication date" field, others will show as
   'unknown'
3) Save the subscription
4) Check the created issue - it will show a published date and a
   planned date (same as "First issue publication date" field)
5) Receive the issue and check the next generated issue, planned
   date and published date should show as 'Unknown'
6) Generate a next issue, planned date and published date should
   also show as 'Unknown'

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described following test plan.
No koha-qa errors

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Also tested:
- multi receiving generates mulitple issues without dates - 'unknown'
- staff detail page shows the dates empty, which is fine
- OPAC detail page shows the dates empty, which is fine
- serial collection page shows 'unknown' and those issues appear
  on the 'manage' tab, as they did in the past
- Editing the issue from the serial collection page leaves the
  date fields empty.
- Receving the issue, setting the status to 'Arrived' the Expected on
  date is set to 'today' automatically. Date published has to be
  entered manually (maybe something we could improve later
- subscription detail > issues tab shows Uknown.
- t/db_dependent/Serials/GetNextDate.t pass.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 20:57:51 +00:00
Galen Charlton
dc6d8a2199 Bug 12098: (follow-up) put can_show_subscription() into use
This patch puts C4::Serials::can_show_subscription() into use.

Note that there is user-visible change: if a subscription has a
blank library, all users with serials permissions will be able
to view and/or edit it.  It remains to be determined whether
we *want* such subscriptions to exist, or if they should only
be tied to specific libraries.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 20:47:48 +00:00
Jonathan Druart
4d78b9588a Bug 12098: Refactor can_*_subscription in C4::Serials
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested on top of patches for 12048 and 12080.

Subscription search
- superlibrarian, IndyBranches on/off - always sees all subscriptions
- superserials, IndyBranches on/off - always sees all subscriptions
- no superserials, IndyBranches on - only sees own subscriptions
Note: Subscriptions without branches will only show, when all subscriptions
      are visible. In a future enh it might be good to enforce setting a
      branch, when IndyBranches is used.
- no superserials, IndyBranches off - always sees all subscriptions

Subscription editing
- superlibrarian, IndyBranches on/off - can edit all subscriptions
- superserials, IndyBranches on/off - can edit all subscriptions
- no superserials, IndyBranches on - can only edit own subscriptons and
  subscriptions without branch
  NOTE: it would make sense to also allow Edit > Edit as new (duplicate)
  here, so one can copy the subscription from another branch to modify
  it for the own branch.

Passes tests in t, xt and QA script, also newly provided unit tests.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 20:46:00 +00:00
Jonathan Druart
4f7803e469 Bug 12098: Fix C4::Serials::can_edit_subscription
This patch fixes a problem whereby staff users could
edit subscriptions they are not permitted to by going directly
to the subscription details page.

It also adds some unit tests for the can_edit_subscription routine
and add a new can_show_subscription routines.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Notes on second patch.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 20:45:59 +00:00
b772969cdd Bug 12080: (follow-up) fix test failure and warnings in Bookseller.t
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixes the tests as promised.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 20:45:59 +00:00
f0e574be4a Bug 12080: restore effect of superserials permission
The superserials permission is meant to allow an operator
to see all subscriptions regardless of branch when IndependentBranches
is on without having to have full superlibrarian permissions.  This
patch restores this behavior.

TEST PLAN
---------
1) Apply the patch for bug 12048 (as needed -- it may be pushed)
2) Ensure you have two users: superlibrarian, non-superlibrarian
   with all access to the staff client except superserials.
3) Ensure you have serials belonging to a different branch than
   the non-superlibrarian.
3) Log into staff client as superlibrarian
4) Click 'Serials'
5) Click the 'Submit' button in the search area.
   -- note the number of results.
6) Log into staff client as non-superlibrarian
7) Click 'Serials'
8) Click the 'Submit' button in the search area.
   -- note the number should be less, note the number.
9) Give the non-superlibrarian superserials access.
10) Home -> Serials
11) Click the 'Submit' button in the search area.
   -- the number will still be the same at the one in step #8.
12) Apply the patch
13) Refresh the page
   -- the number should now match the one in step #5.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 20:45:59 +00:00
214d6b8d13 Bug 12048: restore ability of superlibrarian to see other libraries' subscriptions
This patch fixes a regression in master and 3.14. When a user has
superlibrian permissions, a search on serials subscriptions should
display other libraries' subscriptions even when IndependentBranches
syspref is enabled.

To reproduce/test the bug/patch:

1. Enable IndependentBranches (i.e. 'Prevent' staff...)
2. Login as a user not having superlibrarian permission
3. Search for a serial subscription on:
   /cgi-bin/koha/serials/serials-search.pl
4. Search a title which has at least 2 subscriptions: one in the user
   branch, and one in another branch
5. On the result page, just 1 subscription is displayed: the one
   attached to the userbranch
   => this is normal
6. Login as a user having superlibrarian permission
7. Repeat step 3-5.
8. You get the same result as 5. You should have seen all subscriptions.
   That's what you get after applying this patch.

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

NOTE: I tested a variation. My superlibrarian was a branch that
      was not the same as the non-superlibrarian. The serial was
      the same branch as the non-superlibrarian. Without the
      patch, the superlibrarian saw nothing, with the patch it
      saw the serial as expected.
      Also, remember the superserials permission can affect the
      results. I successfully changed the branch of the
      subscription, and then it ceased to show up with
      superserials not granted to the non-superlibrarian.
      I corrected the system preference name in the text here.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Superlibrarian permission now allows to see all subscriptions
independent from the branch.
Passes all tests and QA script.

But the superserials permission appears broken to me before
and after this patch. If I have superserials - the search
doesn't show all subscriptions. If I don't have superserials
I can still edit any subscription accessing the subscription
detail page through the serial collection page or accessing
the detail page directly by manipulating the URL.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 20:45:59 +00:00
064d5478d3 Bug 12071: improve generation of Z39.50 search links
This patch fixes two problems with the generation of
links to execute a Z39.50 search from the staff client
catalog and cataloguing search results page.

First, if using URI::Escape 3.30 or earlier, performing a simple search
with a double quote (e.g., "histoire algerie"), the Javascript is broken
in results page because of :

function GetZ3950Terms(){
  var strQuery="&frameworkcode=";
  strQuery += "&" + "title" + "=" + ""histoire%20algerie"";

Second, the encoding of non-ASCII characters in the search
term was broken.

This patch moves URI escaping from Perl to template with uri TT filter.

Test plan :
- To reproduce the issue with double quotes, the server
  must be running URI::Escape 3.30 or earlier; the current
  version of URI::Escape properly escapes double quote.
- In staff interface, perform a search with double quotes
  that will return no result, ie "aaa xxx"
=> Without patch, javascript is broken
=> With patch, javascript is not broken
- Click on Z3950 button on results page
=> Without patch, the Title input is empty
=> With patch, the Title input contains the search terms

Additional test:
Do a search with something like äöü and then click Z3950
button on results page.
Without patch, encoding is broken in Z3950 form
With patch, encoding is correct.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixed a few tabs. Passes tests and QA script.
I can't reproduce the Javascript problem, but I can reproduce
the Z39.50 encoding problem and can detect no regression.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 15:37:56 +00:00
7966037747 Bug 11258: fix another case where holds queue made transfer requests that contradict the library holds policy
This patch fixes a problem where the holds queue generator
was making requests where the pickup library is the
same as the item's library but not the patron's branch,
even if there is a "Default holds policy by item type" rule that states
this item can only fill holds for patrons of the same library as the
item.

Test Plan:
1) Create a test record with 2 items with different itemtypes
2) Set the Default holds policy by item type for the first
   item to "From any library"
3) Set the Default holds policy by item type for the second
   item to "From home library"
4) Place a record level hold for a patron from another library,
   but for pickup at the same library as the item is from
5) Rebuild the holds queue
6) View the holds queue, note the item is listed, though this
   patron cannot place a hold on this item
7) Apply this patch
8) Repeat step 5, note the hold is no longer in the queue

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
automated tests pass, functional tests pass. Bug replicated, eradicated by patch.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
I finally managed to reproduce this, patch works as described.
Passes tests and QA script, provided tests fail without patch, but
succeed with the patch.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-18 15:23:23 +00:00
Galen Charlton
695fdebdee Bug 12079: ensure that CheckReserves() includes reserve_id in its response
This patch modifies _Findgroupreserve so that its one caller,
CheckReserves(), would include the reserve_id field in the
hold request it returns.

Failure to include reserve_id in every circumstance resulted
in bug 11947.  This patch is therefore a complementary fix for
that bug, but is not meant to preempt the direct fix for
that bug.

To test:

[1] Verify that t/db_dependent/Reserves.t passes.
[2] Verify that the following test plan taken from
    the patch for bug 11947 works for this patch
    *without* applying the patch for 11947:

* have a few borrowers, say 4.
* have a biblio with a single item (you can scale this up, it should
  work just the same.)
* issue the item to borrower A
* have borrowers B, C, and D place a hold on the item
* return the item, acknowledge that it'll be put aside for B.
* view the holds on the item.

Without the patch:
* the hold priorities in the UI end up being "waiting, 2, 1" when they
  should be "waiting, 1, 2".
* in the database "reserves" table, they're really "0, 2, 3" when they
  should be "0, 1, 2".

With the patch:
* the hold priorities in the UI end up being "waiting, 1, 2"
* in the database, they're "0, 1, 2"

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Work as described. No koha-qa errors. Test pass

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-17 15:32:25 +00:00
Galen Charlton
b324e893e8 Bug 11630: (follow-up) tighten up code
This patch tightens up the initialization of the $restriction_age
variable and uses $bibvalues throughout.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-16 15:32:29 +00:00
31995cbcd1 Bug 11630: (QA Followup) Move code to subroutine, add unit tests
Patch behaves as expected.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-16 14:55:08 +00:00
Pasi Kallinen
19badb2db4 Bug 11630: improve parsing of age restriction markers
This patch makes the parsing of AgeRestrictionMarker values consider
the case where the marker is immediately followed by the age, e.g.
"K16" in Finland.

How I tested:

[1] Configure Age Restricition (see Syspref AgeRestrictionMarker) and
have a biblio record with e.g. PEGI 99 in age restriction field.
[2] Try to check out to a patron with age < 99
[3] Check out should be blocked
[4] Change entry in age restriction field to PEGI99
[5] Checkout should be possible
[6] Apply patch
[7] Checkout schould now be blocked

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-16 14:43:30 +00:00
Galen Charlton
cc7a99ba7f Bug 11797: (follow-up) fix MARC21 branch of bug more idiomatically
This patch puts the MARC21 variant of the bugfix in alignment
with the UNIMARC variant, removing the use of unnecessary
temporary variables.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 16:01:31 +00:00
Jonathan Druart
fa989631f6 Bug 11797: fix odd number of elements in hash (UNIMARC)
I got the same warning for my UNIMARC DB.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 16:01:31 +00:00
Mark Tompsett
129bc32811 Bug 11797: fix odd number of elements in hash warning (MARC21)
This was discovered when someone triggered an authority search
on an authority record that was missing what is assumed the
default subfield for a given field.

It, however, also can be triggered in an OPAC authority search
by looking at the record that lacks the default subfield for a
given field.

TEST PLAN
---------
1) Create an authority record with 180$x and NOT 180$v.
   See C4::AuthoritiesMarc::BuildSummary in the 1.. foreach loop
   for known tags and default values. The default subfields are
   the first letter of the $subfields_to_report string.
2) Trigger the bug:
   Method 1: /cgi-bin/koha/opac-authoritiesdetail.pl?authid=#
             Where # is the authority id of your tweaked record.
             The error occurs in Normal view.
   Method 2: Home -> Cataloging -> + New record
                  -> Click the 'Tag Editor' on 100$a
                  Editing of $a to $b and back may be required.
3) Notice there is an error log entry.
4) Apply the patch
5) Attempt to trigger the bug again
6) That specific error log entry is not generated.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Could generate the warning with a missing 151$a with both methods.
No warning anymore after applying this patch.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 16:01:31 +00:00
Bernardo Gonzalez Kriegel
c212b9328e Bug 5052: make it possible to pick a language if all choices are sublanguage
This was tricky to catch. In current implementation, Bug 6755
introduced in C4/Templates.pm as condition to send the array of
hashrefs of languages that (@$languages_loop<2), but with one
language group that condition is false, there is only one
element in that array.

This patch changes that condition to have more than one language
selected, grouped or not.

Also send $bidi value always, that was only sent if there is
more than one group language.

To test:
1. Translate to en-GB and en-NZ, or simply do mkdirs
on intranet-tmpl/prog and opac-tmpl/bootstrap

2. Go to Administration > System preferences > I18N
enable those languages on staff/opac

3. Check that language chooser is nowhere to be found

4. Apply the patch

5. Reload staff/opac, now you can see language chooser

NOTE: I made little changes on staff, but can't replicate
bootstrap colors for selected/unselected language. Someone
need to touch css files to make it happen. But that is
current behavior.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Good catch!

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 14:36:29 +00:00
Robin Sheat
95056d17b7 Bug 11947 - renumber reserves when hold is confirmed
Currently when a reserve is moved to "waiting" status because it's
acknowledged on checkin, the reserve priorities aren't renumbered. This
causes things to go a bit haywire in the UI, in particular, some
reserves can unjustly end up with priority 1 when they shouldn't. It
also seemed to mess with the logic of who should get it next, but I
didn't look too closely at that.

This patch forces a renumbering so that all the priorities remain
copacetic.

Test plan:
* have a few borrowers, say 4.
* have a biblio with a single item (you can scale this up, it should
  work just the same.)
* issue the item to borrower A
* have borrowers B, C, and D place a hold on the item
* return the item, acknowledge that it'll be put aside for B.
* view the holds on the item.
Without the patch:
* the hold priorities in the UI end up being "waiting, 2, 1" when they
  should be "waiting, 1, 2".
* in the database "reserves" table, they're really "0, 2, 3" when they
  should be "0, 1, 2".
With the patch:
* the hold priorities in the UI end up being "waiting, 1, 2"
* in the database, they're "0, 1, 2"

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

Test plan confirms that the problem exists and that the patch corrects
it.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script, especially t/db_dependent/Reserves.t.
Improves priority calculation.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-15 14:18:19 +00:00
Jonathan Druart
9132f9befb Bug 11960: replace unnecessary call of GetMemberDetails by CanBookBeRenewed
C4::Circulation::CanBookBeRenewed called C4::Members::GetMemberDetails to
retrieve categorycode and branchcode.

- categorycode is used to retrieve the issuing rule
- the borrower information is passed to
  C4::Circulation::_GetCircControlBranch. Which only uses the branchcode
  parameter.

GetMemberDetails does a lot of calls/queries (patronflags,
account, etc.) that are not needed by CanBookBeRenewed.

This patch replaces it with a call to C4::Members::GetMember.

Note: I presented this small optimisation during a quick introduction to
NYTProf (hackfest 14 in Marseille).

Test plan:
- launch member unit tests
- check the code

Checking the code resulted in the following:
CanBookBeRenewed builds a hash reference from the borrowernumber
(2482). Note it is only used in this function and not passed in.

_GetCircControlBranch (2485) requires that hashreference to
have a branchcode key. As stated above.

The following line (2486) requires it have a categorycode key.

As such, C4::Members::GetMemberDetails is confirmed to be
overkill, and C4::Members::GetMember is sufficient.

Testing Done
------------
0) Back up DB
1) Make sure MPL is in the list of libraries.
2) Apply the patch.
3) run the koha qa test tool
4) prove -v t/db_dependent/Circulation.t

Patch applies cleanly. QA Test tool was all OK. All tests ran successfully.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-11 16:22:43 +00:00
612b33ae69 Bug 11711 - Use new DataTables include in circ templates
Bug 10649 introduced a new include file for adding DataTables-related
JavaScript assets. This patch adds use of this include file to all
circ-related pages which use DataTables.

Apply the patch and test the following pages to confirm that table
sorting works correctly:

- Circulation
  - The UseTablesortForCirc system preference must be enabled.
  - Check out to a patron with existing checkouts. Choose a patron who
    is a guarantor to another patron with checkouts in order to test the
    relatives' checkouts table.
  - The checkouts and relatives' checkouts tables have been modified to
    exclude articles when sorting of titles.

- Hold ratios - The title column has been configured to exclude articles
  from sorting

- Transfer to receive

- Holds queue
  - The title column has been configured to exclude articles when
    sorting
  - The date column has been modified to use the title-string filter for
    sorting. An unformatted date is now passed from C4::HoldsQueue.pm to
    the template, where the KohaDates filter is used for formatting.
    Sorting is based on the unformatted date.

- Holds awaiting pickup
  - The "available since" column has been configured for sorting on an
    unformatted date. waitingreserves.pl now passes the unformatted
    date to the template, and formatting is done using the KohaDates
    filter.
  - The title column has been configured to exclude articles when
    sorting.

Edit: Rebased on current master following commit of Bug 11605
Signed-off-by: A. Sassmannshausen <alex.sassmannshausen@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-08 23:47:27 +00:00
Galen Charlton
f4efda5bc5 Bug 7413: (follow-up) improve return of GetSoonestRenewDate()
This patch modifies GetSoonestRenewDate() so that it returns
undef if the patron, item, or loan cannot be found.  This
better reflects the usage of this routine GetSoonestRenewDate(),
as none of its callers tried to check the second return
value containing an error code.

This patch also updates the POD to match.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-08 22:52:01 +00:00
Holger Meißner
065926d224 Bug 7413: Code and intranet template changes
This patch modifies CanBookBeRenewed, so that based on
issuingrules.norenewalbefore a new error "too_soon" can be returned.
Also adds a new subroutine GetSoonestRenewDate.

To test:
1) Create an issuing rule with "No renewal before" set to value X
   and "Unit" set to days.
2) Test the following steps for both:
   Home > Patron > Patron details
   Home > Circulation > Checkouts
3) On the checkout page, test for today's issues as well as previous
   issues. (Check something out on one day and something else on the
   next day, then do the testing.)
4) Confirm that items can't be renewed if current date is more than
   X days before due date.
5) Confirm that the date and time of the soonest possible renewal are
   displayed in the format specified by global sysprefs "dateformat"
   and "TimeFormat".
6) Confirm that items can be renewed if "No renewal before" is
   undefined or current date is X or less days before due date.
7) Confirm that if the number of allowed renewals is exceeded
   "Not renewable" is displayed, no matter what "No renewal before"
   is set to.
8) Test the same things with "Unit" set to hours.

Sponsored-by: Hochschule für Gesundheit (hsg), Germany
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2014-04-07 18:28:57 +00:00
Galen Charlton
c095003b86 Bug 7567: (follow-up) construct values clause in SQL statement more idiomatically
To test:

[1] Verify that prove -v t/db_dependent/NewsChannels.t passes

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:14:21 +00:00
Jonathan Druart
ea6cffdd9f Bug 7567: QA followup: FIX SQL query
The WHERE clause should not erase $query.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:14:19 +00:00
Mark Tompsett
53f15f678f Bug 7567 - Use, display, filter News by library
This adds the ability to specify whether staff, OPAC,
or slip news entries apply to all libraries or just a
particular library.

With the branch parameter added to key functions in
C4/NewsChannels.pm, function calls in C4/Members.pm,
mainpage.pl, opac/opac-main.pl, tools/koha-news.pl, and
t/db_dependent/NewsChannels.t were needed.

Some license texts were updated.

Templates were modified to display, allow for entry and editing
of the branches selected.

TEST PLAN
---------
1) Having logged into the staff client, is the news displaying
   correctly? Have you entered a news item which should not
   display for this branch of logged in user?
2) Find a patron (with some items checked out?)
3) Print a slip
   - News which is labelled 'All Branches' or for the same branch
     as the one printing the slip should display on the slip.
   - THIS DOES NOT AFFECT QUICK SLIPS
4) Home -> Tools -> News
   - Can you edit a news item?
   - Does the change save correctly?
   - Can you filter based on location and branch correctly?
   - Can you add a new entry correctly?
   - Can you delete an entry correctly?
5) Open an OPAC client.
   - Does only the news for all branches display?
6) Log into the OPAC client.
   - Does the news for all branches and the specific branch display?
7) prove -v t/db_dependent/NewsChannels.t
   - Does it run and all succeed?
   - Does the code seem to catch the required cases?
8) Comparing the patched and unpatched versions of files affected,
   are the license changes missing anything?

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:14:19 +00:00
Mark Tompsett
2f2a0cf53e Bug 7567: convert news add/update routines to take hashref; fix bugs
Changed the add and update functions to use a hash reference
for the parameter, so that adding or subtracting parameters
should be easier. Added some POD for the add_opac_news and
upd_opac_news functions, so that developers would know how to
call it.

The hashref changes resulted in being able to return 0 for
failure and 1 for success. This meant adding a couple tests
to the test file.

And while testing, there was some sort of logic problem with
the matter of '' being all, but selecting all only showed
things set for all, and excluded particular languages, or other
interfaces.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:13:56 +00:00
Mark Tompsett
d3eaa62717 Bug 7567: (code cleanup) remove finish calls
"When all the data has been fetched from a SELECT statement,
the driver will automatically call finish for you. So you should
not call it explicitly except when you know that you've not
fetched all the data from a statement handle and the handle
won't be destroyed soon."
(http://search.cpan.org/~timb/DBI-1.627/DBI.pm#finish)

All the $sth variables were scoped within the functions,
and would be destroyed immediately. Additionally, there was
one after a SELECT, for only a single idnew, and so it was
not necessary.

TEST PLAN
---------
1) prove -v t/db_dependent/NewsChannels.t
2) apply patch
3) prove -v t/db_dependent/NewsChannels.t

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

ok 1 - use C4::NewsChannels;
ok 2 - Successfully added the first dummy news item!
ok 3 - Successfully added the second dummy news item!
ok 4 - Successfully updated second dummy news item!
ok 5 - Successfully tested get_opac_new id1!
ok 6 - Successfully tested get_opac_new id2!
ok 7 - Successfully tested get_opac_news!
ok 8 - Successfully tested GetNewsToDisplay!

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:05:41 +00:00
Mark Tompsett
b56f092299 Bug 7567: (code cleanup) update license and use Modern::Perl
Grabbed the current license from
  http://wiki.koha-community.org/wiki/Coding_Guidelines#Licence
and changed the use strict; use warnings; into a
use Modern::Perl instead.

TEST PLAN
---------
1) Log into staff client.
    - Does news look okay?
2) Apply patch
3) Refresh staff client.
    - Does news look the same?

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

Safe no op action

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-04-07 18:05:14 +00:00