On bug 17591 we discovered that there was something weird going on with
the way we export and use subroutines/modules.
This patch tries to standardize our EXPORT to use EXPORT_OK only.
That way we will need to explicitely define the subroutine we want to
use from a module.
This patch is a squashed version of:
Bug 17600: After export.pl
Bug 17600: After perlimport
Bug 17600: Manual changes
Bug 17600: Other manual changes after second perlimports run
Bug 17600: Fix tests
And a lot of other manual changes.
export.pl is a dirty script that can be found on bug 17600.
"perlimport" is:
git clone https://github.com/oalders/App-perlimports.git
cd App-perlimports/
cpanm --installdeps .
export PERL5LIB="$PERL5LIB:/kohadevbox/koha/App-perlimports/lib"
find . \( -name "*.pl" -o -name "*.pm" \) -exec perl App-perlimports/script/perlimports --inplace-edit --no-preserve-unused --filename {} \;
The ideas of this patch are to:
* use EXPORT_OK instead of EXPORT
* perltidy the EXPORT_OK list
* remove '&' before the subroutine names
* remove some uneeded use statements
* explicitely import the subroutines we need within the controllers or
modules
Note that the private subroutines (starting with _) should not be
exported (and not used from outside of the module except from tests).
EXPORT vs EXPORT_OK (from
https://www.thegeekstuff.com/2010/06/perl-exporter-examples/)
"""
Export allows to export the functions and variables of modules to user’s namespace using the standard import method. This way, we don’t need to create the objects for the modules to access it’s members.
@EXPORT and @EXPORT_OK are the two main variables used during export operation.
@EXPORT contains list of symbols (subroutines and variables) of the module to be exported into the caller namespace.
@EXPORT_OK does export of symbols on demand basis.
"""
If this patch caused a conflict with a patch you wrote prior to its
push:
* Make sure you are not reintroducing a "use" statement that has been
removed
* "$subroutine" is not exported by the C4::$MODULE module
means that you need to add the subroutine to the @EXPORT_OK list
* Bareword "$subroutine" not allowed while "strict subs"
means that you didn't imported the subroutine from the module:
- use $MODULE qw( $subroutine list );
You can also use the fully qualified namespace: C4::$MODULE::$subroutine
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When there are several images attached to a bibliographic record, one can click
an image and see it in the image viewer. But the first image is always displayed.
Test plan:
0. Turn on LocalCoverImages and AllowMultipleCovers
1. Attach at least two images to a bibliographic record
2. Go to the detail page, click the "Images" tab and click the second
image.
=> With this patch applied you should see the image you selected
Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In order to know we are dealing with the cover images of a specific item
we need to pass it though the different forms.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In this patch we remove the limitation of 1 cover image per item
Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patchset adds the ability to attach cover images at item level.
This commit message will explain the different patches that are
following.
The main idea is to have cover images for a given item. This is useful
for a bibliographic record linked with a subscription. Each item could
have the cover image for the serial numbers.
In this first patch there is a limitation to allow only 1 cover per
item, but a later patch will remove it. That way we will take advantage
of the recent work done to display nicely cover images (bug 25031), and
reuse it in this development (staff interface only).
In order to use a flexible and robust code, the legacy C4 code
(C4::Images) has been moved to Koha::CoverImages. Also the DB table
biblioimages has been renamed cover_images.
Test plan (for the whole patch set):
0. Turn off AllowMultipleCovers
1. Create a new bibliographic record and items
2. Attach a cover image to the bibliographic record
3. In the item list of the bibliographic detail page, you will notice a
new "Upload image" action. Select it
4. Select an image
=> Notice the new column in the item table
5. Upload another image
=> You cannot upload 2 images, you are going to replace the existing one
6. Turn on AllowMultipleCovers
7. Attach another image to the image
=> Notice the 2 images are displayed nicely, with navigation controls
8. Confirm you can view an image if you click on it and you can delete it
9. Test the OPAC view now
=> Cover image for items are displayed in the table, there is no
navigation controls here however.
Sponsored-by: Gerhard Sondermann Dialog e.K. (presseplus.de, presseshop.at, presseshop.ch)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It defaults to 0 in get_template_and_user
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
There is a "norequest" boolean passed to the include cat-toolbar.inc, to
display or not the "Pace hold" button.
This flag was not calculated in some place (ISBDdetail and moredetail
for instance)
Here we centralize the code to make it more robust and less regression
prone.
Note that the same problem appears at the OPAC (opac-MARCdetail is
always display the button). That will have to be fixed separately
Test plan:
Create biblio A with 0 item, B and C with 1 item and D with 2 items
item for B is not for loan, C is for loan and D has 1 of each
Go to the bibliographic detail page of each record and confirm that the
"Place hold" button appears appropriately.
Test the different tabs of the catalogue module
QA note: This patch is only centralizing the existing code, but it is
still too naive. To manage the visibility of this button we certainly
want to copy what is done in C4::Search::searchResults:
# can place a hold on a item if
# not lost nor withdrawn
# not damaged unless AllowHoldsOnDamagedItems is true
# item is either for loan or on order (notforloan < 0)
$can_place_holds = 1
if (
!$item->{itemlost}
&& !$item->{withdrawn}
&& ( !$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems') )
&& ( !$item->{notforloan} || $item->{notforloan} < 0 )
);
Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is never used.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This is never used.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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>
This patch modifes the process of searching for an existing record
to add to a basket. Now the search is performed as a keyword search in
the regular catalog rather than via a custom search script. Options are
added to the search results and detail pages to add results to an order.
This process follows the same pattern as the "Search to hold" feature:
When the search is initiated, a cookie is set with the requisite
information--in this case vendor id and basket number.
If the search results or bibliographic detail pages detect that a
"searchToOrder" cookie is present, the correct "Add order" link will be
shown. Like with the "search to hold" feature, the cookie expires in 10
minutes.
To test, apply the patch and log into the staff client as a user who has
permission to add to a basket in acquisitions.
- Go to Acquisitions -> Vendor -> Basket -> Add to basket.
- Using the "From an existing record" option, perform a search.
- On the search results page, test the "Add order" link which appears
with each result. Clicking the link should take you to the "New
order" page for the correct vendor and basket. The catalog
details section of the form should include the correct information.
- From the search results page view the bibliographic details page for
any record. There should be a new toolbar button, "Add order." Verify
that it works correctly.
- Test the same thing from all bibliographic detail pages: Normal,
MARC, Labeled MARC, ISBD, as well as the items page
(moredetail.pl).
- Test this process for both the locations in acquisitions where one
can add to an existing basket: Vendor search results and the basket
detail page
- Test the cookie timeout: Wait 10 minutes and perform another catalog
search. The "Add order" link should no longer be present.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Test Plan:
- Check that it now says 'use Modern::Perl;' and not 'use strict; use
warnings;' in the following catalogue perl scripts.
MARCdetail.pl
export.pl
image.pl
imageviewer.pl
issuehistory.pl
labeledMARCdetail.pl
moredetail.pl
search.pl
showmarc.pl
updateitem.pl
Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
GetMember returned a patron given a borrowernumber, cardnumber or
userid.
All of these 3 attributes are defined as a unique key at the DB level
and so we can use Koha::Patrons->find to replace this subroutine.
Additionaly GetMember set category_type and description.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Remove second Koha::Biblios->find in imageviewer.pl
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
C4::Biblio::GetBiblio can be replaced with Koha Biblio->find
Test plan:
Import batch, view issue history, search for items, see the image of a
bibliographic record, modify and delete records in a batch
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The C4::Reserve::GetReservesFromBiblionumber took 3 parameters, the
biblionumber, an optional itemnumber and a "all_dates" flag.
If set, the subroutine returned all the holds placed on a given bibliographic
record, even the ones placed in the future. Almost all of the calls had this
flag set, they will be replaced with a call to Koha::Biblio->holds.
But 5 did not have it:
- C4::Biblio::DelBiblio
-tools/batch_delete_records.pl
=> These 2 were wrong, we want to retrieve the holds to cancel them
before deleting the record. We need to get all the holds, even the ones
placed in the future /!\ CHANGE IN THE BEHAVIOR
- acqui/parcel.pl
=> 1 call per item were made to this subroutine. They have been replaced
with only 1 call to the new method Koha::Biblios->holds_placed_before_today
Then we filter on the itemnumbers.
I think this is wrong: we need the number of holds to know if the record
can be deleted, so even if future holds exist, the deletion should not
be possible.
- serials/routing-preview.pl
- C4::ILSDI::Services::GetRecords
- C4::SIP::ILS::Item->new
=> Seems ok, we just one to display holds placed before today
Test plan:
I would suggest to test this patch with patches from bug 17737 and bug 17738,
to place different kind of holds (biblio and item level, future and
past).
Then do a whole workflow to detect bug, view a record, delete record,
order, place a hold on an item which has been ordered, etc.
The hold's informations should always be the same without or without
these patches.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
C4::Items::GetItemsCount can be replaced with Koha::Biblio->items->count
Test plan:
Create a bibliographic record with items attached
Try to delete the record from a basket (acquisition module), the detail
page and the batch item deletion tool.
=> You should not be able to delete it.
Remove the items and then try again to delete the record
=> Now you must be able to delete it.
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Chris Nighswonger <cnighswonger@foundations.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
http://bugs.koha-community.org/show_bug.cgi?id=9987
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Since we switched to Template Toolkit we don't need to stick with the
sufix we used for HTML::Template::Pro.
This patch changes the occurences of '.tmpl' in favour of '.tt'.
To test:
- Apply the patch
- Install koha, and verify that every page can be accesed
Regards
To+
P.S. a followup will remove the glue code.
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
The JavaScript included in cat-toolbar.inc expects several variables to
be passed to the template for use in calculations. Several scripts
do not pass the required variables to the template, leading to a JS
error.
This patch re-adds (See Bug 9823) hold count calculation to the required
scripts and copies the acquisitions-related count calculation from
detail.pl into related bibliographic detail scripts so that the
commonly-included cat-toolbar.inc can receive correct values.
To test, locate a bibliographic record which has no holds and is not
used in a current or deleted order, but which has a local cover image
attached. Check the following views to confirm there is no JavaScript
error:
- Normal
- MARC
- Labeled MARC
- ISBD
- Items
- Images (from the Images tab on the "Normal" detail page)
Next, locate a bibliographic record with no items attached which is used
in an Acquisitions order. On each of the pages listed above, choose
"Delete record" from the Edit menu. This should trigger a warning that
the record is used in an order. Cancel the deletion.
Finally, delete the order which references the bibliographic record. On
each of the pages listed above, choose "Delete record" again. This
should trigger a warning that the record is used in a deleted order.
Cancel the deletion.
I went through the test plan, everything works OK.
It resolves Bug 11831 as well.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
- Printing from MARC and ISBD view works again
- No more Javascript errors found
- Deleting a record with attached order gives the correct warning,
both for ordered and cancelled.
Passes all tests and QA script.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
The template params holds and holdcount are not used in the ISBD template.
Removed the associated code from catalogue/ISBDdetail.pl.
Same applies for catalogue/MARCdetail.pl and labeledMARCdetail.pl.
Same applies also for catalogue/imageviewer.pl.
Same applies also for catalogue/moredetail.pl.
In catalogue detail.tt only the number of holds is used. Removed the code
that passed the holds array to the template.
For consistency opac-detail should also incorporate future holds into its
holds count; added the all_dates parameter for that reason.
The Reserves module is no longer needed in moredetail. Removed it. Checked
the other scripts also.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
The return from GetReservesFromBiblionumber contains an unnecessary
extra variable. In scalar context an array returns its element count.
Maintaining a separate count can lead to unforeseen bugs
and imposes ugly constructions on the subroutine's users.
Remove the useless count variable from the return
This patch also changes the parameters: now the routine takes a hashref.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Placed biblio holds, future holds and item holds. Works as expected.
Tested Holds.t and Reserves.t. Pass.
Tested /cgi-bin/koha/ilsdi.pl?service=GetRecords&id=999 with two holds on
one item. Fine.
C4/SIP/ILS/Item.pm: Looked for "whatever" and "arrayref" and could not find
them anymore. Looks good.
Handled a few unneeded calls in QA follow-up.
Left only one point to-do for serials/routing-preview.pl. See Bugzilla.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Test Plan:
1) Apply patch
2) Create a record
3) Create an item for the record
3) Place a hold on the bib
4) Attempt to 'Delete all items', you should recieve
an error message stating to delete all holds before
deleting all items.
Also, it is possible to get into a situation where a record has
holds but no items. In this situation, it is not possible to
view/delete the holds without adding an item back to the record.
In this case, attempting to delete the bib causes a warning, but
does not prevent deletion.
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Passes tests - do note that it was a design decision to leave the delete links clickable even though they are grey.
The reasoning is that librarians will want to be able to know *why* they cannot delete a given item or bib - I like this.
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
* Show the "Upload images" button when OPACLocalCoverImages is enabled but
LocalCoverImages (i.e. local cover images on the staff client) is not
* Correct copyright and license comments in new files
* perltidy and replace tabs by four spaces
Signed-off-by: Koustubha Kale <kmkale@anantcorp.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
This patch revises the image viewer page to match other
biblio detail pages: a toolbar, a sidebar menu, etc.
Additional data is now pulled by imageviewer.pl in order
to provide information necessary for the proper functioning
of the toolbar and menus: item counts, hold availability,
biblio view options.
Other changes have been made to match the changes in the
OPAC interface like styling of the images and no-javascript
compatibility
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Magnus Enger <magnus@enger.priv.no>
Patch failed to apply because of multiple Content-Type lines in the patch.
Corrected the same with this new patch.
http://bugs.koha-community.org/show_bug.cgi?id=6473
This patch adds display of local cover images in the following places:
1. OPAC Results page
2. OPAC Details page
3. Separate image viewer page for the OPAC
4. Intranet Details page
5. Separate image viewer page for the Intranet
Display in the Details and (OPAC) Results pages is handled via Javascript
rather than via direct embedding, to better handle the situation where most
records have local cover images, but a few do not.
Local cover images do not currently display in the Intranet Results page
How to use/test :
Assign user permission to the user Tools > (upload_local_cover_images Upload
local cover images). In order to upload local images, login to the staff
client. Go to Home > Tools > Upload Cover Images. Here you can upload cover
images either singly or in bulk in the form of a zip file. If uploading
singly, click on image file, browse the image from your local disk, type in
the biblio number of the catalogue entry and press upload. If uploading in
bulk as a zip file, the zip file must contain (in addition to cover images)
one text file named either datalink.txt OR idlink.txt. This file should
have mapping of biblionumber to image file name in the zip one per line
with comma or tab as delimiters. For example:
1, scanned_cover_image_of_bib_no_1.jpg
2, scanned_cover_image_of_bib_no_1.jpg
Cover images will be resized to a large image of 800x600 and a thumbnail of
200x140. Depending on the setting of AllowMultipleCovers, it is possible to
upload multiple images for a single bibliographic record. However, even if
multiple covers are permitted, you have the option of replacing the existing
covers by checking the "Replace existing covers" option on the upload screen.
1. The patch adds a menu link in Tools from where you can upload local cover
images
2. It adds a user permission to enable access control to this menu item under
Tools
3. It adds a system preference OPACLocalCoverImages under Enhanced Content.
This needs to be turned on to show local cover images in OPAC.
Once you have uploaded local images, if you search for the biblio, the local
cover should show up in search as well as search detail pages in the OPAC, and
the details view in the Intranet.
Koustubha Kale is working on another patch which will allow us to set a cover
image source priority in system preferences, and which will then gracefully
fail over to the next source if image is not available from the first choice
source.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Magnus Enger <magnus@enger.priv.no>
Signed-off-by: Koustubha Kale <kmkale@anantcorp.com>
Rebased the patch because it was failing with koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css due to changes pushed in master.
Applies cleanly now.
