[Bug 30657] New: Make patron attributes available via Template Toolkit in overdues
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Bug ID: 30657 Summary: Make patron attributes available via Template Toolkit in overdues 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: andrew@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org When using Template Toolkit to fetch patron values, one should be able to fetch patron attribute values. This would be useful both for printing those values into the notice content and for using those values in logic to vary the notices per patron. -- 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=30657 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Michael Adamyk <madamyk@ckls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |madamyk@ckls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #1 from Michael Adamyk <madamyk@ckls.org> --- +1 This would be great if it could be applied to all notices using Template Toolkit (particularly ISSUE and HOLD slips). -- 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=30657 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- +1 -- 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=30657 George Williams (NEKLS) <george@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |george@nekls.org --- Comment #3 from George Williams (NEKLS) <george@nekls.org> --- +1 -- 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=30657 Christopher Brannon <cbrannon@cdalibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |cbrannon@cdalibrary.org --- Comment #4 from Christopher Brannon <cbrannon@cdalibrary.org> --- +1 for sure! -- 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=30657 koha-US bug tracker <bugzilla@koha-us.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bugzilla@koha-us.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us --- Comment #5 from Rebecca Coert <rcoert@arlingtonva.us> --- +1 -- 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=30657 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=30657 Barbara Johnson <barbara.johnson@bedfordtx.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |barbara.johnson@bedfordtx.g | |ov --- Comment #6 from Barbara Johnson <barbara.johnson@bedfordtx.gov> --- +1 -- 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=30657 Thibault Keromnès <thibault.keromnes@univ-paris8.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |thibault.keromnes@univ-pari | |s8.fr -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36270 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36270 [Bug 36270] Remove special handling for <<borrower-attribute:CODE>> -- 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=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com Blocks|36270 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36270 [Bug 36270] Remove special handling for <<borrower-attribute:CODE>> -- 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=30657 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- There are already a couple of options here I believe.. but I think we should perhaps make it simpler/clearer. You can use all accessors available from the Koha::Patron object assuming you've been passed said object into the template. We have a few different attribute accessors: * 'extended_attributes' which returns a resultset of attached extended attributes * 'get_extended_attribute(code)' which returns the attribute associated with the code passed (but doesn't work properly for repeatable attributes) To use those in TT: * [% FOREACH extendedattribute IN patron.extended_attributes %] <li class="patronattribute"> <span class="patronattributelabel">[% extendedattribute.type.description | html %]</span>: [% extendedattribute.description | html %] </li> [% END %] * [% SET attribute = patron.get_extended_attribute('code') %] [% attribute.description | html %] The second one is clearly simpler to use, but only works for non-repeatable attributes so you need to know a little about the underlying patron data setup. I also think it's a get wordy 'get_' and would prefer, I think, to have a simple 'extended_attribute($code)' accessor. -- 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=30657 --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Annoyingly the extended_attributes accessor is also set to be a setter which means it won't pass through any filters you want to pass.. we should really rectify that. In my opinion we should have two simple accessors that both accept standard dbic query filters. * extended_attributes($where, $attr); * extended_attribute($where,$attr); We wrap our DBIx::Clas objects and thus remove the nice easy scalar handling of _rs on accessors which means we also have to deal with that in 'clever' ways at the TT level.. I can't remember the details for that off the top of my head though. We really need to document all this better somewhere in short. -- 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=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Hackfest -- 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=30657 --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
In my opinion we should have two simple accessors that both accept standard dbic query filters.
* extended_attributes($where, $attr); * extended_attribute($where,$attr);
I like the look of this and I think you have made a very good point to make it easier to use. This will help people creating notices, but also be generally useful. In your example code: $attr would be the attribute code. What is $where here? -- 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=30657 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Sorry, I wasn't clear with what those parameters meant. They're a fairly standard pair of names for dbix query parameters.. i.e where is the SQL abstract where "field => value", then attr is other conditions like joins. But that give perhaps more complication than end users in notices want.. it's a balance between simplicity and functionality. -- 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=30657 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=30657 Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |emily.lamancusa@montgomeryc | |ountymd.gov -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36505 -- 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=30657 --- Comment #11 from Lucas Gass <lucas@bywatersolutions.com> --- A real world example I had to cook up for a HOLD_SLIP: [% SET alt_id = '' %] [% FOREACH ba IN borrower.extended_attributes %] [% IF ba.code == 'UNCLE' %] [% alt_id = ba.attribute %] [% END %] [% END %] [% IF alt_id == 'BOB' %] Warning: Bob is your uncle. [% ELSE %] Warning: Bob is NOT your uncle. [% END %] Would be nice to make it a bit easier. -- 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=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |15278 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15278 [Bug 15278] [Omnibus] Port default slips and notices from bespoke syntax to Template Toolkit -- 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=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36588 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36588 [Bug 36588] Add a template toolkit field picker to the notice editor -- 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=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |15277 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15277 [Bug 15277] Add Template Toolkit syntax for slips and notices to editor -- 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=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36270 -- 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=30657 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 169049 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169049&action=edit Bug 30657: Unit tests This patch adds some basic unit tests for dynamically added accessors based on extended patron attributes. -- 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=30657 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 169050 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169050&action=edit Bug 30657: Dynamically add attribute accessors This patch adds dynamic creation of accessors for extended patron attributes based on attribute code. It should work for Koha::Patron objects resulting from calls to Koha::Patron->new(), Koha::Patrons->search, Koha::Patrons->find and any relations that return a Koha::Patron object. We return a single value for non-repeatable attributes and an arrayref of values for repeatable ones. This should make it simple to access such attributes in Koha Notice Templates. Simply use the 'code' in your variables.. example: You've added a 'smartcard' extended attribute to your system, you would refer to it in your notice as [% patron.smartcard | html %] -- 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=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- 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=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Make patron attributes |Make patron attributes |available via Template |available via Template |Toolkit in overdues |Toolkit in notices -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=20443 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Bug 20443 adds Koha::Patron::get_extended_attribute_value which works for non-repeatable attributes fwiw. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 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=30657 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169049|0 |1 is obsolete| | --- Comment #15 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 169056 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169056&action=edit Bug 30657: Unit tests This patch adds some basic unit tests for dynamically added accessors based on extended patron attributes. 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=30657 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169050|0 |1 is obsolete| | --- Comment #16 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 169057 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=169057&action=edit Bug 30657: Dynamically add attribute accessors This patch adds dynamic creation of accessors for extended patron attributes based on attribute code. It should work for Koha::Patron objects resulting from calls to Koha::Patron->new(), Koha::Patrons->search, Koha::Patrons->find and any relations that return a Koha::Patron object. We return a single value for non-repeatable attributes and an arrayref of values for repeatable ones. This should make it simple to access such attributes in Koha Notice Templates. Simply use the 'code' in your variables.. example: You've added a 'smartcard' extended attribute to your system, you would refer to it in your notice as [% patron.smartcard | html %] 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=30657 --- Comment #17 from Brendan Lawlor <blawlor@clamsnet.org> --- Test notes: This worked but I found the name of the object was borrower instead of patron: [% borrower.party_mode | html %] The test Patron.t passed everything, but the qa script returned one failure: FAIL Koha/Patron.pm FAIL critic # TestingAndDebugging::ProhibitNoStrict: Got 1 violation(s). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- We're really inconsistent in our notice varaibles unfortunately.. sometimes it's borrower, sometimes it's patron and other times it's more descriptive like 'owner' or 'librarian'. I'd really like to try and use the more prescriptive variable names long term and in my preference, I'd rather see fewer top-level variables available and instead encourage end users to understand the relationships between things to be able to lookup the information they need via object traversal. In reality, we need to really improve the editor to help with that. Thanks for testing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36270 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36270 [Bug 36270] Remove special handling for <<borrower-attribute:CODE>> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- What are people's general opinion on the accessors proposed here? With this patch we add accessors per attribute code. For non-repatable it's as simple as: [% borrower.attribute_code %] Whilst for repeatable we have: [% FOR attribute IN borrower.attribute_code %] [% attribute %] [% END %] This adds a bit of code complexity and possibly has some effect on performance whilst we do already offer some alternatives. For non-repeatable we already support: [% attribute = borrower.get_extended_attribute(attribute_code) %] [% attribute.description %] For repeatable we have [% FOREACH ba IN borrower.extended_attributes %] [% IF ba.code == 'UNCLE' %] [% alt_id = ba.attribute %] [% END %] [% END %] Is the extra code I introduce here to simplify for the end user worth it's weight? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #20 from David Cook <dcook@prosentient.com.au> --- I'm going to be away the next few days, but I'll take a look at this next week -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #21 from Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> --- I really like the new accessors added in this patch! I do think there's a lot of value in making it more intuitive to access extended attributes in notice templates, since that can be a big usability improvement. The performance question is worth considering... If I'm correct in thinking that Koha::Patron::Attribute::Types->search(); will add an extra database call each time a patron object is created, I could see how that may add up when a script creates a large number of patron objects. Can we work the same basic logic into an AUTOLOAD instead, so that it won't run unless it's actually needed? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #22 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Well.. I did actually have some code that does it in an autoload.. we stripped it from another submission recently.. I'll dig it out -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Ray Delahunty <r.delahunty@arts.ac.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |r.delahunty@arts.ac.uk --- Comment #23 from Ray Delahunty <r.delahunty@arts.ac.uk> --- (In reply to Martin Renvoize (ashimema) from comment #18)
We're really inconsistent in our notice variables unfortunately.. sometimes it's borrower, sometimes it's patron
[...] encourage end users to understand the relationships between things to be able to lookup the information they need via object traversal.
Am I off on a tangent here? This may be one of the reasons we librarians trying to use template toolkit to tidy up our notices struggle with TT. The table is borrowers in Koha but in template toolkit it is borrower. I'm gradually knowing what to use (not that I know why). Is there something like a schema for TT (probably a silly question), or can the TT variables (accessors??) gradually replace the alligator code on the left hand side of the notice design interface? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #24 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- (In reply to Ray Delahunty from comment #23)
(In reply to Martin Renvoize (ashimema) from comment #18)
We're really inconsistent in our notice variables unfortunately.. sometimes it's borrower, sometimes it's patron
[...] encourage end users to understand the relationships between things to be able to lookup the information they need via object traversal.
Am I off on a tangent here? This may be one of the reasons we librarians trying to use template toolkit to tidy up our notices struggle with TT. The table is borrowers in Koha but in template toolkit it is borrower. I'm gradually knowing what to use (not that I know why). Is there something like a schema for TT (probably a silly question), or can the TT variables (accessors??) gradually replace the alligator code on the left hand side of the notice design interface?
Unfortunately Ray it's not quite as simple as that :(. There's lots of history here.. and some technical challenges too. Notices are all generated by called to a method called 'GetPreparedLetter'.. that method is passed the template (which you define in your notices) and a "bunch of variables" the template can use. The challenge comes in identifying which variables are likely to get passed into the call to GetPreparedLetter so you can use them in your template.. it's a bit of a 'chicken and the egg' problem.. before we call the method we can't know what's in there.. but we can't call the method until we have a template to pass it.. but how does the person building the template know what variables that can use to build their template? I've been wanting for a long time to work on cleaning up our notice generation and write such a schema so the notice editor can definitely know what variables it's getting passed for each notice type and you can therefore build notices with more knowledge. (right now the <<>> helpers are a 'best guess' and often get it wrong). Back to this bug.. the aim here is to make extended patron attributes more accessible for any notice that pass passed a patron object.. said patron objects might be exposed with various variable names.. most often 'borrower'.. but I've also seen 'librarian', 'patron', 'user', 'author' etc etc in use... we need that schema writing and maintaining in the future. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169056|0 |1 is obsolete| | --- Comment #25 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173817 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173817&action=edit Bug 30657: Unit tests This patch adds some basic unit tests for dynamically added accessors based on extended patron attributes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #169057|0 |1 is obsolete| | --- Comment #26 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173818&action=edit Bug 30657: Dynamically add attribute accessors This patch adds dynamic creation of accessors for extended patron attributes based on attribute code. It should work for Koha::Patron objects resulting from calls to Koha::Patron->new(), Koha::Patrons->search, Koha::Patrons->find and any relations that return a Koha::Patron object. We return a single value for non-repeatable attributes and an arrayref of values for repeatable ones. This should make it simple to access such attributes in Koha Notice Templates. Simply use the 'code' in your variables.. example: You've added a 'smartcard' extended attribute to your system, you would refer to it in your notice as [% patron.smartcard | html %] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #27 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173819 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173819&action=edit Bug 30657: Move to AUTOLOAD -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173819|0 |1 is obsolete| | --- Comment #28 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173901 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173901&action=edit Bug 30657: Move to AUTOLOAD -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #29 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- So, whilst this still works as an AUTOLOAD I think the code is very slightly less clear and because we actually autoload so much from the parent (include ALL the object field accessors) we don't actually save the DB call we were aiming to save at all.. We could possibly try running the parent AUTOLOAD prior to the child AUTOLOAD from within the child to negate this.. but I couldn't get that to pass the tests consistently yet. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #30 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173902 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173902&action=edit Bug 30657: Try parent method first This patch adds a try/catch inside the Koha::Patron AUTOLOAD to try and find the method on the parent, including it's AUTOLOAD, prior to running the local AUTOLOAD code to generate attribute accessors and then use the called method. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173817|0 |1 is obsolete| | --- Comment #31 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173910 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173910&action=edit Bug 30657: Unit tests This patch adds some basic unit tests for dynamically added accessors based on extended patron attributes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173818|0 |1 is obsolete| | --- Comment #32 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173911 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173911&action=edit Bug 30657: Dynamically add attribute accessors This patch adds dynamic creation of accessors for extended patron attributes based on attribute code. It should work for Koha::Patron objects resulting from calls to Koha::Patron->new(), Koha::Patrons->search, Koha::Patrons->find and any relations that return a Koha::Patron object. We return a single value for non-repeatable attributes and an arrayref of values for repeatable ones. This should make it simple to access such attributes in Koha Notice Templates. Simply use the 'code' in your variables.. example: You've added a 'smartcard' extended attribute to your system, you would refer to it in your notice as [% patron.smartcard | html %] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173901|0 |1 is obsolete| | --- Comment #33 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173912 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173912&action=edit Bug 30657: Move to AUTOLOAD -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173902|0 |1 is obsolete| | --- Comment #34 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173913 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173913&action=edit Bug 30657: Try parent method first This patch adds a try/catch inside the Koha::Patron AUTOLOAD to try and find the method on the parent, including it's AUTOLOAD, prior to running the local AUTOLOAD code to generate attribute accessors and then use the called method. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #35 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 173914 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=173914&action=edit Bug 30657: Further unit tests Test the repeatable handling and error rethrow. There's also an attempt at counting calls to the database for subsequent attribute accessors, but it fails in an odd way I'm struggling to understand, so that test is commented for now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |roadmap_24_05, | |roadmap_24_11, | |roadmap_25_05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #36 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Have you discussed adding a prefix to those accessors for safety? Something like `attr_$code`, for example. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #37 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #36)
Have you discussed adding a prefix to those accessors for safety? Something like `attr_$code`, for example.
For ILL that's exactly what we did.. but you also won't hit the attribute accessor here if you clash with an existing patron field name. Honestly, I wasn't sure which way to jump, but the basic code only felt nicer looking to me for end users. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|roadmap_24_05, | |roadmap_25_05 | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #38 from Jonathan Druart <jonathan.druart@gmail.com> --- This is nice but could be confusing. I would prefer an explicit way to retrieve the extended attributes. patron.get_extended_attribute("attribute") patron.extended_attributes.get("attribute") patron.extended_attributes["attribute"] I don't think adding (black) magic for people writing template notices is a good idea. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #39 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Small rebase needed here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #40 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- (In reply to Jonathan Druart from comment #38)
This is nice but could be confusing. I would prefer an explicit way to retrieve the extended attributes.
patron.get_extended_attribute("attribute") patron.extended_attributes.get("attribute") patron.extended_attributes["attribute"]
I don't think adding (black) magic for people writing template notices is a good idea.
I talk about exactly that in comment #19. The general replies have been they prefer these as far as I can see. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173910|0 |1 is obsolete| | --- Comment #41 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 178686 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178686&action=edit Bug 30657: Unit tests This patch adds some basic unit tests for dynamically added accessors based on extended patron attributes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173911|0 |1 is obsolete| | --- Comment #42 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 178687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178687&action=edit Bug 30657: Dynamically add attribute accessors This patch adds dynamic creation of accessors for extended patron attributes based on attribute code. It should work for Koha::Patron objects resulting from calls to Koha::Patron->new(), Koha::Patrons->search, Koha::Patrons->find and any relations that return a Koha::Patron object. We return a single value for non-repeatable attributes and an arrayref of values for repeatable ones. This should make it simple to access such attributes in Koha Notice Templates. Simply use the 'code' in your variables.. example: You've added a 'smartcard' extended attribute to your system, you would refer to it in your notice as [% patron.smartcard | html %] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173912|0 |1 is obsolete| | --- Comment #43 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 178688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178688&action=edit Bug 30657: Move to AUTOLOAD -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173913|0 |1 is obsolete| | --- Comment #44 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 178689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178689&action=edit Bug 30657: Try parent method first This patch adds a try/catch inside the Koha::Patron AUTOLOAD to try and find the method on the parent, including it's AUTOLOAD, prior to running the local AUTOLOAD code to generate attribute accessors and then use the called method. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #173914|0 |1 is obsolete| | --- Comment #45 from Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> --- Created attachment 178690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=178690&action=edit Bug 30657: Further unit tests Test the repeatable handling and error rethrow. There's also an attempt at counting calls to the database for subsequent attribute accessors, but it fails in an odd way I'm struggling to understand, so that test is commented for now -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Martin Renvoize (ashimema) <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |roadmap_25_05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #46 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Martin Renvoize (ashimema) from comment #40)
(In reply to Jonathan Druart from comment #38)
This is nice but could be confusing. I would prefer an explicit way to retrieve the extended attributes.
patron.get_extended_attribute("attribute") patron.extended_attributes.get("attribute") patron.extended_attributes["attribute"]
I don't think adding (black) magic for people writing template notices is a good idea.
I talk about exactly that in comment #19. The general replies have been they prefer these as far as I can see.
Ok but then we are now 2 to prefer something else, don't we? :D With this approach a clash is also possible with existing attributes (what you also said already). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #47 from David Cook <dcook@prosentient.com.au> --- (In reply to Jonathan Druart from comment #46)
(In reply to Martin Renvoize (ashimema) from comment #40)
(In reply to Jonathan Druart from comment #38)
This is nice but could be confusing. I would prefer an explicit way to retrieve the extended attributes.
patron.get_extended_attribute("attribute") patron.extended_attributes.get("attribute") patron.extended_attributes["attribute"]
I don't think adding (black) magic for people writing template notices is a good idea.
I talk about exactly that in comment #19. The general replies have been they prefer these as far as I can see.
Ok but then we are now 2 to prefer something else, don't we? :D
With this approach a clash is also possible with existing attributes (what you also said already).
Yeah, I think something like patron.smartcard is dangerous, because it's impossible to know what people will name their attributes. It would mean that we could never be 100% certain when adding a new column or method to the Koha::Patron object. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #48 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- It is pretty easy for me to get in trouble when writing notices: -Created a attribute called 'TEST' -Called it like this in an ISSUESLIP, borrower.TEST -It works -Tried borrower.text -Get ERROR PROCESSING TEMPLATE: undef error - The method Koha::Patron->test is not covered by tests! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Marion Durand <marion.durand@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marion.durand@biblibre.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Mathieu Saby <mathsabypro@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathsabypro@gmail.com --- Comment #49 from Mathieu Saby <mathsabypro@gmail.com> --- I would like to take advantage of this feature in the future. Do you think it could be integrated into 26.05? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #50 from Lisette Scheer <lisette@bywatersolutions.com> --- I think just doing borrower.{attr code} is too dangerous for future possibility of fields added. not sure if borrower_attr.{cod} or similar might be better? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #51 from David Cook <dcook@prosentient.com.au> --- (In reply to Lisette Scheer from comment #50)
I think just doing borrower.{attr code} is too dangerous for future possibility of fields added.
not sure if borrower_attr.{cod} or similar might be better?
+1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #52 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- This functionality would be helpful to get completed. I've just seen an example of a library using a script in their HOLD_SLIP to make the content differ based on a patron attribute. It would be simpler and more stable if they could do that with TT. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30657 --- Comment #53 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Lucas just poked me to say this is already doable, and now I've re-read Martin's previous comments saying that while this is possible now his code attempts to make it simpler. I'm all for simper. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org