Commit graph

322 commits

Author SHA1 Message Date
5c190388ae Bug 26133: Remove GetMarc* calls in detail.pl
There are several calls in catalogue/detail.pl that can be removed:
GetMarcISBN, GetMarcAuthors, GetMarcSubjects, GetMarcSeries, GetMarcUrls and GetMarcHosts
They pass a variable to the template that is never used.

Test plan:
Confirm that this TT variable is never used.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-08-05 17:36:28 +02:00
50302c714e Bug 26063: Use Koha::Plugins->call for some other hooks
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-30 17:44:23 +02:00
9676f537a4 Bug 25961: Add hooks for plugins to inject variables to OPAC XSLT
This patch adds the following plugin hooks:
- opac_results_xslt_variables
- opac_detail_xslt_variables

This hooks will inject variables returned by the plugin in the form of a
hashref, into the ones that are passed to the XSLT processing code.

To test:
1. Apply the 'DO NOT PUSH' commit
2. Install the Kitchensink plugin
3. Restart all
4. Search biblios in the OPAC
=> SUCCESS: A text is injected in front of the biblio title
5. Enter the detail page of any of the results
=> SUCCESS: A text is injected in front of the biblio title
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-30 17:30:23 +02:00
dbaf146266 Bug 25801: Add itemnumber parameter to opac-detail
Note: The GetItemsInfo call is now suboptimal. Leaving it as-is
for now in the hope that item refactoring picks it up ;)

Test plan:
Test opac-detail via biblionumber (regular use).
Test opac-detail by passing an itemnumber in the URL:
    /cgi-bin/koha/opac-detail?itemnumber=999

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

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-07-20 17:45:31 +02:00
2a216e206c
Bug 25416: Let OPAC XSLTs know if the context is an anonymous session
This patch makes use of the 'variables' parameter in XSLTParse4Display
method in the different places that it is used in the OPAC. It does by
passing this parameter with

    anonymous_session => 1|0

