| Summary: | Warns in subscription-add.pl | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Aleisha Amohia <aleisha> |
| Component: | Serials | Assignee: | Aleisha Amohia <aleisha> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | minor | ||
| Priority: | P5 - low | CC: | brendan, colin.campbell, f.demians, jonathan.druart, joonas.kylmala, julian.maurice |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Bug 14641: Warns in subscription-add.pl
Bug 14641: Warns in subscription-add.pl Bug 14641: Warns in subscription-add.pl Bug 14641: [SIGNED-OFF] Warns in subscription-add.pl Bug 14641: [SIGNED-OFF] Warns in subscription-add.pl |
||
|
Description
Aleisha Amohia
2015-08-04 02:31:03 UTC
Created attachment 41325 [details] [review] Bug 14641: Warns in subscription-add.pl I think the warns are being triggered because there are brackets around it (like my ($selected_lettercode) ) which doesn't seem necessary as we're only declaring one variable? So I've removed the brackets. To test: 1) Reproduce warns by clicking New Subscription 2) Apply patch 3) Click New Subscription again 4) Confirm warns are gone The warning is triggered when get_letter_loop() is called without an argument. We should maybe check if @_ is true, and if it's not then use $_ or "" as the value? The brackets should be kept, for consistency at least. I haven't tested, but this patch is certainly wrong. Moreover, tests should be provided. Created attachment 45623 [details] [review] Bug 14641: Warns in subscription-add.pl These warns now appear on line 242. Have changed get_letter_loop() to be empty unless given a value ( get_letter_loop //= '') To test: 1) Reproduce warns by clicking New Subscription 2) Apply patch 3) Click New Subscription again 4) Confirm warns are gone Aleisha, with this patch the parameter sent to the subroutine is not taken into account anymore. (In reply to Joonas Kylmälä from comment #2) > The warning is triggered when get_letter_loop() is called without an > argument. We should maybe check if @_ is true, and if it's not then use $_ > or "" as the value? I'm pretty new to perl so don't really know how I would write this. Do you have any suggestions so that we can give this bug a patch? The verbose way to do it is:
my ($selected_lettercode) = @_;
if ( not defined $selected_lettercode ) {
$selected_lettercode = '';
}
A better way, IMO is to use `unless` instead of `if not`
my ($selected_lettercode) = @_;
unless ( defined $selected_lettercode ) {
$selected_lettercode = '';
}
A much more better way is to do the same but more readable (because less verbose):
my ($selected_lettercode) = @_;
$selected_lettercode //= '';
`//=` means `assign what is at the right if what is at the left is not defined`
`||=` would means `assign what is at the right if what is at the left is not defined or is 0 or is an empty string`
Created attachment 46500 [details] [review] Bug 14641: Warns in subscription-add.pl Removes warn using Jonathan's suggestion! To test: 1) Reproduce warns by clicking New Subscription 2) Apply patch 3) Click New Subscription again 4) Confirm warns are gone Created attachment 46560 [details] [review] Bug 14641: [SIGNED-OFF] Warns in subscription-add.pl Removes warn using Jonathan's suggestion! To test: 1) Reproduce warns by clicking New Subscription 2) Apply patch 3) Click New Subscription again 4) Confirm warns are gone Signed-off-by: Magnus Enger <magnus@libriotech.no> Patch removes the warn that come when clicking "New subscription" Created attachment 46580 [details] [review] Bug 14641: [SIGNED-OFF] Warns in subscription-add.pl Removes warn using Jonathan's suggestion! To test: 1) Reproduce warns by clicking New Subscription 2) Apply patch 3) Click New Subscription again 4) Confirm warns are gone Signed-off-by: Magnus Enger <magnus@libriotech.no> Patch removes the warn that come when clicking "New subscription" Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to Master - Should be in the May 2016 release. Thanks! Patch pushed to 3.22.x, will be in 3.22.3 This patch has been pushed to 3.20.x, will be in 3.20.9. |