Koha-community/Koha - Koha - Koha: The world's first free and open source library system

Commit Graph

Author	SHA1	Message	Date
Jonathan Druart	9d6d641d1f	Bug 17600: Standardize our EXPORT_OK 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>	8 years ago
Jonathan Druart	6f204fdf96	Bug 28591: Don't pass debug to get_template_and_user 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#\sdebug\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>	3 years ago
Jonathan Druart	638786e719	Bug 24663: Remove authnotrequired if set to 0 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>	4 years ago
Nick Clemens	6ac037f817	Bug 24840: Replace DateTime->now with dt_from_string We should use Koha::DateUtils instead of Date::Time directly This patch simplay replaces calls to now() with a call to dt_from_string() which does effectively the same thing. Probably reading the code and verifying changes is sufficient but... To test: 1 - confirm the files all compile 2 - confirm all tests pass 3 - confirm Koha still works Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>	4 years ago
Jonathan Druart	7d8b96803f	Bug 24545: Fix license statements 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>	4 years ago
Jonathan Druart	eaee34f47a	Bug 24018: Remove die "Not logged in" Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>	5 years ago
Owen Leonard	ea7245631e	Bug 21013: Missing itemtype for checkut makes patron summary print explode This patch moves retrieval of the item type description from the script to the template using the ItemTypes template plugin. To test, apply the patch and locate an item which is checked out to someone. Modify the database record for that item to remove the item type (items.itype). View the print summary for the patron who has that item checked out. The page should display correctly. Checked-out items which have an item type should show that item type description correctly. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>	5 years ago
Katrin Fischer	3c2cc8da26	Bug 22120: Add price formattig to patron summary print The patron summary print lists the replacement prices and rental charges of items and totals them. With this patch they will be formatted according to the CurrencyFormat system preference. To test: - Find or create a patron with some checkouts - Make sure some items have replacement price set and some have a rental charge - "Print summary" from the account in staff - Verify that charges and prices in the checkouts table are formatted correctly - Toggle CurrencyFormat to different settings Signed-off-by: helene hickey <hickeyhe@wgc.school.nz> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>	5 years ago
Nick Clemens	0fd329724b	Bug 20656: Don't print lines that have no balance on print summary To test: 1 - Add at least two fines to a patron 2 - Pay off one of them 3 - Print summary - all 3 lines show 4 - Apply patch 5 - Print summary - only line with balance shows Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>	6 years ago
Jonathan Druart	0161a106e7	Bug 19935: Replace GetPendingIssues - summary-print We are in the notices part, so we need to fetch all the data to avoid regressions. Test plan: Print a summary slip before and after this patch. They must be the same Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>	6 years ago
Jonathan Druart	51aa6db46c	Bug 12001: Move GetMemberAccountRecords to the Koha namespace The GetMemberAccountRecords may be a perf killer, it retrieves all the account lines of a patron and then the related item and biblio information. Most of the time we only want to know how much the patron owns to the library (sum of amountoutstanding). We already have this information in Koha::Patron->account->balance. This patch replaces the occurrences of this subroutine by fetching only the information we need, either the balance, the detail, or both. It removes the formatting done in the module, to use the TT plugin 'Price' instead. There is a very weird and error-prone behavior/feature in GetMemberAccountBalance (FIXME): as the accountlines.accounttype is a varchar(5), the value of the authorised value used for the ManInvInNoissuesCharge pref (category MANUAL_INV) is truncated to the 5 first characters. That could lead to unexpected behaviors. On the way, this patchset also replace the GetMemberAccountBalance subroutine, which returns the balance, the non issues charges and the other charges. We only need to have the balance and the non issues charges to calcul the third one. Test plan: Add several fees for a patron and play with HoldsInNoissuesCharge, RentalsInNoissuesCharge and ManInvInNoissuesCharge. The information (biblio and item info, as well as the account line) must be correctly displayed on the different screens: 'Fines' module, fine slips, circulation module Note that this patchset could introduce regression on price formatting, but will be easy to fix using the TT plugin. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>	6 years ago
Jonathan Druart	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>	7 years ago
Jonathan Druart	cee2cf9ff9	Bug 18403: Add sub output_and_exit_if_error - unknown_patron & cannot_see_patron_infos Test plan: Login with a patron that is not allowed to see patron's information for patrons outside of his group. Try to access patron's information from scripts of the patron module (members/) and circ/circulation.pl. You should be able to access patron's information of patrons outside of your group and get "You are not allowed to see the information of this patron." If you try and access a patron page with a borrowernumber that does not exist, you should get "This patron does not exist" Technical note: A new C4::Output subroutine is created in this patch: "output_and_exit_if_error" Executed at the beginning of the script it will permit not to copy/paste all the different checks to know if the logged in user is authorised to see patron's information. The design here can be discussed, but I did not find an alternative with as less changes. On the way I refactor what we did with 'unknowuser' previously: it will now work with all patron pages, not only the few that used it. Note that the 'or die "Not logged in";' part should not be needed, but... who trusts C4::Auth? I think it could be used as a safeguard later. I am willing to sed and remove them if required. Changes in discharge.pl are mainly indentation changes. With this patch we should now have a $patron variable that refer to the patron we want to access. That will be very useful to remove plenty of code in members/ and only pass this variable to the template (instead of 1 variable per patron's attribute). Signed-off-by: Signed-off-by: Jon McGowan <jon.mcgowan@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>	7 years ago
Jonathan Druart	9af6c4e34b	Bug 19080: Handle non-existing patrons gratefully This is a recurrent bug we have over the last years. When a script is called with non-existent borrowernumber it will crashes. We need to handle this gracefully instead of letting the script crashes. On bug 18403 a new subroutine is added to the codebase (output_and_exit_if_error) to handle this kind of errors correctly. Since it is not pushed yet, I propose to just redirect to a script that handle it correctly (circulation.pl) instead of adding this message to all these scripts. Test plan: Hit different scripts from the members module and pass a non-existent borrowernumber. You must be redirected to circulation.pl with a friendly message. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>	7 years ago
Jonathan Druart	2b90ea2cb0	Bug 17829: Move GetMember to Koha::Patron 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>	8 years ago
Marcel de Rooy	ca0bde1e7e	Bug 17843: [QA Follow-up] Some polishing Resolve warning from members/summary-print.pl: "my" variable $itemtype masks earlier declaration in same scope Test if find returns a Koha object in GetDescription. Test if find returns a Koha object too in shelves.pl. While testing, I had a crash on a biblioitem with itemtype NULL (bad record, but these things tend to happen somehow.) Can't call method "imageurl" on an undefined value at virtualshelves/shelves.pl line 253. Same for opac/opac-shelves.pl. Note: Did not add tests everywhere but generally, I have the impression that we do not sufficiently test on the results of Koha::Object->find. Mostly we just assume that it will find a record. Several reports include fixes to resolve that wrong assumption. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>	7 years ago
Jonathan Druart	091d6c513b	Bug 17843: Replace C4::Koha::getitemtypeinfo with Koha::ItemTypes The C4::Koha::getitemtypeinfo subroutine did the almost same job as GetItemTypes. On top of that it returned the imageurl value processed by C4::Koha::getitemtypeimagelocation. This value is only used from the 2 [opac-]shelves.pl scripts. Then it's better not retrieve it only when we need it. Test plan: Play with the different scripts touched by this patch and focus on item types. The same description as prior to this patch must be displayed. Note that sometimes it is not the translated description which is displayed, but that should be fixed on another bug report. Indeed we do not expect this patch to change any behaviors. Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>	8 years ago
Jonathan Druart	2cf1e97e5c	Bug 17128: Make summary-print.pl plack safe $borrowernumber is used in build_issue_data but not correctly defined (Variable "$borrowernumber" is not available) That may cause wrong charge displayed in the summary slip. Test plan: - Set rental charge for an item type - Define a rental discount for that item type in the circ rules - check in an item matching this rule Without this patch the charge displayed in the summary slip won't be calculated with the discount With this patch applied, the warning in the logs will no longer appear and the values will be correctly calculated. 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>	8 years ago
Marc Véron	492a64cef9	Bug 17100: Do not display payments if patron has nothing to pay This alternative patch moves logic and formatting to the template file. To test: * without patch 1/ find a patron with no lines in accountlines table : print summary shows no "account fines and payments" => OK 2/ find a patron with some lines in accountlines table and the total amount > 0 : print summary shows a table "account fines and payments" with fines to recover => OK 3/ find a patron with some lines in accountlines table but the total amount = 0 : print summary shows a table "account fines and payments" with nothing in it => NOK * with the patch, same cases as before : 1/ same as without patch 2/ same as without patch 3/ print summary does not show "account fines and payments" - Additionally, verify that formatting follows syspref 'CurrencyFormat' - Verify that amount column is right-aligned Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>	8 years ago
Jonathan Druart	bde685fa92	Bug 15542: Always display the patron's info the same way. The patron's information displayed in the member module (includes/circ-menu.inc and includes/member-display-address-style-*.inc) are not always displayed the same way. Sometimes the streetnumber is missing, sometimes it's the streettype. Sometimes the streettype is after the address, sometimes before... Test plan: Go on a patron detail page, and open all the tabs on the left (Check out, Fines, Notices, etc.) Without this patch, the patron's info displayed will differ from one page to another. With this patch, they will be displayed the same everywhere. Followed test plan, works as expected. (Tested both patches together.) Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com	9 years ago
Aleisha	8255e2ec1f	Bug 10468: (followup) fix indentation Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>	9 years ago
Aleisha	d46f180b2e	Bug 10468: Adding holds table to summary print To test: 1) Add a hold to a patron 2) Go to patron page 3) Click Print Summary 4) Confirm that 'Pending Holds' table displays with correct information under appropriate headings (should be Title, Author, Placed on (reserve date), Expires on (expiration date), and Pick up library) Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>	9 years ago
Marc Véron	c292dde75b	Bug 15068: Get rid of warnings in members/summary-print.pl This patch removes warnings when printing a summary from a member's detail page, like: - use of uninitialized value in sprintf at /usr/share/kohaclone/members/summary-print.pl line 47 - Use of uninitialized value $roadtype in concatenation (.) or string at /usr/share/kohaclone/members/summary-print.pl line 61 - Use of uninitialized value in addition (+) at /usr/share/kohaclone/members/summary-print.pl line 87 - Argument "2015-11-03 23:59:00" isn't numeric in numeric comparison (<=>) at /usr/share/kohaclone/members/summary-print.pl line 103 To test: - Apply patch - Go to a detail page with a member who has a lot of fines - Print summary - Verify that warnings like the ones above do no longer appear. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Have been able to see those warnings in Apache log file, and notice their disappearance after applying this patch. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>	9 years ago
Jonathan Druart	c0b51eafdd	Bug 10423: Show date due with time if it's a hourly loan On printing the summary from the patron account, the hours are not displayed if needed. The as_date_due flag should be set to display it correctly. Problem, GetPendingIssues modify the value retrieved from the database. In order to not add regression and check all calls to GetPendingIssues, this patch backup the value before the change. Test plan: Check some items out, specify a hourly loan for some. Click on print > print summary and confirm the date due are correctly formatted. Followed test plan. Date + time display as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	9 years ago
Kyle Hall	1e98ff655d	Bug 12971 [QA Followup] - Fix error caused by patron not having any checkouts Signed-off-by: Owen Leonard <oleonard@myacpl.org> This successfully fixes the problem observed when a patron has no checkouts. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	10 years ago
Kyle Hall	2ca4841ea4	Bug 12971: [QA Followup] * Makes the status column display "Overdue!" if overdue * Fixes the due date formatting * Sorts the checkouts by date due ( oldest to newest ) Note: I found no evidence that this data was previously sorted, so I kept it simple. Sorting based on system preferences could be a future enhancement. Signed-off-by: Owen Leonard <oleonard@myacpl.org> This fixes the issues described for patrons with existing checkouts. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	10 years ago
Kyle Hall	255f02cad1	Bug 12971 - Regression: Patron print summary doesn't show checkouts A patron's print summary should contain a list of checked out items as it did in 3.16.2 and earlier. Please note, as of 3.16.2 reserves were no longer part of the print summary and thus are not part of this bug fixing patch. Test Plan: 1) Find a patron with checked out items 2) Choose Print -> Print summary 3) Note the lack of a list of checkouts 4) Apply this patch 5) Reload the page 5) Print the summary again 6) Note the list of checkouts Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Works as described, apart from the missing status information that Owen already noted on the bug. Passes tests and QA script. Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>	10 years ago

27 Commits (36ef9a219de1f76bd460c2b75cdd4bb67e79ca68)