The value will depend on the output from get_template_and_user (i.e. if
there's a returned borrowernumber).

A special case takes place in search results, as the call to
XSLTParse4Display happens in C4::Search::searchResults. So a new
parameter 'xslt_variables' is added to it.

To test:
1. Apply the [DO NOT PUSH] patch
2. Open the OPAC in your browser
3. Try detail pages, search results, tags and lists/shelves pages with
   or without an active session
=> FAIL: It always says (somewhere) 'Anonymous session: Yes'
4. Apply this patch, restart_all
5. Repeat 3
=> SUCCESS: It will tell the Yes/No correctly regarding anonymous
sessions!
6. Sign off :-D

Sponsored-by: Universidad ORT Uruguay
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-15 09:33:22 +01:00
Nazlı Çetin
660edd0524
Bug 13547: Add item field 3 (Materials specified) to the OPAC holdings table
Test plan:

1- View a record with Materials specified (field 3) data in the opac
2- Apply patch
3- Log in to staff client
4- Home->Administration->Column Settings->OPAC->holdingst
5- Set item_materials visibility
6- Refresh OPAC page
7- Confirm that the materials specified column has been added after the
   Call number column.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-12 11:44:32 +01:00
Katrin Fischer
117b55c702
Bug 24854: Remove IDreamBooks integration
The IDreamBooks service has not seen updates in a long time, so
we should remove the service from Koha as it's no longer operational.

To test:
- Apply patch and run the database update
- Verify that the IDreamBooks related system preferences are gone
- Verify that opac detail pages and result lists still work
  as expected
- Run t/db_dependent/UsageStats.t

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-24 08:07:23 +00:00
0a07597f20
Bug 17896: Remove duplicated use statements
and remove uneeded '&'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-21 15:43:57 +00:00
2dc33d4df2
Bug 17896: load BakerTaylor module with use
We are incinsistent here, Amazon and Syndetics module are always loaeded in some places
BakerTaylor is conditional everywhere, and causes issues under plack

For simplicity sake I think we should just load this (small) module where it might be needed

To test:
1 - Disable Baker and Taylor images
2 - Restart plack
3 - Visit opac-readingrecord, opac-detail, opac-search, opac-shelves, opac-user
    Log in to opac
    Virew your reading history
    Make/view a list
    Search the catalog
    Look at an individual title
4 - Enable BakerTaylorEnabled
    If you don't have Baker and Taylor credentials, simply fudge them with bad data and enable
5 - Repeat steps above, in the word of Joubu "Kaboom"
6 - Apply patch
7 - Repeat 1-4
8 - You shoudl be able to load all the pages after enabling the pref

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-21 15:42:58 +00:00
82716a0172
Bug 23084: Replace grep {^$var$} with grep {$_ eq $var}
We certainly faced 3 similar bugs due to this syntax: bug 23006, bug
22941 and bug 17526.

To prevent other issues related to this syntax this patch suggests to
replace them all in one go.

Test plan:
Confirm that the 2 syntaxes are similar
Eyeball the patch and confirm that there is no typo!

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-17 10:44:45 +00:00
Jesse Weaver
1c43a26525
Bug 18936: (follow-up) Fix tests, replace old get_onshelfholds_policy method
Signed-off-by: Minna Kivinen <minna.kivinen@hamk.fi>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-04 09:56:25 +00:00
cce358f9af
Bug 23846: Handle exception gracefully at the OPAC
I do not think we should have the same trick as the intranet, and
display a message. This should be enough.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-11-13 08:04:23 +00:00
Agustin Moyano
426a055a07
Bug 22581: Show and play musical inscripts
This patch adds musical inscripts to OPAC's detail page

To test:
1. run previous patch test plan
2. apply this patch
3. edit a the marc structure of a MARC bibliographic framework, and in tag 031 enable the following subfiels to be visible in editor: 2, g, n, o, p, u
4. search the catalog for a record that belongs to that framework, and edit tag 031 with the following:
   * 2:pe
   * g:G-2
   * n:xFCGD
   * o:3/8
   * p:'6B/{8B+(6B''E'B})({AFD})/{6.E3G},8B-/({6'EGF})({FAG})({GEB})/4F6-
   * u:http://nonexistent.org/url/of/a/midi
5. save and click in opac view
CHECK => even though you add a 031 tag there is no musical inscript shown in opac view
6. in admin module enable OPACShowMusicalInscripts preference
7. refresh opac view
SUCCESS => it takes a few seconds to load, but you see a link that says 'Audio file' pointing to the URL you placed in 'u' subfield, and below you see the musical inscript
8. in admin module enable OPACPlayMusicalInscripts preference
9. refresh opac view
SUCCESS => You see a play button below the musical inscript, and when you click, the song is played
10. sign off

Sponsored-by: Biblioteca Provincial Fr. Mamerto Esquiú (Provincia Franciscana de la Asunción)
Co-authored-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-11-03 08:11:38 +00:00
d09083fc87
Bug 23392: Don't display private notes in MARC21
To test:
1 - Add some notes to a record in fields 541,542,561,583,590
2 - Ensure all of these are visible in the frameworks
3 - Note they appear in the 'Title notes'/'Description' tabs on OPAC/Staff client
4 - Mark first indicator '0' on all notes
5 - They still display
6 - Apply patch
7 - Notes no longer show on OPAC
8 - Notes still show on Staff client
9 - prove -v t/Biblio/GetMarcNotes.t

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-10-24 10:58:02 +01:00
Ere Maijala
a1a05db1b6
Bug 11529: Add templates for biblio title display. Unify display.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-08-05 15:03:19 +01:00
Ere Maijala
4ea26c0a69
Bug 11529: Use new biblio fields whenever possible
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-08-05 15:03:17 +01:00
fd5686b156
Bug 12537: Don't retrieve XISBN results for the same biblionumber
For a biblio with multiple ISBNS we sometimes get our own record back when
check XISBN, we should test for this

To test:
1 - Edit a record in the catalogue, add two isbns:
     0521240670
     0521284198
2 - Enable ThingISBN and FRBRizeEditions and OPACFRBRizeEditions
3 - View the record in staff and OPAC
4 - You should see editions tab pointing to the same record
5 - Apply patch
6 - Reload the record details, you should no longer see editions tab
7 - Add the second ISBN to another record
8 - Reload details for original record, you shoudl see editions linking to the record with second ISBN
9 - prove -v t/db_dependent/XISBN.t

NOTE: Current tests don't work under elasticsearch, but the code does, tests should be rewritten on another bug

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Arthur Bousquet <arthur.bousquet@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-07-15 11:27:59 +01:00
14e6375637
Bug 14419: Expanding facets (Show more) performs a new search
This patch removes the constraint of only passing 5 facets to the template unless the list is expanded, in fact, it removes the 'expanded' attribute from Search.pm
Now that all facets are passed to page it adds a 'show more' link at the bottom of lists and allows user to expand or collapse any facet set without reloading page.

Updated tests included.

To test:
1  - Perform an OPAC search that returns more than 5 of any given facet type
2  - Click the "Show more" link on the facets and see that the search is reloaded
3  - Apply patch
4  - Repeat search
5  - Note that you can click "Show more" without reloading page
6  - Test that page load is not greatly affected
7  - Ensure that all facet links function normally
8  - Ensure that facets are the same a prior to patch
9  - Repeat for staff client
10 - Prove t/Search.t

NOTE: This patch makes it much easier to see that there is an existing issue with marking the "active" facet.  Ending punctuation seems to confuse the matcher.

Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-07-04 09:22:02 +01:00
d99d32d033 Bug 8995: (follow-up) Add tests, move open_url/coins routines to Koha namespace
Test plan:
1) Ensure the COinS span tag is still included on this pages. You need
to look into html source and search for span tag with class 'Z3988',
   which has COinS string in title.
   Staff client:
       catalogue -> ISBDdetail
       catalogue -> MARCdetail
       catalogue -> detail
       virtualshelves -> shelves
    OPAC (you should have COinSinOPACResults system preference enabled):
        opac detail
        opac search
        opac shelves
2) Run tests:
prove t/Biblio.t t/db_dependent/Biblio.t t/db_dependent/Koha/Biblio.t

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested with all 9 current patches. Works as advertised, including
OPACURLOpenInNewWindow. If a record has no items, no OpenURL link
is displayed. All the suggested tests pass. I did not test with
XSLT turned off.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-29 15:34:10 +00:00
2331b8a295 Bug 22360: (bug 21205 follow-up) Restore OPACAcquisitionDetails behavior
Caused by
  commit 7d10549ae8
  Bug 21205: Replace C4::Items::GetOrderFromItemnumber calls

