[Koha::Object] Related records and calling methods from templates
Hi devs, I am seeing more Koha objects that include methods for related records like Koha::Patron, sub image referring to Koha::Patron::Images. As a second example, on bug 14610 (article requests), a biblio, item, branch and borrower method is added to Koha::ArticleRequest. Obviously, they all follow the same pattern. Could we generalize that, and keep our code cleaner? And do we always need the full Koha object? For use in a template we may need only a few columns. We start passing these objects to the templates now too, making it possible to call methods from the template. It is possible, but do we really want that? More code ends up in the template, maybe harder to find since you now use borrower.method instead of borrower->method etc. Secondly, reading borrower.something might not immediately tell you that something is a method instead of a column. In other cases we have added a sort of wrapper like Branches.GetName from the Branches plugin. I am interested to know if we want to enforce this wrapper approach for instance, or just continue on the current road. Thx, Marcel
I'm not entirely sure what you mean in the first paragraph, so I don't have much to add there. Personally, I like having access to objects in the templates. In terms of CRUD, it seems to make writing interfaces much easier and cleaner. I suppose you could say that more code ends up in the template, but you could also say that more of the presentation ends up in the template as well I think. I suppose that's a debateable point. As for borrower.something vs borrower->something, I don't know if it matters if it looks like a method or a column. It would be quick enough to find out if it mattered. I'm not sure what you mean by the wrapper approach exactly, but personally I think we could do with fewer wrappers and just try to keep things as simple as possible. Re-reading your example, do you mean that you'd pass a branch code to the template and then use Branches.GetName from the Branches plugin to replace that with the branch name? In theory, it would be nice if you could do item.branchcode.branchname (where branchcode is a foreign key and using the branchname method causes the Item object to fetch the corresponding Branch object), which is something that I recall Jonathan doing perhaps in some of the templates? Of course, I think using the plugin approach, you might be able to add more multilingual support for other things. For instance. item.itype would retrieve "BOOK". Item.itype.item_description would be "Book", but if you used ItemTypes.GetName (or something like that), you could possibly choose a translated template instead of just the value from the database. Since most of my work is in English, I'll defer to others regarding translations in templates. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595 From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] On Behalf Of Marcel de Rooy Sent: Thursday, 15 September 2016 9:03 PM To: koha-devel@lists.koha-community.org Subject: [Koha-devel] [Koha::Object] Related records and calling methods from templates Hi devs, I am seeing more Koha objects that include methods for related records like Koha::Patron, sub image referring to Koha::Patron::Images. As a second example, on bug 14610 (article requests), a biblio, item, branch and borrower method is added to Koha::ArticleRequest. Obviously, they all follow the same pattern. Could we generalize that, and keep our code cleaner? And do we always need the full Koha object? For use in a template we may need only a few columns. We start passing these objects to the templates now too, making it possible to call methods from the template. It is possible, but do we really want that? More code ends up in the template, maybe harder to find since you now use borrower.method instead of borrower->method etc. Secondly, reading borrower.something might not immediately tell you that something is a method instead of a column. In other cases we have added a sort of wrapper like Branches.GetName from the Branches plugin. I am interested to know if we want to enforce this wrapper approach for instance, or just continue on the current road. Thx, Marcel
Hello people, First of all I wishes everyone have spent a nice summer! At my work we're trying to use ILSDI to get availability of all items for a biblionumber. Thanks to Julian we were able to use the GetAvailability method to get availability for one item at a time. https://catalogue.univ-lyon3.fr/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=510054&id_type=item According to the documentation it is possible to query this script with biblionumber rather than itemnumber to get availability of all items in one shot. https://catalogue.univ-lyon3.fr/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=334726&id_type=bib&return_type=item The late query is then supposed to work but it doesn't. Am I doing something wrong? Thanks in advance for your wisdom and enlightenment! Best regards, Arthur Suzuki University Jean Moulin Lyon 3
Hello Arthur, You're not doing anything wrong. The GetAvailability implementation in Koha works only when id_type=item. And I believe the parameter 'return_type' is not used at all. Please open a bug report on https://bugs.koha-community.org/ On 16/09/2016 10:45, SUZUKI Arthur wrote:
Hello people, First of all I wishes everyone have spent a nice summer!
At my work we're trying to use ILSDI to get availability of all items for a biblionumber.
Thanks to Julian we were able to use the GetAvailability method to get availability for one item at a time.
https://catalogue.univ-lyon3.fr/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=510054&id_type=item
According to the documentation it is possible to query this script with biblionumber rather than itemnumber to get availability of all items in one shot.
The late query is then supposed to work but it doesn't. Am I doing something wrong?
Thanks in advance for your wisdom and enlightenment! Best regards,
Arthur Suzuki University Jean Moulin Lyon 3
-- Julian Maurice <julian.maurice@biblibre.com> BibLibre
Hello Julian, Thanks for your reply! I've opened a bug report here : https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17317 Arthur Le 16/09/2016 à 11:08, Julian Maurice a écrit :
Hello Arthur,
You're not doing anything wrong. The GetAvailability implementation in Koha works only when id_type=item. And I believe the parameter 'return_type' is not used at all.
Please open a bug report on https://bugs.koha-community.org/
On 16/09/2016 10:45, SUZUKI Arthur wrote:
Hello people, First of all I wishes everyone have spent a nice summer!
At my work we're trying to use ILSDI to get availability of all items for a biblionumber.
Thanks to Julian we were able to use the GetAvailability method to get availability for one item at a time.
https://catalogue.univ-lyon3.fr/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=510054&id_type=item
According to the documentation it is possible to query this script with biblionumber rather than itemnumber to get availability of all items in one shot.
The late query is then supposed to work but it doesn't. Am I doing something wrong?
Thanks in advance for your wisdom and enlightenment! Best regards,
Arthur Suzuki University Jean Moulin Lyon 3
-- Arthur SUZUKI Service informatique des bibliothèques BIBLIOTHÈQUES UNIVERSITAIRES Université Jean Moulin Lyon 3 6 Cours Albert Thomas - B.P. 8242 – 69355 Lyon Cedex 08 ligne directe : +33 (0)4 78 78 79 16 | http://bu.univ-lyon3.fr L'Université Jean Moulin est membre fondateur de l'Université de Lyon
Yeah, the documentation is wrong, if I recall correctly. Hurray for opening a bug report! David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595
-----Original Message----- From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel- bounces@lists.koha-community.org] On Behalf Of SUZUKI Arthur Sent: Monday, 19 September 2016 6:53 PM To: Julian Maurice <julian.maurice@biblibre.com>; koha-devel@lists.koha- community.org Subject: Re: [Koha-devel] ILSDI - GetAvailability
Hello Julian,
Thanks for your reply!
I've opened a bug report here : https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17317
Arthur
Le 16/09/2016 à 11:08, Julian Maurice a écrit :
Hello Arthur,
You're not doing anything wrong. The GetAvailability implementation in Koha works only when id_type=item. And I believe the parameter 'return_type' is not used at all.
Please open a bug report on https://bugs.koha-community.org/
On 16/09/2016 10:45, SUZUKI Arthur wrote:
Hello people, First of all I wishes everyone have spent a nice summer!
At my work we're trying to use ILSDI to get availability of all items for a biblionumber.
Thanks to Julian we were able to use the GetAvailability method to get availability for one item at a time.
https://catalogue.univ-lyon3.fr/cgi-bin/koha/ilsdi.pl?service=GetAvai lability&id=510054&id_type=item
According to the documentation it is possible to query this script with biblionumber rather than itemnumber to get availability of all items in one shot.
https://catalogue.univ-lyon3.fr/cgi-bin/koha/ilsdi.pl?service=GetAvai lability&id=334726&id_type=bib&return_type=item
The late query is then supposed to work but it doesn't. Am I doing something wrong?
Thanks in advance for your wisdom and enlightenment! Best regards,
Arthur Suzuki University Jean Moulin Lyon 3
-- Arthur SUZUKI Service informatique des bibliothèques BIBLIOTHÈQUES UNIVERSITAIRES Université Jean Moulin Lyon 3 6 Cours Albert Thomas - B.P. 8242 69355 Lyon Cedex 08 ligne directe : +33 (0)4 78 78 79 16 | http://bu.univ-lyon3.fr L'Université Jean Moulin est membre fondateur de l'Université de Lyon
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha- community.org/ bugs : http://bugs.koha-community.org/
Hello David, Hurray, this bug now has a patch which needs reviewing and sign-off! Could you check this out please? I've also added Pascale (French Translation Team) since she tolds us she was interested into this feature development. All the best, Arthur ________________________________________ De : David Cook <dcook@prosentient.com.au> Envoyé : mardi 20 septembre 2016 02:25 À : SUZUKI Arthur; 'Julian Maurice'; koha-devel@lists.koha-community.org Objet : RE: [Koha-devel] ILSDI - GetAvailability Yeah, the documentation is wrong, if I recall correctly. Hurray for opening a bug report! David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595
-----Original Message----- From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel- bounces@lists.koha-community.org] On Behalf Of SUZUKI Arthur Sent: Monday, 19 September 2016 6:53 PM To: Julian Maurice <julian.maurice@biblibre.com>; koha-devel@lists.koha- community.org Subject: Re: [Koha-devel] ILSDI - GetAvailability
Hello Julian,
Thanks for your reply!
I've opened a bug report here : https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17317
Arthur
Le 16/09/2016 à 11:08, Julian Maurice a écrit :
Hello Arthur,
You're not doing anything wrong. The GetAvailability implementation in Koha works only when id_type=item. And I believe the parameter 'return_type' is not used at all.
Please open a bug report on https://bugs.koha-community.org/
On 16/09/2016 10:45, SUZUKI Arthur wrote:
Hello people, First of all I wishes everyone have spent a nice summer!
At my work we're trying to use ILSDI to get availability of all items for a biblionumber.
Thanks to Julian we were able to use the GetAvailability method to get availability for one item at a time.
https://catalogue.univ-lyon3.fr/cgi-bin/koha/ilsdi.pl?service=GetAvai lability&id=510054&id_type=item
According to the documentation it is possible to query this script with biblionumber rather than itemnumber to get availability of all items in one shot.
https://catalogue.univ-lyon3.fr/cgi-bin/koha/ilsdi.pl?service=GetAvai lability&id=334726&id_type=bib&return_type=item
The late query is then supposed to work but it doesn't. Am I doing something wrong?
Thanks in advance for your wisdom and enlightenment! Best regards,
Arthur Suzuki University Jean Moulin Lyon 3
-- Arthur SUZUKI Service informatique des bibliothèques BIBLIOTHÈQUES UNIVERSITAIRES Université Jean Moulin Lyon 3 6 Cours Albert Thomas - B.P. 8242 – 69355 Lyon Cedex 08 ligne directe : +33 (0)4 78 78 79 16 | http://bu.univ-lyon3.fr L'Université Jean Moulin est membre fondateur de l'Université de Lyon
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha- community.org/ bugs : http://bugs.koha-community.org/
2016-09-15 13:03 GMT+02:00 Marcel de Rooy <M.de.Rooy@rijksmuseum.nl>:
Hi devs,
Hi :)
I am seeing more Koha objects that include methods for related records like Koha::Patron, sub image referring to Koha::Patron::Images.
As a second example, on bug 14610 (article requests), a biblio, item, branch and borrower method is added to Koha::ArticleRequest.
These methods should highlight the relationships (fk or missing fk at DB level) between objects. They make sense to me as long as we want to manipulate Koha::Objects. Note that branch should be replaced with library ;)
Obviously, they all follow the same pattern.
Could we generalize that, and keep our code cleaner?
And do we always need the full Koha object? For use in a template we may need only a few columns.
What do you suggest? It's much easier to always get and pass a full object than sometimes a smaller one without knowing exactly what it represents (which methods you can call on it). I don't think it will hurt perfs too much. Of course there will certainly be some rooms for improvements here, it should not be a rule written in stone.
We start passing these objects to the templates now too, making it possible to call methods from the template. It is possible, but do we really want that? More code ends up in the template, maybe harder to find since you now use borrower.method instead of borrower->method etc. Secondly, reading borrower.something might not immediately tell you that something is a method instead of a column.
Yes, I don't think it's a good thing. We should avoid calling methods in template, only accessors. If the method is trivial, I'd say it could be ok.
In other cases we have added a sort of wrapper like Branches.GetName from the Branches plugin.
That was to help us, and that will continue to help us, because the fk don't always exist. If should not hurt perf neither.
I am interested to know if we want to enforce this wrapper approach for instance, or just continue on the current road.
Where and why? :) Usually I don't think TT plugins should be used for such things. In an ideal world, we would manipulate objects in templates and library.branchname would display the library name (in a ideal world it would be library.name...). Or patron.library.branchname, but we should not need a plugin for that: Branches.GetName( patron.branchcode ) Cheers
Thx,
Marcel
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
What do you suggest? It's much easier to always get and pass a full object than sometimes a smaller one without knowing exactly what it represents (which methods you can call on it). I don't think it will hurt perfs too much. Of course there will certainly be some rooms for improvements here, it should not be a rule written in stone.
I agree. Passing the full object to the template will actually affect the performance the least. What we pass is a ref. If we start munging the data before we give it to the template, we are just adding needless overdead.
We start passing these objects to the templates now too, making it possible to call methods from the template. It is possible, but do we really want that? More code ends up in the template, maybe harder to find since you now use borrower.method instead of borrower->method etc. Secondly, reading borrower.something might not immediately tell you that something is a method instead of a column.
Yes, I don't think it's a good thing. We should avoid calling methods in template, only accessors. If the method is trivial, I'd say it could be ok.
I think it's important to consider the template is our View, and logic is fine in the template as long as it's View logic. Formatting a date for example. It shouldn't matter whether we call and object method or a plugin to handle that view logic. In fact, I'd put money on calling an object method being faster and more efficient.
In other cases we have added a sort of wrapper like Branches.GetName from the Branches plugin.
That was to help us, and that will continue to help us, because the fk don't always exist. If should not hurt perf neither.
We can work around the lack for fk constrains within our objects as well!
I am interested to know if we want to enforce this wrapper approach for instance, or just continue on the current road.
Where and why? :)
Usually I don't think TT plugins should be used for such things. In an ideal world, we would manipulate objects in templates and library.branchname would display the library name (in a ideal world it would be library.name...). Or patron.library.branchname, but we should not need a plugin for that: Branches.GetName( patron.branchcode )
I agree with this completely. I think it makes for a much more natural language and a cleaner implementation. Kyle
Agreed with what Kyle has said here. Especially with the part about considering templates as our View and that View logic is OK. I don’t think we should be handling View logic in the controller script if it can be avoided. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595 From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] On Behalf Of Kyle Hall Sent: Wednesday, 21 September 2016 2:37 AM To: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Cc: koha-devel@lists.koha-community.org Subject: Re: [Koha-devel] [Koha::Object] Related records and calling methods from templates What do you suggest? It's much easier to always get and pass a full object than sometimes a smaller one without knowing exactly what it represents (which methods you can call on it). I don't think it will hurt perfs too much. Of course there will certainly be some rooms for improvements here, it should not be a rule written in stone. I agree. Passing the full object to the template will actually affect the performance the least. What we pass is a ref. If we start munging the data before we give it to the template, we are just adding needless overdead.
We start passing these objects to the templates now too, making it possible to call methods from the template. It is possible, but do we really want that? More code ends up in the template, maybe harder to find since you now use borrower.method instead of borrower->method etc. Secondly, reading borrower.something might not immediately tell you that something is a method instead of a column.
Yes, I don't think it's a good thing. We should avoid calling methods in template, only accessors. If the method is trivial, I'd say it could be ok. I think it's important to consider the template is our View, and logic is fine in the template as long as it's View logic. Formatting a date for example. It shouldn't matter whether we call and object method or a plugin to handle that view logic. In fact, I'd put money on calling an object method being faster and more efficient.
In other cases we have added a sort of wrapper like Branches.GetName from the Branches plugin.
That was to help us, and that will continue to help us, because the fk don't always exist. If should not hurt perf neither. We can work around the lack for fk constrains within our objects as well!
I am interested to know if we want to enforce this wrapper approach for instance, or just continue on the current road.
Where and why? :) Usually I don't think TT plugins should be used for such things. In an ideal world, we would manipulate objects in templates and library.branchname would display the library name (in a ideal world it would be library.name...). Or patron.library.branchname, but we should not need a plugin for that: Branches.GetName( patron.branchcode ) I agree with this completely. I think it makes for a much more natural language and a cleaner implementation. Kyle
participants (6)
-
David Cook -
Jonathan Druart -
Julian Maurice -
Kyle Hall -
Marcel de Rooy -
SUZUKI Arthur