[Bug 19966] New: Add ability to pass objects directly to slips and notices
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Bug ID: 19966 Summary: Add ability to pass objects directly to slips and notices Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Koha spends an incredible amount of time on parsing and processing parameters passed in to slips and notices. It would be immensely more efficient to be able to pass objects directly to GetPreparedLetter so it doesn't need to do any fetching / processing on them. -- 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=19966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@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=19966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 70470 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=70470&action=edit Bug 19966 - Add ability to pass objects directly to slips and notices Koha spends an incredible amount of time on parsing and processing parameters passed in to slips and notices. It would be immensely more efficient to be able to pass objects directly to GetPreparedLetter so it doesn't need to do any fetching / processing on them. Test plan: 1) Apply this patch 2) prove t/db_dependent/Letters/TemplateToolkit.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19191 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19191 [Bug 19191] Add ability to email receipts for account payments and write-offs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Josef Moravec <josef.moravec@gmail.com> 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=19966 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #70470|0 |1 is obsolete| | --- Comment #2 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 71998 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=71998&action=edit Bug 19966 - Add ability to pass objects directly to slips and notices Koha spends an incredible amount of time on parsing and processing parameters passed in to slips and notices. It would be immensely more efficient to be able to pass objects directly to GetPreparedLetter so it doesn't need to do any fetching / processing on them. Test plan: 1) Apply this patch 2) prove t/db_dependent/Letters/TemplateToolkit.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | CC| |jonathan.druart@bugs.koha-c | |ommunity.org, | |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Kyle: do you think some other methods could be blacklisted? (_resultset, etc). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 72013 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=72013&action=edit [FOR DISCUSSION] Bug 19966: Add Koha::Object->read_only If ->read_only is called, only accessor methods will be allowed on this object. use Koha::Patrons; my $p = Koha::Patrons->find(1); say $p->borrowernumber; $p->surname('another surname')->store; say $p->surname; => Will work $p = Koha::Patrons->find(1); $p->read_only; $p->surname('another surname again')->store; => Will explode Problem: use Koha::Patrons; my $p = Koha::Patrons->find(1); say $p->borrowernumber; $p->read_only; $p->{_read_only} = 0; $p->surname('another surname again')->store; => Will not explode -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Kyle and Tomas, What about this last patch? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #5)
Kyle and Tomas, What about this last patch?
I think (in terms of the problem you highlight) that the read_only flag should be passed when creating the object, and immutable. Like this: my $patrons = Koha::Patrons->search( $criteria, $attributes, $read_only ); And then, it should be inherited by each of the generated objects: while (my $patron = $patrons->next ) { print "Patron " . $patron->id . " readonly!" if $patron->read_only; } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Yes I thought about that, but it may be problematic as we do not necessarily create the object for the notices only. So if we do it that way (when creating it) we may need to fetch it twice. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #7)
Yes I thought about that, but it may be problematic as we do not necessarily create the object for the notices only. So if we do it that way (when creating it) we may need to fetch it twice.
I agree. I don't think we should have to fetch an object twice. I think the fact that you can set read_only but not unset it is sufficient. Basically, and object can be passed around the entire lifecycle of a script call, then be marked read_only at the end and passed to the template. The *other* option would be a new_from_object method that close the passed in object. Then you could have read_only be immutable and set on creation. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Kyle M Hall from comment #8)
(In reply to Jonathan Druart from comment #7)
Yes I thought about that, but it may be problematic as we do not necessarily create the object for the notices only. So if we do it that way (when creating it) we may need to fetch it twice.
I agree. I don't think we should have to fetch an object twice. I think the fact that you can set read_only but not unset it is sufficient. Basically, and object can be passed around the entire lifecycle of a script call, then be marked read_only at the end and passed to the template.
The *other* option would be a new_from_object method that close the passed in object. Then you could have read_only be immutable and set on creation.
I agree with either option, but we still need a way to tell which methods have/don't have side effects that should then be forbidden. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Jessie Zairo <jzairo@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jzairo@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|19191 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19191 [Bug 19191] Add ability to email receipts for account payments and write-offs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 123106 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=123106&action=edit Bug 19966: Add ability to pass objects directly to slips and notices Koha spends an incredible amount of time on parsing and processing parameters passed in to slips and notices. It would be immensely more efficient to be able to pass objects directly to GetPreparedLetter so it doesn't need to do any fetching / processing on them. Test plan: 1) Apply this patch 2) prove t/db_dependent/Letters/TemplateToolkit.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #71998|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #72013|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- This patch has been stuck at In Discussion for three years do to a related be separate issue. To get this moving I've split my patch into two parts, retaining the ability to pass objects to slips and notices here, while the rest of mine and jonathan's patch have been moved to bug 28739 where we can discuss the issue, which has existed from before this enhancement, and continues to exist to this day. Considering the issues raised are in Koha now, I see no reason for this patch to be stymied by them. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- I dont want to disappoint you but the whole discussion on bug 28883 will have a bearing on this one too. The point is that TT calls methods in list context. Which is what you dont want when chaining. So e.g. object.filterbyX.count does not even work when filter returns a list while in perl would return an iterator here. And specific to this bug, how would a 'power user' editing a notice know that difference? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am sorry, but there is a little issue here. Maybe some variables are not reset? - Place multiple holds for different patrons on a record - Click on the X to cancel a single hold, verify the question has the correct information and abort with cancel. - Use the checkboxes to mark multiple holds for cancelling - Use the "Cancel selected" button - The confirmation will still ask about cancelling the hold for the perevious user, although you are cancelling multiple for different users. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Wrong bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This patch is required for the conversion of digest notices to TT.. it would be really beneficial to see it moving again. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #123106|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 128546 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=128546&action=edit Bug 19966: Add ability to pass objects directly to slips and notices Koha spends an incredible amount of time on parsing and processing parameters passed in to slips and notices. It would be immensely more efficient to be able to pass objects directly to GetPreparedLetter so it doesn't need to do any fetching / processing on them. Test plan: 1) Apply this patch 2) prove t/db_dependent/Letters/TemplateToolkit.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Really glad this bug got split up.. it's now a simple QA. Everything works as described and causes no obvious regressions. QA Script is happy. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|tomascohen@gmail.com |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- What are the next steps then? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Marcel de Rooy from comment #12)
I dont want to disappoint you but the whole discussion on bug 28883 will have a bearing on this one too. The point is that TT calls methods in list context. Which is what you dont want when chaining. So e.g. object.filterbyX.count does not even work when filter returns a list while in perl would return an iterator here.
And specific to this bug, how would a 'power user' editing a notice know that difference?
I would argue that this bug has no change of effect here.. whilst we don't allow passing objects through from the GetPreperaredLetter call yet, we DO already pass objects into the actual template parser from within GetPreparedLetter.. pass tables etc and you get given objects back. As such, I think we can safely push this and defer the decision/challenge of context as it's a pre-existing issue. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28739 --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Ok but so, what's next? bug 28739? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |28739 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #20)
Ok but so, what's next? bug 28739?
Yes, that seems like an excellent companion to this bug. I've set this one as a dependency for it. Feel free to change that to a see also if you think it is incorrect! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #22 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- The dependency is reversed IMO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #22)
The dependency is reversed IMO.
I agree with the current way around... we already expose object methods to notices.. this patch doesn't change or expose anything worse so blocking this one with that one would be silly in my opinion. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30751 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30751 [Bug 30751] Print autorenewal notice to console when running in verbose. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_22_11_candidate |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Needs documenting CC| |lucas@bywatersolutions.com --- Comment #25 from Lucas Gass <lucas@bywatersolutions.com> --- Enhancement will not be backported 22.05.x series -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED CC| |caroline.cyr-la-rose@inlibr | |o.com --- Comment #26 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Not sure what needs documenting, seems like backend stuff. Feel free to enlighten me, if needed :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |37198 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37198 [Bug 37198] POD for GetPreparedLetter doesn't include 'objects' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19966 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org