At this point $order is a Koha::Acquisition::Order object, not a hashref
anymore.

Test plan:
Create an order, receive items
Enable OPACAcquisitionDetails
At the detail page of the bibliographic record you should see
"X items are on order." at the bottom of the items list

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-03-04 18:20:54 +00:00
Katrin Fischer
bc96fe5b41 Bug 12318: Show shelving location on subscription tab in OPAC and staff
The shelving location can be helpful to locate an item in the library.
Especially, if the library has decided not to create items for a
subscription this information is currently not visible to the patron.

To test:
- Apply patch
- Create a subscription, set location
- View the subscription tab in detail and staff
- Verify that the location now shows above the callnumber
- Unset location in the subscription
- Verify that the page still looks ok

Signed-off-by: Mikaël Olangcay Brisebois <mikael.olangcay-brisebois@inLibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-02-27 09:14:21 -05:00
7266d50dd2 Bug 22140: Use of EasyAnalyticalRecords pref in search
Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, there are other places where code must be dependant on this pref.

Test plan :
1) Build an analitical record with 773$0 and $9
2) Enable EasyAnalyticalRecords
3) Don't apply patch
4) Go to OPAC
5) Perform a search that displays the record, check there is the linked item
6) Open record detail page, check there is the linked item
7) Apply patch and redo 5) and 6)
8) Disable EasyAnalyticalRecords
9) redo 5) and 6), you should not see the linked item

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-02-15 13:34:57 +00:00
7d10549ae8 Bug 21205: Replace C4::Items::GetOrderFromItemnumber calls
This is done to ease the move of C4::Items (bug 18252) to Koha::Items

  my @itemnumbers = GetItemnumbersFromOrder($order->{ordernumber});
