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>
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>
This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes
almost all perlcrictic violations according to this .perlcriticrc
The remaining violations are silenced out by appending a '## no critic'
to the offending lines. They can still be seen by using the --force
option of perlcritic
This patch also modify t/00-testcritic.t to check all Perl files using
the new .perlcriticrc.
I'm not sure if this test script is still useful as it is now equivalent
to `perlcritic --quiet .` and it looks like it is much slower
(approximatively 5 times slower on my machine)
Test plan:
1. Run `perlcritic --quiet .` from the root directory. It should output
nothing
2. Run `perlcritic --quiet --force .`. It should output 7 errors (6
StringyEval, 1 BarewordFileHandles)
3. Run `TEST_QA=1 prove t/00-testcritic.t`
4. Read the patch. Check that all changes make sense and do not
introduce undesired behaviour
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 9978 should have fixed them all, but some were missing.
We want all the license statements part of Koha to be identical, and
using the GPLv3 statement.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch does the following:
[1] Move some POD lines from Cache to Caches.
[2] Correct C4::Plugins to Koha::Plugins in POD line of Koha::Plugins
[3] POD Koha/AuthorisedValue.pm: lib_opac moved to opac_description
[4] The POD in Koha/Patron.pm uses head2 and head3 inconsistently.
Ran s/^=head2/=head3/ on those lines (7 substitutions on 7 lines)
[5] Correct a copied POD line from reports/issues_stats.pl in
reports/reserve_stats.pl.
[6] Correct a test description in t/db_dependent/Koha/Authorities.t.
You should never delete the library :)
[7] Correct typo shouild in a comment of rebuild_zebra.pl
Test plan:
[1] Read the patch. Does it make sense?
[2] Run perldoc Koha/Cache.pm and Koha/Caches.pm
[3] Run t/db_dependent/Koha/Authorities.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Change to Modern::Perl.
Remove null_to_zzempty and add a few blank lines between subs.
Remove unused vars: $podsp, $type, $daysel, $monthsel and $mime.
Removing trailing 1;
Test plan:
Run a few reports from reports/reserves_stats.pl
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Emma Smith <emma.nakamura.smith@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Wrongly pasted from reports/issues_stats.pl initially
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Ok I am silly, we needed to replace to use the cache mechanism for
search_by_koha_field, not find_by_koha_field...
Let's create another subroutine
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch replaces the call to C4::Koha::GetKohaAuthorisedValues with
Koha::AuthorisedValues->search_by_koha_field
Test plan:
AV descriptions should be displayed on the following pages:
- XSLT view - location and ccode
- Bibliographic detail, moredetail and OPAC pages - location, ccode, copynumber
- returns - location
- opac-basket - ccode, location
- The 3 reports: catalogue_stats.pl, issues_stats.pl and
reserves_stats.pl - location, ccode
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Lot of errors are raised by this script under plack because of var scope
issues:
Variable "$ccodes" is not available at
/home/koha/src/reports/reserves_stats.pl line 337.
Variable "$locations" is not available at
/home/koha/src/reports/reserves_stats.pl line 338.
Variable "$itemtypes" is not available at
/home/koha/src/reports/reserves_stats.pl line 339.
Variable "$Bsort1" is not available at
/home/koha/src/reports/reserves_stats.pl line 344.
Variable "$Bsort2" is not available at
/home/koha/src/reports/reserves_stats.pl line 350.
Variable "@patron_categories" is not available at
/home/koha/src/reports/reserves_stats.pl line 356.
The lazy way to fix that is that reinit them from the subroutines they
are used.
Test plan:
Execute the report, after this patch applied you won't see the error in
the log.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan
=========
On Reports > reservations, select all statuses as line, pickup library as column (empty) and a range of hold date. Keep the results
Run the same reports but with statuses as column and pickup library as line
You should get the same results on another display.
* Without patch, if you've got representative data, most of the time you can observe significant differences between both results
* With the patch, results are always the same (and right if you request directly your database)
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This is the fourth and last patch set to remove C4::Branch.
The real purpose of this patch is to standardise and refactor some code
which is related to the libraries selection/display.
Its unconfessed purpose is to remove the C4::Branch package.
Before this patch set, only 6 subroutines still existed in the C4::Branch
package:
- GetBranchName
- GetBranchesLoop
- mybranch
- onlymine
- GetBranches
- GetBranch
GetBranchName basically returns the branchname for a given branchcode.
The branchname is only used for a display purpose and we don't need to
retrieve it in package or pl scripts (unless for a few exceptions).
We have a `Branches` template plugin with a `GetName` method which does
exactly this job.
To achieve this removal, we will use this template plugin and delete the
GetBranchName from pl and pm files.
The `Branches.all()` will now select the library of the logged in user
if no `selected` parameter has been passed.
This new behavior could cause regressions, for instance there are some
places where we do not want an option preselected (batch item
modification for instance), keep that in mind when testing.
GetBranchesLoop took 3 parameters: $branch and $onlymine.
The first one was used to set a "selected" flag, for a display purpose:
select an option in the libraries dropdown lists.
The second one was useless: If not passed or set to 0, the
`C4::Branch::onlymine` subroutine was called.
This onlymine flag was use to know if the logged in user was able to see
other libraries infos.
A patron can see the infos from other libraries if IndependentBranches
is not set OR if he has the superlibrarian permission.
Prior to this patch set, the "onlymine test" was done on different
places (neworderempty.pl, additem.pl, holidays.pl, etc.), including the
Branches TT plugin. In this patch set, this test is only done on one
place (C4::Context::only_my_library, code moved from
C4::Branch::onlymine).
To accomplish the same job as this subroutine, we just need to call the
`Branches.all()` method from the `Branches` TT plugin. It already
accepts a `selected` parameter to set a flag on the option to select.
To avoid the repetitive
[% IF selected %]<option selected="selected">[% ELSE %]<option>[% END %]
pattern, a new `html_helpers` TT include file has been created, it
defines an `options_for_libraries` block, which takes a `selected`
parameter. We could imagine to use this include file for other
selects.
The 'mybranch` and `onlymine` subroutines of the C4::Branch package have
been moved to C4::Context. onlymine has been renamed with
only_my_library. There are only 4 occurrences of it, against 11 before
this patch set.
There 2 subroutines are Context-centric and it makes sense to put them
in `C4::Context` (at least it's the least worst place!)
GetBranches is the tricky part of this patch set: It retrieves all the
libraries, independently of the value of IndependentBranches.
To keep the same way as the existing calls of `Branches.all()`, I have
added a `unfiltered` parameter. If set, the `Branches.all()` will call
a usual Koha::Libraries->search method, otherwise
Koha::Libraries->search_filtered will be called. This new method will
check if the logged in user is allowed to see other libraries or only
its library.
Note that this `GetBranches` subroutine also created a `category` key:
it allowed to get the list of groups (of libraries) where this library
existed. Thanks to a previous patch set (bug 15295), this value was
not used anymore (I may have missed something!).
Note that the only use of `GetBranch` was buggy (see bug 15746).
Test plan (for the whole patch set):
The best way to test this whole patch set is to test with 2 instances: 1
with the patch set applied, 1 using master, to be sure there is no
regression.
It would be good to test the same with `IndependentBranches` and the
without `IndependentBranches`.
No difference should be found.
The tester must focus on the library dropdowns on as many forms as
possible.
You will notice changes in the order of the options: the libraries will
now be ordered by branchname (instead of branchcode in some places).
A special attention will be given to the following page:
- acqui/neworderempty.pl
- catalogue/search.pl
- members/members-home.pl (header?)
- opac/opac-topissues.pl
- tools/holidays.pl
- admin/branch_transfer_limits.pl
- admin/item_circulation_alerts.pl
- rotating_collections/transferCollection.pl
- suggestion/suggestion.pl
- tools/export.pl
Notes for QA:
- There are 2 FIXMEs in the patch set, I have kept the existing behavior,
but I am not sure it's the good one. Feel free to open a bug report and
I will fill a patch if you think it's not correct. Otherwise, remove the
FIXME lines in a follow-up patch.
- The whole patch set is huge and makes a lot of changes.
But it finally will tremendously reduce the number of lines:
716 insertions for 1910 deletions
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
C4::Members::GetBorrowercategoryList returned all the patron categories,
ordered by description.
This can be done using the search method of the Koha::Patron::Categories
module.
Test plan:
- Same prerequisite as before
For the following pages, you should not see patron categories limited to other
libraries. They should be ordered as before this patch, by description.
- On the "patron attribute types" admin page (admin/patron-attr-types.pl).
- On the "Batch patron deletion/anonymization"
(tools/cleanborrowers.pl), the dropdown list should be filled
correctly.
- On the import patron tool
- On the following report:
reports/reserves_stats.pl
Confirm that the patron categories are displayed and that the report
still works as before.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The C4::Category module contained only 1 method to return the patron
categories available for the logged in user.
The new method Koha::Patron::Categories->search_limited does exactly the
same thing (see tests) and must be used in place of it.
Test plan:
- Same prerequisite as before
For the following pages, you should not see patron categories limited to
other libraries.
- On the 'Item circulation alerts' admin page
(admin/item_circulation_alerts.pl), modify the settings for check-in
and checkout (NOTE: Should not we display all patron categories on
this page? If yes, it must be done in another bug report to ease
backporting it).
- Search for patrons in the admin (budget) and acquisition (order) module.
- On the patron home page (search form in the header)
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Remove C4::Dates from following files:
- reports/acquisitions_stats.pl
- reports/borrowers_stats.pl
- reports/reserves_stats.pl
- reports/serials_stats.pl
- reports/issues_avg_stats.pl
- reports/issues_stats.pl
To test:
- Go to Home > Reports
- Verify that following reports work as before:
- Other > Average loan time
- Statistic wizards
- Acquisitions
- Patrons
- Circulation
- Serials
- Holds
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This patch removes similar instances on 5 files, same code.
Also removes a commented one, changed in case some plan to
resurrect the code.
This bug depends on Bug 12696.
In all cases but the commented one, what was replaced
is a pulldown with one value, 'CSV'
To test:
1. Apply patch for Bug 12696
2. Apply this patch
Check for the pulldown 'Into an application'
3. Go to Reports > Statistics wizard > Borrowers
4. Go to Reports > Statistics wizard > Circulation
5. Go to Reports > Statistics wizard > Holds
6. Go to Reports > Top lists > Most-circulated items
7. Go to Reports > Inactive > Patrons who haven't checked out
8. There is also a commented code on catalogue_stats, no fuctional
changes.
Followed test plan. Patch behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes tests and QA script.
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
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 dateformat value is send to all templates (from
C4::Auth::get_template_and_user)
- remove all assignment of dateformat in all .pl files
- the DHTMLcalendar_dateformat variable is unused
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixed conflicts:
- opac/sco/sco-main.pl
- reports/acquisitions_stats.pl
- tools/cleanborrowers.pl
All tests pass, perlcritic problems appeared in some files
before and after these patches were applied.
Checked sorting in following pages:
- acqui/addorderiso2709.tt - list of staged imports in acq
- acqui/histsearch.tt - sorting of dates in acq search result list
- acqui/invoices.tt - billing date in list of invoices in acq
- acqui/lateorders.tt - list of late orders in acq
- acqui/ordered.tt - ordered titles and estimated costs for a fund
- acqui/parcels.tt - receive shipment page
- acqui/spent.tt - received titles and actual costs for a fund
...
- serials-search.tt - subscription search result list
...
- opac/sco/sco-main.tt - due dates in list of checked out items
- reports/acquisitions-stats.tt - date searches, display of dates
- tools/cleanborrowers.tt
- tools.holidays.tt - different views of dates library is closed,
adding dates
Checked dates display according to system preference everywhere and
searching, entering dates etc. still worked as expected.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
In Bug 7772 Ian correctly noted that reports have exit(1) all over the place.
This is left over from old code, and this patch changes them to exit(0).
I decided to use plain exit as opposed to explicit exit(0) since it produces
cleaner code, but I'm welcoming suggestion on this.
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
This patch fixes several errors in reserves_stats.pl and reserves_stats.tt.
Testing -
To test this patch, data must be in either the reserves table or old_reserves or both. The following SQL will give you the raw data that is used by the report.
SELECT priority, found, reservedate, notificationdate, reminderdate,
waitingdate, cancellationdate, borrowers.categorycode, items.itype,
reserves.branchcode, holdingbranch, items.homebranch, items.ccode,
items.location, items.itemcallnumber, borrowers.sort1, borrowers.sort2
FROM reserves
LEFT JOIN borrowers on (borrowers.borrowernumber = reserves.borrowernumber)
LEFT JOIN items on (items.itemnumber = reserves.itemnumber)
UNION SELECT priority, found, reservedate, notificationdate, reminderdate,
waitingdate, cancellationdate, borrowers.categorycode, items.itype,
old_reserves.branchcode, holdingbranch, items.homebranch, items.ccode,
items.location, items.itemcallnumber, borrowers.sort1, borrowers.sort2
FROM old_reserves
LEFT JOIN borrowers on (borrowers.borrowernumber = old_reserves.borrowernumber)
LEFT JOIN items on (items.itemnumber = old_reserves.itemnumber)
To test the notificationdate and reminderdate, I added data to the old_reserves table, since I have never run notices on my test machine.
Ex:
UPDATE old_reserves
SET notificationdate = "2012-01-29",
reminderdate = "2012-01-29"
WHERE timestamp = "2012-01-29 20:09:34";
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Confirm original bug -- Reports work as expected now!
prove t xt t/db_dependent no different from master.
