[Bug 8004] New: Items on Hold Lose Transfer After Being Scanned Twice
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Priority: P5 - low Change sponsored?: --- Bug ID: 8004 CC: gmcharlt@gmail.com Assignee: koha.sekjal@gmail.com Summary: Items on Hold Lose Transfer After Being Scanned Twice QA Contact: koha.sekjal@gmail.com Severity: enhancement Classification: Unclassified OS: All Reporter: kyle.m.hall@gmail.com Hardware: All Status: NEW Version: rel_3_6 Component: Hold requests Product: Koha When a hold is placed on an item where the pickup location is different than the holding library, Koha initiates a branch transfer for that item to fill that hold when items is run through the returns system. If the item is then run through returns a second time, the system is supposed to close that transfer as a 'wrongtransfer', and open a new transfer with the same from and to branches as the original. The problem is that the original transfer is closed, but the new transfer is not created. This is because at some point, someone replaced the template variable WrongTransfer, which had previously contained the branchcode for the library to transfer to, with the full name of the library instead ( I assume to make a look nicer ). The consequence of this, is the the name of the library is passed to updateWrongTransfer instead of the branchcode, causing the failure. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 --- Comment #1 from Kyle M Hall <kyle.m.hall@gmail.com> --- Created attachment 9289 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=9289&action=edit Bug 8004 - Items on Hold Lose Transfer After Being Scanned Twice When a hold is placed on an item where the pickup location is different than the holding library, Koha initiates a branch transfer for that item to fill that hold when items is run through the returns system. If the item is then run through returns a second time, the system is supposed to close that transfer as a 'wrongtransfer', and open a new transfer with the same from and to branches as the original. The problem is that the original transfer is closed, but the new transfer is not created. This is because at some point, someone replaced the template variable WrongTransfer, which had previously contained the branchcode for the library to transfer to, with the full name of the library instead ( I assume to make a look nicer ). Solved by removing the line that was changing the variable contents from the branchcode to the branch name, and adding a new TT filter, KohaBranchName that functions in the same way as KohaDates, except it takes a branchcode and returns the branch name for the given branchcode. The consequence of this, is the the name of the library is passed to updateWrongTransfer instead of the branchcode, causing the failure. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Melissa Geist <mgeist@sekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |mgeist@sekls.org --- Comment #2 from Melissa Geist <mgeist@sekls.org> --- Test with a sandbox. Works as advertised. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.poulain@biblibre.com --- Comment #3 from Paul Poulain <paul.poulain@biblibre.com> --- QA comment: * Kyle, what is fun is the answer to your question:
This is because at some point, someone replaced the template variable WrongTransfer, which had previously contained the branchcode for the library to transfer to, with the full name of the library instead ( I assume to make a look nicer ). git blame circ/returns.pl, says: 74f6e35c (Kyle M Hall 2010-04-01 15:45:25 +0000 340) $messages->{'WrongTransfer'} = GetBranchName( $messa...
:D :D (and, incenditally, it's 2 years old !) However, this is a tiny patch, that adds a new plugin. After the presentation chris made today about T::T plugins/views/..., i'm wondering if creating a plugin for that is the best option we have. Shouldn't it be a view ? Plus, if I push this patch, it means we should get rid of any branchname display everywhere, remove LEFT JOIN branches USING(branchname) in many SQL queries, cache GetBranchName to avoid useless SQL queries (branches are not updated frequently, they are a good candidate for caching). I'm not against this idea (in fact, I like it quite a lot), but we must advertise it as much as possible, and if you can promize you'll take care of patches cleaning branchname dependancies, that would be a good + ;-) I won't push the patch immediately, I suggest we speak of it tomorrow with other hackers, and chris_c first, as he is experienced on T::T -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 --- Comment #4 from Kyle M Hall <kyle.m.hall@gmail.com> --- I guess I need to yell at myself from two years ago ; ) I don't think this is a good case for a view, considering the simplicity of the action. We can definitely discuss it though. I do think we could indeed get rid of perl code that gets the branchname and replace it with this filter, it will make code simpler and more understandable. It seems more correct to me, considering it is a matter of presentation, which is the job of the templates. (In reply to comment #3)
QA comment:
* Kyle, what is fun is the answer to your question:
This is because at some point, someone replaced the template variable WrongTransfer, which had previously contained the branchcode for the library to transfer to, with the full name of the library instead ( I assume to make a look nicer ). git blame circ/returns.pl, says: 74f6e35c (Kyle M Hall 2010-04-01 15:45:25 +0000 340) $messages->{'WrongTransfer'} = GetBranchName( $messa...
:D :D (and, incenditally, it's 2 years old !)
However, this is a tiny patch, that adds a new plugin. After the presentation chris made today about T::T plugins/views/..., i'm wondering if creating a plugin for that is the best option we have. Shouldn't it be a view ?
Plus, if I push this patch, it means we should get rid of any branchname display everywhere, remove LEFT JOIN branches USING(branchname) in many SQL queries, cache GetBranchName to avoid useless SQL queries (branches are not updated frequently, they are a good candidate for caching). I'm not against this idea (in fact, I like it quite a lot), but we must advertise it as much as possible, and if you can promize you'll take care of patches cleaning branchname dependancies, that would be a good + ;-)
I won't push the patch immediately, I suggest we speak of it tomorrow with other hackers, and chris_c first, as he is experienced on T::T
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha.sekjal@gmail.com |kyle.m.hall@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 --- Comment #5 from Kyle M Hall <kyle.m.hall@gmail.com> --- Spoke to Chris about his, he also feels that the filter is the right way to go. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master Version|rel_3_6 |rel_3_10 --- Comment #6 from Paul Poulain <paul.poulain@biblibre.com> --- Patch pushed. thinking of it Kyle, I think naming it KohaBranchName.pm could be improved to Branchname.pm, because it's all about Koha, so... if you want to change the name in the follow-ups you'll do... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Items on Hold Lose Transfer |Items on hold lose transfer |After Being Scanned Twice |after being scanned twice -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #9289|0 |1 is obsolete| | --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 13536 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13536&action=edit Bug 8004 - Followup - Rename KohaBranchName filter to BranchName -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |ASSIGNED CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13536|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 13537 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13537&action=edit Bug 8004 - Followup - Rename KohaBranchName filter to BranchName -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #9 from Owen Leonard <oleonard@myacpl.org> --- Please add a commit message with a test plan. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13537|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 17911 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17911&action=edit Bug 8004 - Followup - Rename KohaBranchName filter to BranchName This patch renames the TT filter KohaBranchName to just BranchName, as the Koha is superfluous. Test Plan: 1) Apply patch 2) Attempt to transfer an item to the incorrect branch, you should see the name of the branch in the "please return to" message. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #17911|0 |1 is obsolete| | --- Comment #11 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 20582 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20582&action=edit [SIGNED-OFF] Bug 8004 - Followup - Rename KohaBranchName filter to BranchName This patch renames the TT filter KohaBranchName to just BranchName, as the Koha is superfluous. Test Plan: 1) Apply patch 2) Attempt to transfer an item to the incorrect branch, you should see the name of the branch in the "please return to" message. Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 --- Comment #12 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 20583 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20583&action=edit Bug 8004 - Followup [follow-up] Rename KohaBranchName filter to BranchName Since the original follow-up to 8004 was submitted the KohaBranchName filter was put to use in several other templates and a test was added. This patch corrects the filter name in the templates and changes the name of, and in, the test. To test: - Enable IndependentBranches and log in to Koha as a non-superlibrarian. Try to check out an item which belongs to another library. The library name should appear correctly in the warning message. - In Acquisitions, view the list of basket groups for any vendor. The "billing place" and "delivery place" columns should show library names correctly. - View the detail page for a subscription (serials/subscription-detail.pl?subscriptionid=X). The library name should appear correctly. - In suggestions management, view the details for a suggestion (/suggestion/suggestion.pl?suggestionid=X&op=show). Library names should appear correctly in the "suggestions management" table and in the "acquisition information" block. - Test the Koha_template_plugin_BranchName.t test: In a test database that has the sample branchesloaded, run prove -v t/db_dependent/Koha_template_plugin_BranchName.t -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|3.10 |master -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Campbell Reid-Tait <campbellreidtait@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20583|0 |1 is obsolete| | CC| |campbellreidtait@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Campbell Reid-Tait <campbellreidtait@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Assignee|kyle.m.hall@gmail.com |campbellreidtait@gmail.com --- Comment #13 from Campbell Reid-Tait <campbellreidtait@gmail.com> --- Created attachment 20682 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20682&action=edit Signed off patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Assignee|campbellreidtait@gmail.com |kyle@bywatersolutions.com QA Contact|koha.sekjal@gmail.com | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA Depends on| |10626 Severity|enhancement |normal --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Kyle, I am afraid this conflicts with 10626 - could you check please? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- I'm a but confused about the state of this bug. The initial patch is in master. Do we still need these followups? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20682|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20582|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8004 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |RESOLVED Resolution|--- |FIXED --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Initial patch was pushed to master. These followups are covered by bug 10626, and thus are not needed. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org