will become
  my @itemnumbers = $order_object->items->get_column('itemnumbers');

Test plan:
- Create an order with several items
- Receive some items

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 20:47:16 +00:00
70651422a7 Bug 14385: (QA follow-up) Additional changes and fixes
[1] searchResults: second my $interface can be removed: unused
[2] call of getitemtypeimagelocation on L2119 needs interface key
[3] ISBDdetail: No need to find patron again (line 182 vs 84)
[4] opac-search: No need to find patron twice (657 and 631)
[5] tabs on line 2220 of C4/Search.pm (qa tools warn)
[6] Ugly hack to overcome "Undefined subroutine &C4::Items::ModZebra"
    by loading C4::Items before C4::Biblio when running tests
    Koha/BiblioUtils/Iterator.t and Labels/t_Label.t.
    This is a more general problem that needs attention somewhere else.
    It seems that Biblio.pm is one of the suspects.
[7] This patch set makes Search.t crash/fail with me. Note that without
    these patches Search.t still passed! Why o why..
    A little debugging pointed me to a missing MPL branch (aarg).
    Adding the simple test on the result of Libraries->find in
    C4::Biblio::GetAuthorisedValueDesc made the test continue.
[8] Resolve: Variable "$borcat" is not available at opac-detail.pl line 246
    Lexical $borcat cannot be used in sub searchAgain in opac-detail.pl
    under Plack. Must be defined with our (or passed as argument).
[9] Resolve crash on TWO serious typos in opac-basket on ONE line:
        Koha::Patron->find({ borrowernumber -> $borrowernumber })
    Yeah: find is in Koha::Patrons and we need => !!
    No need to pass a hash to find method btw for a pk value.
[10] Serious bugfixing here. Add List::Util to opac-basket.
    Can't locate object method "none" via package "1".
    You can't test everything :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
After this longer list I renamed Final to Additional in the patch title :)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:33:12 +00:00
Mark Tompsett
e1b5fa657d Bug 14385: Squash of a lot of patches rebased
- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!
- Rebased t/db_dependent/Items.t conflict.

The test plan is in comment #112 last I checked.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:33:12 +00:00
Chris Cormack
5e4e10c4ca Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories
Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

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

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:33:09 +00:00
8d0e08fdf5 Bug 21475: Fix crash on missing default itemtype
Test plan:
Enable ArticleRequests.
Find book without itemtype (942c). Maybe you need this:
  update biblioitems set itemtype=NULL where biblionumber=[...]
Goto opac detail for that book. No crash anymore?

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-19 15:51:26 +00:00
e85d6e12ea Bug 17530: (QA follow-up) Move may_article_request to ItemType
As requested by QA, we should move may_article_request out of Biblio.

For reasons of performance removed the wrapper layer of may_article_request
in opac-search. No need to look up all item types. For readability kept
the routine in the detail scripts.

Note for running ArticleRequests.t: A possible failure on the subtest
'search_limited' is addressed on bug 20866. So you can ignore that one.
As long as the subtest for may_article_request passes.

Test plan:
See previous patches.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-07 13:16:08 +00:00
b6813142dd Bug 17530: Use can_article_request to control sidebar link
Before this patch, the 'Request article' link is displayed whenever the
pref is enabled. In many cases this might be useless. Instead of a guess
as in opac-search, we now call can_article_request to know for sure.
Note: at least this is the case when a user has logged in.

Update sidebar template with template variable artreqpossible.
Add code in opac-detail, MARCdetail and ISBDdetail to fill it.

