[Bug 35216] New: Use return variable names from CanBookBeIssued in circulation.pl for consistency
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Bug ID: 35216 Summary: Use return variable names from CanBookBeIssued in circulation.pl for consistency Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com CanBookBeIssued returns: issuingimpossible and needsconfirmation variables In circulation.pl they are called errors and question Let's be consistent -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 158185 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158185&action=edit Bug 35216: Rename question to needsconfirmation -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 158186 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158186&action=edit Bug 35216: Rename error to issuingimpossible Test plan: 1 - Apply two patches 2 - Confirm code changes make sense 3 - Confirm you can check out items 4 - Confirm checkout blocks still block -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158186|0 |1 is obsolete| | --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 158187 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158187&action=edit Bug 35216: Rename error to issuingimpossible Test plan: 1 - Apply two patches 2 - Confirm code changes make sense 3 - Confirm you can check out items 4 - Confirm checkout blocks still block -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |blawlor@clamsnet.org --- Comment #4 from Brendan Lawlor <blawlor@clamsnet.org> --- I tried testing this out and verified the variable name changes look consistent. I tested checking out and checking in items, which mostly worked but if I tried to checkout a barcode that was already checked out I got this error with line 395 highlighted: Can't use string ("RENEW_ISSUE") as a HASH ref while "strict refs" in use at /kohadevbox/koha/circ/circulation.pl line 395 in (eval) at /kohadevbox/koha/circ/circulation.pl line 395 392: 393: # pass needsconfirmation to template if issuing is possible and user hasn't yet confirmed. 394: foreach my $needsconfirmation ( keys %$needsconfirmation ) { 395: $template_params->{$needsconfirmation} = $$needsconfirmation{$needsconfirmation}; 396: $template_params->{getTitleMessageIteminfo} = $biblio->title; 397: $template_params->{getBarcodeMessageIteminfo} = $item->barcode; 398: $template_params->{NEEDSCONFIRMATION} = 1; Also got this error once but I can't remember how: Can't use string ("issued_cardnumber") as a HASH ref while "strict refs" in use at /kohadevbox/koha/circ/circulation.pl line 395 in (eval) at /kohadevbox/koha/circ/circulation.pl line 395 392: 393: # pass needsconfirmation to template if issuing is possible and user hasn't yet confirmed. 394: foreach my $needsconfirmation ( keys %$needsconfirmation ) { 395: $template_params->{$needsconfirmation} = $$needsconfirmation{$needsconfirmation}; 396: $template_params->{getTitleMessageIteminfo} = $biblio->title; 397: $template_params->{getBarcodeMessageIteminfo} = $item->barcode; 398: $template_params->{NEEDSCONFIRMATION} = 1; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #5 from Brendan Lawlor <blawlor@clamsnet.org> --- I think the error I got was due to something wrong in my dev environment. I set everything up again fresh, applied the bug and re tested. This time everything worked as expected and I could not recreate the error. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #6 from David Nind <david@davidnind.com> --- Are you able to add your sign-off? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #7 from Brendan Lawlor <blawlor@clamsnet.org> --- Hi David, I am having trouble going through the process with git bz and now am afraid I could be doing something wrong. I reset the Koha clone, deleted the local branch I was testing on and tried again. git reset --hard origin/master git checkout -b bug_35216 ktd up ktd --shell git bz apply 35216 This time I got a merge conflict in circ/circulation.pl. I'm using VS Code and I went through and accepted the incoming changes and checked to make sure the code looked right. Then when I tested again, if I checked out an item that was already checked out, I got the error that I got the first time: Can't use string ("RENEW_ISSUE") as a HASH ref while "strict refs" in use at /kohadevbox/koha/circ/circulation.pl line 419 in (eval) at /kohadevbox/koha/circ/circulation.pl line 419 416: 417: # pass needsconfirmation to template if issuing is possible and user hasn't yet confirmed. 418: foreach my $needsconfirmation ( keys %$needsconfirmation ) { 419: $template_params->{$needsconfirmation} = $$needsconfirmation{$needsconfirmation}; 420: $template_params->{getTitleMessageIteminfo} = $biblio->title; 421: $template_params->{getBarcodeMessageIteminfo} = $item->barcode; 422: $template_params->{NEEDSCONFIRMATION} = 1; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #8 from David Nind <david@davidnind.com> --- Hi Brendan. This is what I normally do in KTD (assuming your Bugzilla credentials are in the .env file): 1. Koha clone - up-to-date: on master branch, git pull 2. KTD up-to-date: git pull, ktd pull 3. Start up KTD: ktd up 4. Access Koha using the shell: ktd --shell 5. Checkout a branch for testing the bug: git checkout -B bzXXXXXX 6. Apply the patches: git bz apply XXXXXXX 7. Test using the test plan, if there is one, or you know what it is about 8. Sign-off once happy: 8.1 git bz so X (X = the number of patches, in this case 2) (if you git log, your sign-off line should show at the end of the patch) 8.2 git bz attach -e XXXXXX HEAD (for one patch) or git bz attach -e XXXXXX HEAD~X.. (for multiple patches, where X = number of patches, in this case 2) 9. Check on Bugzilla that the status was changed to signed off. XXXXXX = the bug number I tried to apply the patch, but it no longer applies - I'm not a developer, so don't normally try and resolve the conflict, and change the status in Bugzilla to Patch doesn't apply. If I get an error when going through the test plan, or it doesn't work, I would normally indicate this in a comment on the bug and change the status to Failed QA. Patch doesn't apply message ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ kohadev-koha@kohadevbox:koha(bz35307)$ git bz apply 35216 Bug 35216 - Use return variable names from CanBookBeIssued in circulation.pl for consistency 158185 - Bug 35216: Rename question to needsconfirmation 158187 - Bug 35216: Rename error to issuingimpossible Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 35216: Rename question to needsconfirmation Applying: Bug 35216: Rename error to issuingimpossible error: sha1 information is lacking or useless (circ/circulation.pl). error: could not build fake ancestor Patch failed at 0001 Bug 35216: Rename error to issuingimpossible hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-35216-Rename-error-to-issuingimpossible-mjakx8dk.patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #9 from David Nind <david@davidnind.com> --- Also, once I've tested something and signed off, etc., I normally do: 1. git checkout master 2. git branch -D bzXXXXXX 3. git reset --hard origin/master 4. reset_all (resets Koha - gives you a clean setup, with needing to shut down and restart KTD) Here is a link to my KTD cheat sheet if that helps: https://gitlab.com/-/snippets/1893788 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #10 from Brendan Lawlor <blawlor@clamsnet.org> --- Thanks, David. That's a really helpful resource. I thought this bug would be a good beginner bug to test since it's just changing a couple variable names, but I guess there's more to it in this case. I'll keep watching this to see where it goes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Brendan, always happy to have a new tester around! Let us know if we can help with anything :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158185|0 |1 is obsolete| | --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159039&action=edit Bug 35216: Rename question to needsconfirmation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158187|0 |1 is obsolete| | --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159040&action=edit Bug 35216: Rename error to issuingimpossible Test plan: 1 - Apply two patches 2 - Confirm code changes make sense 3 - Confirm you can check out items 4 - Confirm checkout blocks still block -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #14 from Brendan Lawlor <blawlor@clamsnet.org> --- I tried testing this again, but when I tested circ, searching for a patron raised errors that $error and $question were undefined. Could not compile /kohadevbox/koha/circ/circulation.pl: Global symbol "$error" requires explicit package name (did you forget to declare "my $error"?) at /kohadevbox/koha/circ/circulation.pl line 391. Global symbol "$error" requires explicit package name (did you forget to declare "my $error"?) at /kohadevbox/koha/circ/circulation.pl line 392. Global symbol "$question" requires explicit package name (did you forget to declare "my $question"?) at /kohadevbox/koha/circ/circulation.pl line 427. Global symbol "$question" requires explicit package name (did you forget to declare "my $question"?) at /kohadevbox/koha/circ/circulation.pl line 448. BEGIN not safe after errors--compilation aborted at /kohadevbox/koha/circ/circulation.pl line 714. at /usr/share/perl5/CGI/Compile.pm line 144 I looked at the code for those lines and replaced a couple more instances of $error with $issuingimpossible and $question with $needsconfirmation. That resolved the error, and I could look up a patron and checkout items. But if I checked out an item that was already checked out I got the same error when testing the first patch: Can't use string ("RENEW_ISSUE") as a HASH ref while "strict refs" in use at /kohadevbox/koha/circ/circulation.pl line 420 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159039|0 |1 is obsolete| | Attachment #159040|0 |1 is obsolete| | --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159056 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159056&action=edit Bug 35216: Rename question to needsconfirmation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159057 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159057&action=edit Bug 35216: Rename error to issuingimpossible Test plan: 1 - Apply two patches 2 - Confirm code changes make sense 3 - Confirm you can check out items 4 - Confirm checkout blocks still block -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Brendan Lawlor from comment #14)
I tried testing this again, but when I tested circ, searching for a patron raised errors that $error and $question were undefined.
Ah, bookings introduced new uses, patches updated -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- still needs work -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159056|0 |1 is obsolete| | --- Comment #19 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159072 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159072&action=edit Bug 35216: Rename question to needsconfirmation -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159057|0 |1 is obsolete| | --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 159073 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159073&action=edit Bug 35216: Rename error to issuingimpossible Test plan: 1 - Apply two patches 2 - Confirm code changes make sense 3 - Confirm you can check out items 4 - Confirm checkout blocks still block -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159072|0 |1 is obsolete| | --- Comment #21 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 159085 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159085&action=edit Bug 35216: Rename question to needsconfirmation Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159073|0 |1 is obsolete| | --- Comment #22 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 159086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159086&action=edit Bug 35216: Rename error to issuingimpossible Test plan: 1 - Apply two patches 2 - Confirm code changes make sense 3 - Confirm you can check out items 4 - Confirm checkout blocks still block Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159085|0 |1 is obsolete| | --- Comment #23 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 159180 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159180&action=edit Bug 35216: Rename question to needsconfirmation Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #159086|0 |1 is obsolete| | --- Comment #24 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 159181 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=159181&action=edit Bug 35216: Rename error to issuingimpossible Test plan: 1 - Apply two patches 2 - Confirm code changes make sense 3 - Confirm you can check out items 4 - Confirm checkout blocks still block Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Status|Signed Off |Passed QA --- Comment #25 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Works, makes sense, QA script happy, code looks good, passing QA :) tested checkout - with lots of money owed - conflicting with someone else's hold - already checked out - with a past date -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #26 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- When reading the bug report, I expected the change to be the other way around, but that's ok :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #27 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- It looks like t/db_dependent/Circulation.t also uses $error and $question. I'll let you decide if that is worth a follow-up. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable Version(s)|24.05.00 |24.05.00,23.11.02 released in| | --- Comment #29 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35216 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED CC| |lucas@bywatersolutions.com Resolution|--- |FIXED --- Comment #30 from Lucas Gass <lucas@bywatersolutions.com> --- Doesn't apply clean to 23.05.x, no backport. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org