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>
There is a "debug" parameter we are passing from the controller scripts
to C4::Auth::get_template_and_user, but it's not actually used!
Test plan:
Confirm the assumption
Review the changes from this patch
Generated with:
perl -p -i -e 's#\s*debug\s*=\>\s*(0|1),?\s*##gms' **/*.pl
git checkout misc/devel/update_dbix_class_files.pl # Wrong catch
+ Manual fix in acqui/neworderempty.pl
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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>
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>
Dealt with that previously in the module during the rebase.
It conflicted with bug 23281.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We can then now start to move methods from C4::Members::AttributeTypes
as well.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
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>
Changes:
- Replace getting preference ExtendedPatronAttributes by Koha.Preference
in templates
- Add Koha::Patron->attributes for getting patrons extended attributes
- Use this method in circ-menu.inc
- Remove getting attributes from members perl scripts
Test plan:
0) Apply the patch
1) Add some patron attributes type - with free text, authorised value,
limited by libraries...
2) Add some values to this attributes for some patrons
3) Go through as many patron pages as you can and confirm that
attributes are shown at side panel when they shoul and are not shown
when they should not be shown
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Removed Koha/Schema/Result/BorrowerAttribute.pm
[EDIT] Added missing semicolon on L114 in Koha/Patron/Attribute.pm
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
In Reports > Patrons statistics, the combination of activity and extended attributes generates an invalid SQL query with error :
ERROR 1052 (23000): Column 'borrowernumber' in IN/ALL/ANY subquery is ambiguous
This is because FROM contains tables borrowers and borrower_attributes both containing borrowernumber column.
Test plan :
1) Be sure to have a patron extended attribute
2) Be sure you have some recent circulation activity
3) Go to Reports > Patrons statistics
4) Select 'active' in 'Patron activity' and 1 years of activity
5) Check 'Row' in 'Patron category'
6) Check 'Columns' in a extended attribute
7) Submit
8) Without patch you have no results, with patch you have exepected results
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch was generated using codespell
Test plan:
Read through changes and confirm they make sense
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
https://bugs.koha-community.org/show_bug.cgi?id=21706
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
TEST PLAN
---------
run patron statistics without filtering
-- notice lots of text at top.
run patron statistics with filtering
-- notice lots of text at top.
apply patch
run it without and without filtering
-- amount of text should be small or none and be related to the filtering selected.
run koha qa test tools
Signed-off-by: Andreas Roussos <arouss1980@gmail.com>
Works as expected, i.e. only filtering information is displayed.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The "inactive" for "patron activity" choice is now effective.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The subroutine C4::Koha::GetAuthorisedValueByCode returned the
description (staff or opac) for a given authorised value.
Note that we may need a unique key to ->find instead of ->search.
Test plan:
- Checkin an item that cannot be checked in because it's lost, the
message should display the AV description
- Generate a letter with borrowers.streettype equals an ROADTYPE AV, the
description should be displayed.
- Edit a patron attribute type, the AV dropdown list should be
displayed
- Create the PA_CLASS AV category (see bug 7154) and make sure it
behaves as before when editing a patron
- The checkout list should display descriptions for LOC, LOST and
DAMAGED
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
One of the patches removed %cathash. Some kind of silly remainder
was left in a string. Just used as a text field to explain the
filters used on borrowers_stats.pl. The filter name cathash is not
really easy and also was confusing in relation to the variable.
So just a string patch, replacing it with Patron category.
NOTE: The output of borrowers_stats.pl with the filters listed, is
not one of the most beautiful Koha screens.
Furthermore, translatibility of these filters is still an issue to
be handled on another report.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch replaces sql queries done in some pl script and in
C4::Reports::Guided.
Since we have now a Koha::Patron::Categories module, we should use it
where it is possible.
Test plan:
- Prerequisite: Be sure you have several patron categories created, with
different option enabled, and limit some to certain libraries.
- On the 'Circulation and fine rules' admin page (admin/smart-rules.pl),
all the patron categories should be displayed (even the ones limited to
another library), ordered by description. Try to add/update existing rules.
- On the overdue rules page (tools/overduerules.pl), all the patron
categories with overduenoticerequired set should be displayed.
Try to add/update existing rules.
- On the following reports:
reports/borrowers_stats.pl
reports/issues_avg_stats.pl
The patron categories should be displayed. Note that there is an
inconsistency with these 2 reports: the patron categories limited to
other libraries are displayed on them, when they are not on the other
reports. This should certainly be fixed (on another bug report).
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>
This patch replaces the occurrences of
my @foo = $cgi->param('foo');
with
my @foo = $cgi->multi_param('foo');
perl -p -i -e
's/^(\s*my\s*@\w+\s*=\s*)\$(cgi|input|query)\->param\(/$1\$$2\->multi_param\(/xms'
**/*.pl
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
To test:
- Log in to Staff Interface, go to Administration, Libraries and Groups (cgi-bin/koha/admin/branches.pl). Click 'New library' and confirm that it says 'ZIP/Postal Code:' in the form.
- Go back to Administration, Cities and towns (cgi-bin/koha/admin/cities.pl). Confirm it says 'ZIP/Postal code' in table heading. Click 'New city' and confirm it says 'ZIP/Postal code:' in form
- Go back to Administration, Global system preferences, I18L/L10N (cgi-bin/koha/admin/preferences.pl?tab=i18n_l10n). Under AddressFormat, confirm all available options say '[ZIP/Postal Code]'
- Go to Reports, Patrons (cgi-bin/koha/reports/borrowers_stats.pl). Confirm it says 'ZIP/Postal code' under Title
- Go to Tools, Batch patron modification (cgi-bin/koha/tools/modborrowers.pl), enter any card number and click 'Continue'. Confirm it says 'ZIP/Postal code' in the table of patron details, and in the form to edit
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
NOTE: Tested together with first patch.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch removes stray C4::Dates from following files:
reports/borrowers_stats.pl
To test: Verify that C4::Dates is not used in the file
reports/guided_reports.pl:
To test:
- Run a saved report with date input and with date range input, e.g.
- Verify that you are asked for date range or date and that the results
are the same as without patch
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
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>
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>
This patch removes the use of smartmatch operators in report scripts.
Regards
To+
Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes QA script and tests.
Acquisition and Patron statistics wizard tested, no regressions found.
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
There are several warnings and one non-Plack-compatible variable in
reports/borrower_stats.pl.
To test:
1) Apply patch
2) Use the borrower report wizard to create a report that includes an
extended patron attribute.
3) Check the logs, and make sure that you don't see any of the
following messages:
Variable "$period" is not available at
/home/jcamins/kohaclone/reports/borrowers_stats.pl line 265.
Use of uninitialized value in hash element at
/home/jcamins/kohaclone/reports/borrowers_stats.pl line 375.
Use of uninitialized value in hash element at
/home/jcamins/kohaclone/reports/borrowers_stats.pl line 376.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Fix bug that prevented zero values to be shown in the results.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Fixes the problem with differentiating 0 and NULL values.
Passes all tests.
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Check input parameters to avoid SQL injection
Rewrite a for loop for readability
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests pass, perlcritic is unchanged.
Test plan
- Verify ExtendedPatronAttributes hides/shows new patron attribute
search criteria
- Verify patron attribute show up correctly
- pull down list for attributes linked to authorized values
- input field for other attributes
- Verify search works correctly
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
This patch allows to choose row and column amongst extended patron
attributes and to filter on them in reports/borrower_stats.pl
Signed-off-by: Marc Veron <veron@veron.ch>
Great, I was looking for such a feature.
Patch works as expected.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.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>
The intended functionality of this system preference was never
implemented. To avoid template changes prior to 3.2.0, put in
hard-coded string 'CSV' to (accurately) identify the type of
the output files in the affected reports.
Signed-off-by: Galen Charlton <gmcharlt@gmail.com>
This repurposes the Reports module for general reports, and moves its functionality to a different module, Reports::Guided. It also updates all of the reports that can export to correctly treat the 'tabulation' value of the 'delimiter' syspref.
Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
Fixes following error when using the patron statistics report:
DBD::mysql::st execute failed: Unknown column 'sort' in 'where clause' at /home/gmc/koha/dev/reports/borrowers_stats.pl line 127
Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
This patch changes all $foo eq undef's to !defined($foo). It also makes misc/spellcheck_suggest/build_spellcheck_suggest.pl have proper syntax.
Signed-off-by: Joshua Ferraro <jmf@liblime.com>
At client request, I added code for a rowtitle_display and coltitle_display. This
allows the script to substitute human-readable lables into the table instead of just
the literal hashkeys. For this client with dozens of numerical patron categorycodes
having a row titled "29" was not very useful.
I cleaned up the templates and a lot of the report code. It is of the same style as
the other copy/paste reports and still need further cleanup. Added sql and debug
feedback (default styled to hidden), as I have for other reports.
Signed-off-by: Joshua Ferraro <jmf@liblime.com>
need some testing on data since I donot have valid data in my fresh created database.
Note : Shouldnot updatedatabase also cope with data "moving"
when a table is created and that some data should go from one table to another ?
Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
Signed-off-by: Joshua Ferraro <jmf@liblime.com>
Rule :
- always format_date() before sending date to template
- always format_date_in_iso immediatly after reading a date coming from a parameter
- deal internally only with dates in iso
Note that :
- I could not test things related to issues stats due to the my DB -issues.issuedate not filled, see commit about 3.00.00.006
- acquisitions_stats does not work when you filter on date
Signed-off-by: Joshua Ferraro <jmf@liblime.com>