Test plan:
[1] Look for two biblios with items: one that should allow article requests
    and one that should not (respecting branch, patron, item type).
[2] Verify on detail, ISBD and MARC that the link is displayed for
    the first biblio and hidden for the second biblio.

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

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-07 13:16:07 +00:00
662e64f766 Bug 20898: Replace OPAC detail's results browser with non-JavaScript version
This patch moves generation of the OPAC detail page's results browser
from JavaScript to the template. This makes the template easier to
understand and easier to debug. It also makes it possible for the widget
to be completely non-dependent on JavaScript.

To test, apply the patch and regenerate the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client)

 - Enable the OpacBrowseResults system preference and perform a search
   in the OPAC which will return multiple results.
 - Click on any title in the first page of search results.
 - On the bibliographic detail page there should be a "Browse results"
   link in the right-hand sidebar just as before.
   - Test that the "Previous," "Back to results," and "Next" links work
     correctly.
   - Click the "Browse results" link. A list of the first 20 search
     results should appear. An arrow should indicate the title you're
     viewing.
   - Click any title in the results browser. The page should correctly
     load that record.
   - Clicking the numbered links at the top of the results browser
     should do the same.

Signed-off-by: Cab Vinton <bibliwho@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-06 16:54:07 +00:00
d8a3fae361 Bug 20737: Use https for baker and taylor cover images
Easy change, should be able to verify with code review or testing with
dummy values

To test:
1 - Put some values in baker and taylor prefs (don't need to be valid)
2 - Do a search on the opac (and have some items with isbns)
3 - Inspect the cover images links to ensure they are formed correctly
4 - prove -v t/External/BakerTaylor.t

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-05-11 11:36:23 -03:00
5c7ff786d5 Bug 19855: Move getalert, addalert and delalert to Koha::Subscription
This patch removes 3 subroutines from C4::Letters:
- getalert
- addalert
- delalert

And add 3 methods to Koha::Subscription:
- subscribers
- add_subscriber
- remove_subscriber

It makes the code cleaner for future cleanup.
TODO - we should remove alert.alertid and alert.type, and rename
alert.externalid with alert.subscriptionid
That way alert will be renamed borrowers_subscriptions (or similar) and
will become a simple join table between borrowers and subscriptions.
We will need to deal with FK that could not be satisfied.
Let's do that after this patch is pushed.

Test plan:
Subscribe and unsubscribe to email notifications sent when a new issues
is available.
Make sure everything works as before and you receive the emails.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-23 14:22:15 -03:00
0bd1f30c8c Bug 19855: Remove $type from the alerts
It looks like this feature has never been finished. It has been
developed with more flexibility in mind, but only 'issue' is used for
this parameter. Apparently it could have been 'virtual', for virtual shelves.

Let remove this parameter and clean the code a bit.
TODO: Remove the DB column

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-23 14:22:15 -03:00
5c3ead6ecd Bug 20422: Fix warning on uri_escape_utf8 in Output.pm
When opac-details calls parametrized_url, it triggers an uninitialized
warning when you would have a record without e.g. author, like:
    Use of uninitialized value in subroutine entry at /usr/share/perl5/URI/Escape.pm line 184.

This is (imo) actually a bug in URI::Escape; it should check its args.
But we resolve the warning here by adding the "// q{}" in parametrized_url.

NOTE: Along the way we do something similar in the arrParamsBusc loop.
If the variable is undefined, jump to the next one. (Consistent with the
approach in the if-part preceding it.)

Test plan:
[1] Run t/Output.t again. Should pass now.

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

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-03-26 17:31:13 -03:00
6d1525dbf1 Bug 20321: Remove get_biblionumber_from_isbn
To test:
1 - grep get_biblionumber_from_isbn
2 - verify all occurences are not actual calls (except for test)
3 - Apply patch
4 - grep get_biblionumber_from_isbn
5 - Verify it is removed

Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-03-19 13:55:47 -03:00
0ab22e1c7c Bug 18789: Send Koha::Patron object to the templates
In order to simplify and make uniform the code, the controller scripts send
a Koha::Patron object to the templates instead of all attributes of a patron.

That will make the code much more easier to maintain and will be less
error-prone.

The variable "patron" sent to the templates is supposed to represent the
patron the librarian is editing the detail.

In the members module and some scripts of the circulation module, the
patron's detail are sent one by one to the template. That leads to
frustration from developpers (making sure everything is passed from all
scripts) and to regression (we got tone of bugs in the last year because
of this way to do).
With this patch set it will be easy access patron's detail, passing only
1 variable from the controllers.

Test plan:
Play with the patron and circulation module and make sur the detail of
the patron you are editing/seeing info are correctly displayed.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-16 13:03:58 -03:00
149cb50149 Bug 19301: (QA follow-up) Add POD, use statements and correct typo
Add POD for new sub in Koha/IssuingRules.pm.
Adding use Koha::IssuingRules to opac-detail and opac-MARCdetail.
Adding use Koha::Items to opac-detail and opac-MARCdetail.
Correct typo $items => $item in opac-MARCdetail.pl.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-13 13:36:00 -03:00
39e1fbcbe9 Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy
Following the pattern introduced by bug 19300, we are going to move the
OnShelfHoldsAllowed logic to Koha::IssuingRules->get_onshelfholds_policy

Test plan:
Make sure the onshelfholds policy is correct when placing a hold

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-13 13:36:00 -03:00
Alex Arnaud
a3c922c2a1 Bug 4319: (QA follow-up) Rename hasItemswaitingOrInTransit to has_items_waiting_or_intransit
and udate pod

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-13 13:35:44 -03:00
9905247a48 Bug 4319: (QA follow-up) Use ReservableItems in all scripts
[1] Call CountItemsIssued or hasItemswaitingOrInTransit when needed only.
[2] Add this logic to ISBD and MARC detail too, since they also use
    this include.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Confirming that Place hold now comes up if you have a waiting item and
circulation rule == If any unavailable.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-13 13:02:23 -03:00
Alex Arnaud
a9779b67d6 Bug 4319: [OPAC] Allow holds on waiting/transit items
Test plan:

 - Checkout an item
 - Place hold on this item,
 - Return the item
 - Make sure the hold is waiting (found W) and AllowOnShelfHolds is
   not to 'Allow'
 - Check that the button "Place hold" appears in opac detail page of
   the biblio

 - do the samewith items/reserves in transit

Changes on C4::Reserves::IsAvailableForItemLevelRequest

Make sure this tests pass:
  - t/db_dependent/Reserves.t
  - t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t

Rebased - 2017-12-12 - Alex Arnaud

Bug 4319 - [QA fix] Create Koha::Biblio->hasItemswaitingOrInTransit

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-13 13:02:23 -03:00
2ba4af723c Bug 19319: Only fetch the record if it exists
We already know if the bibliographic record exists (404 redirect),
we can avoid unecessary fetches

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-09 16:02:25 -03:00
950fc8e101 Bug 19319: Reflected XSS Vulnerability in opac-MARCdetail.pl
Try going to this URL on your site: /cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2"><TEST>

Test Plan:
1) Go to /cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2"><TEST>
2) Note <TEST> is embedded all over the html
3) Apply this patch
4) Refresh the page, note the injection is gone!
5) run koha qa test tools

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

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-09 16:02:25 -03:00
2cd52f68cb Bug 19808: (follow-up) Handle deleted reviewers gracefully - opac-detail
Make the tests easier to read

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-21 11:07:37 -03:00
Victor Grousset
12882d824e Bug 19808: Handle deleted reviewers gracefully - opac-detail
And other display issues when the patron was NULL.
Which allows to keep the review even if it has no patron.
Because it might be useful.

For example when disconnected, the borrowernumber is null. So the
comments from deleted patrons were displayed as if the disconnected
user wrote them. So it had the edit button...

And fix borrowernumber not being passed to the template when
OpacStarRatings was false.

Test plan
1. Log in as a patron
2. Leave a comment/review on a record
3. Librarian: approve this comment
4. Delete the borrower
5. See the record (opac:/cgi-bin/koha/opac-detail.pl?biblionumber=RELEVANT_BIB_NUMBER)
6. Then you should see an error
7. Apply this patch
8. Refresh the page
9. Then you should see the record page with the comment

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

Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-21 11:07:37 -03:00
Mark Tompsett
a6d709dcc3 Bug 19576: Remove extra 'use Koha::Biblios' statement
Marcel noticed this while QA'ing another bug.

TEST PLAN
---------
Apply patch and confirm the page still loads and works as expected.
Run Koha QA Test tools

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-03 12:59:10 -03:00
e711c8e418 Bug 19038: Remove the OPACShowBarcode syspref
This patch removes the OPACShowBarcode syspref in favour of the new
columns settings option introduced by bug 16759.

On the upgrade step, it picks the value for OPACShowBarcode and uses it
to populate the columns_settings table.

To test:
- Verify the upgrade process maintains the current behaviour

Regards

Sponsored-by: Dover

Followed test plan and works as expected. Functionality of patch from bug 16759
appears intact too.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-25 16:12:46 -03:00
662a98345a Bug 19028: Add 'shelving location' to holdings table in detail page
This patch adds the option to show shelving locations on a separate
column. This is controlled by a new syspref, 'OpacLocationOnDetail',
which replaces 'OpacLocationBranchToDisplayShelving', adding a
conveniente 'column' option.

The new 'Shelving location' column is conveniently added to the columns
configuration entry added by bug 16759 for this purpose.

The current behaviour is preserved.

To test:
- Apply this patches
- Run the upgrade:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ perl installer/data/mysql/updatedatabase.pl
=> SUCCESS: Upgrade doesn't fail
- Have an item with shelving location set to something not void
- Have the item set home and holding libraries for testing purposes.
- Set 'OpacLocationBranchToDisplay' to 'home and holding libraries' [*]
- Visit the OPAC detail page for the record containing the item
=> SUCCESS: Both home and holding libraries are displayed.
- Loop through all OpacLocationOnDetail options (except from 'column', we leave it for later).
=> SUCCESS: Works as expected.
- Go to Administration > Columns settings
- Make item_shelving_location available in the OPAC section
- Reload the OPAC detail page
=> SUCCESS: No change
- Set OpacLocationOnDetail to 'on a separate column'
- Reload the OPAC detail page
=> SUCCESS: Shelving location is displayed on a separate column
- Sign off :-D

Sponsored-by: Dover

[*] For testing purposes

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-25 12:14:41 -03:00
f18af55a39 Bug 15685: (follow-up) K:A:O->find and ->fetch are no longer used
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-11 13:08:46 -03:00
Jesse Weaver
b29493265b Bug 15685: Allow creation of items (AcqCreateItem) to be customizable per-basket
This adds a new basket attribute (create_items) that can optionally be
set to override AcqCreateItem.

The following have been modified to reflect this (with the value of
create_items that causes them to behave differently in parentheses):
  * Cancelling receipt of an order (receiving)
  * Creating an order by hand or from MARC (ordering)
  * Receiving an order (receiving)
  * Showing orders with uncertain price (ordering)
  * Showing orders (receiving)
  * Showing acquisition details in the OPAC (ordering)

Test plan:
  1) Create baskets with "Create items when:" set to ordering,
     receiving, cataloging and unset.
  2) Test each of the above for each of these baskets, verifying that
     the basket-specific attribute overrides AcqCreateItem if set and
     falls back to the syspref otherwise.

NOTE: A check of AcqCreateItem in opac-detail.tt was removed because it
was redundant; the code path in question cannot be triggered unless
create_items/AcqCreateItems is set to the correct value anyway.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Barbara Fondren <bfondren@roundrocktexas.gov>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-11 13:06:06 -03:00