A case against some uses of C4::Context->userenv
Hi all, I've been looking at the 'Claims returned' work (awesome!) [1] and I've noticed a pattern we should, in my opinion, start avoiding and reverting. It is quite common to find uses of C4::Context->userenv in the Koha::* namespace, either as a plain requirement (if the borrowernumber is not set we bail out) or just as a fallback. I think it is wrong to expect such environment status in our logic. It complicates things in various ways. [2] It is done this way because: - We come from the CGI era, in which it made perfect sense - Cookie auth is well established on the UI - When migrating from C4:* into Koha:* it is easier to just keep the current behavior and then evolve. I propose we leave them out, and add as a parameter whatever we are expecting to find in the userenv hashref. That way we could use this new libs in the API (which is not cookie-only). Looking forward to hear your opinions [1] https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14697 [2] CLI scripts are using Koha::* libraries, and we then need to 'emulate' that userenv or even add code to detect we don't have a userenv. -- Tomás Cohen Arazi Theke Solutions (http://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F
I generally agree here, I'd rather explicitly pass things like this into objects on instantiation and die if they're not present.. that way we catch cases of missing environments early and as you state, userenv harks back the CGI days and we should generally be moving forward and away from those patterns in my opinion. *Martin Renvoize* <https://www.ptfs-europe.com> Development Team Manager *Phone:* +44 (0) 1483 378728 *Mobile:* +44 (0) 7725 985 636 *Email:* martin.renvoize@ptfs-europe.com *Fax:* +44 (0) 800 756 6384 www.ptfs-europe.com Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30 The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com On Fri, 27 Sep 2019 at 17:42, Tomas Cohen Arazi <tomascohen@gmail.com> wrote:
Hi all, I've been looking at the 'Claims returned' work (awesome!) [1] and I've noticed a pattern we should, in my opinion, start avoiding and reverting.
It is quite common to find uses of C4::Context->userenv in the Koha::* namespace, either as a plain requirement (if the borrowernumber is not set we bail out) or just as a fallback. I think it is wrong to expect such environment status in our logic. It complicates things in various ways. [2]
It is done this way because: - We come from the CGI era, in which it made perfect sense - Cookie auth is well established on the UI - When migrating from C4:* into Koha:* it is easier to just keep the current behavior and then evolve.
I propose we leave them out, and add as a parameter whatever we are expecting to find in the userenv hashref. That way we could use this new libs in the API (which is not cookie-only).
Looking forward to hear your opinions
[1] https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14697 [2] CLI scripts are using Koha::* libraries, and we then need to 'emulate' that userenv or even add code to detect we don't have a userenv.
-- Tomás Cohen Arazi Theke Solutions (http://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://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/
Sounds good to me. But as you know, refactoring a large codebase is a tedious process. ________________________________ Museumstraat 1 Postbus 74888 1070 DN Amsterdam Rijksmuseum.nl Nu te zien: 12x Erwin Olaf Louise Bourgeois in de Rijksmuseumtuinen Operatie Nachtwacht T/m 18 jaar gratis Please think before you print Van: Koha-devel <koha-devel-bounces@lists.koha-community.org> namens Renvoize, Martin <martin.renvoize@ptfs-europe.com> Verzonden: vrijdag 27 september 2019 19:13 Aan: Tomas Cohen Arazi <tomascohen@gmail.com> CC: koha-devel <koha-devel@lists.koha-community.org> Onderwerp: Re: [Koha-devel] A case against some uses of C4::Context->userenv I generally agree here, I'd rather explicitly pass things like this into objects on instantiation and die if they're not present.. that way we catch cases of missing environments early and as you state, userenv harks back the CGI days and we should generally be moving forward and away from those patterns in my opinion. Martin Renvoize [https://www.ptfs-europe.com/wp-content/uploads/2018/10/ptfs-logo-full-email-sig.png]<https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ptfs-europe.com&data=02%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C048e6e0d082f4f8a8ae608d7436e21f6%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C637052012606694897&sdata=qo0dpMjlegzzDhLI4meNlpDyStDamMNZeqx2GeArI%2Fo%3D&reserved=0> Development Team Manager Phone: +44 (0) 1483 378728 Mobile: +44 (0) 7725 985 636 Email: martin.renvoize@ptfs-europe.com<mailto:martin.renvoize@ptfs-europe.com> Fax: +44 (0) 800 756 6384 www.ptfs-europe.com<https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ptfs-europe.com&data=02%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C048e6e0d082f4f8a8ae608d7436e21f6%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C637052012606694897&sdata=qo0dpMjlegzzDhLI4meNlpDyStDamMNZeqx2GeArI%2Fo%3D&reserved=0> Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30 The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com<mailto:info@ptfs-europe.com> On Fri, 27 Sep 2019 at 17:42, Tomas Cohen Arazi <tomascohen@gmail.com<mailto:tomascohen@gmail.com>> wrote: Hi all, I've been looking at the 'Claims returned' work (awesome!) [1] and I've noticed a pattern we should, in my opinion, start avoiding and reverting. It is quite common to find uses of C4::Context->userenv in the Koha::* namespace, either as a plain requirement (if the borrowernumber is not set we bail out) or just as a fallback. I think it is wrong to expect such environment status in our logic. It complicates things in various ways. [2] It is done this way because: - We come from the CGI era, in which it made perfect sense - Cookie auth is well established on the UI - When migrating from C4:* into Koha:* it is easier to just keep the current behavior and then evolve. I propose we leave them out, and add as a parameter whatever we are expecting to find in the userenv hashref. That way we could use this new libs in the API (which is not cookie-only). Looking forward to hear your opinions [1] https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14697<https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.koha-community.org%2Fbugzilla3%2Fshow_bug.cgi%3Fid%3D14697&data=02%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C048e6e0d082f4f8a8ae608d7436e21f6%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C637052012606704891&sdata=pEj0U2nGAHpCTqEQNur9R8XCRcxdggKALtPmohPjd2c%3D&reserved=0> [2] CLI scripts are using Koha::* libraries, and we then need to 'emulate' that userenv or even add code to detect we don't have a userenv. -- Tomás Cohen Arazi Theke Solutions (http://theke.io<https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftheke.io%2F&data=02%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C048e6e0d082f4f8a8ae608d7436e21f6%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C637052012606704891&sdata=rJ7Wq1rK256YA6BTdSophwfAQOYL%2FEd2qrBSu4LdrjM%3D&reserved=0>) ✆ +54 9351 3513384 GPG: B2F3C15F _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org<mailto:Koha-devel@lists.koha-community.org> https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel<https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.koha-community.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fkoha-devel&data=02%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C048e6e0d082f4f8a8ae608d7436e21f6%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C637052012606714888&sdata=MSCs0c6X61QWoRbeQQDmIgYfkt1wMhj2CVDIERhlJ%2BY%3D&reserved=0> website : http://www.koha-community.org/<https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.koha-community.org%2F&data=02%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C048e6e0d082f4f8a8ae608d7436e21f6%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C637052012606714888&sdata=Ysg8%2F7hSlFWQxBXKOEIaVPQehnFiW4tTzs%2FR2613YPY%3D&reserved=0> git : http://git.koha-community.org/<https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fgit.koha-community.org%2F&data=02%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C048e6e0d082f4f8a8ae608d7436e21f6%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C637052012606724882&sdata=P%2Fr5B5nQI7tlAw2CyWeDR9Yq2KU%2Fmvd8UMmP7D5jbU4%3D&reserved=0> bugs : http://bugs.koha-community.org/<https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fbugs.koha-community.org%2F&data=02%7C01%7Cm.de.rooy%40rijksmuseum.nl%7C048e6e0d082f4f8a8ae608d7436e21f6%7C635b05eb66c748e1a94fb4b05a1b058b%7C0%7C0%7C637052012606724882&sdata=hH4KAhh61qM6lGWmuqvQfhPhiAEoJIQBmVieS%2BrfSR8%3D&reserved=0>
Totally agree too. Also makes unit testing a million times easier. 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 <koha-devel-bounces@lists.koha-community.org> On Behalf Of Renvoize, Martin Sent: Saturday, 28 September 2019 3:14 AM To: Tomas Cohen Arazi <tomascohen@gmail.com> Cc: koha-devel <koha-devel@lists.koha-community.org> Subject: Re: [Koha-devel] A case against some uses of C4::Context->userenv I generally agree here, I'd rather explicitly pass things like this into objects on instantiation and die if they're not present.. that way we catch cases of missing environments early and as you state, userenv harks back the CGI days and we should generally be moving forward and away from those patterns in my opinion. Martin Renvoize <https://www.ptfs-europe.com/> Development Team Manager Phone: +44 (0) 1483 378728 Mobile: +44 (0) 7725 985 636 Email: <mailto:martin.renvoize@ptfs-europe.com> martin.renvoize@ptfs-europe.com Fax: +44 (0) 800 756 6384 <https://www.ptfs-europe.com> www.ptfs-europe.com Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30 The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com <mailto:info@ptfs-europe.com> On Fri, 27 Sep 2019 at 17:42, Tomas Cohen Arazi <tomascohen@gmail.com <mailto:tomascohen@gmail.com> > wrote: Hi all, I've been looking at the 'Claims returned' work (awesome!) [1] and I've noticed a pattern we should, in my opinion, start avoiding and reverting. It is quite common to find uses of C4::Context->userenv in the Koha::* namespace, either as a plain requirement (if the borrowernumber is not set we bail out) or just as a fallback. I think it is wrong to expect such environment status in our logic. It complicates things in various ways. [2] It is done this way because: - We come from the CGI era, in which it made perfect sense - Cookie auth is well established on the UI - When migrating from C4:* into Koha:* it is easier to just keep the current behavior and then evolve. I propose we leave them out, and add as a parameter whatever we are expecting to find in the userenv hashref. That way we could use this new libs in the API (which is not cookie-only). Looking forward to hear your opinions [1] https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14697 [2] CLI scripts are using Koha::* libraries, and we then need to 'emulate' that userenv or even add code to detect we don't have a userenv. -- Tomás Cohen Arazi Theke Solutions (http://theke.io <http://theke.io/> ) ✆ +54 9351 3513384 GPG: B2F3C15F _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org <mailto:Koha-devel@lists.koha-community.org> https://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 about the 'Search My Library First' and 'Independent Branches' settings? Those are dependent right now on reading the users branch from userenv. I'm not familiar enough with the code base to know where else those settings would get the users branch; is it kept in a cookie? On Sun, Sep 29, 2019 at 6:03 PM <dcook@prosentient.com.au> wrote:
Totally agree too. Also makes unit testing a million times easier.
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 <koha-devel-bounces@lists.koha-community.org> *On Behalf Of *Renvoize, Martin *Sent:* Saturday, 28 September 2019 3:14 AM *To:* Tomas Cohen Arazi <tomascohen@gmail.com> *Cc:* koha-devel <koha-devel@lists.koha-community.org> *Subject:* Re: [Koha-devel] A case against some uses of C4::Context->userenv
I generally agree here, I'd rather explicitly pass things like this into objects on instantiation and die if they're not present.. that way we catch cases of missing environments early and as you state, userenv harks back the CGI days and we should generally be moving forward and away from those patterns in my opinion.
*Martin Renvoize*
<https://www.ptfs-europe.com/>
Development Team Manager
*Phone:* +44 (0) 1483 378728
*Mobile:* +44 (0) 7725 985 636
*Email:* martin.renvoize@ptfs-europe.com
*Fax:* +44 (0) 800 756 6384
www.ptfs-europe.com
Registered in the United Kingdom No. 06416372 VAT Reg No. 925 7211 30
The information contained in this email message may be privileged, confidential and protected from disclosure. If you are not the intended recipient, any dissemination, distribution or copying is strictly prohibited. If you think that you have received this email message in error, please email the sender at info@ptfs-europe.com
On Fri, 27 Sep 2019 at 17:42, Tomas Cohen Arazi <tomascohen@gmail.com> wrote:
Hi all, I've been looking at the 'Claims returned' work (awesome!) [1] and I've noticed a pattern we should, in my opinion, start avoiding and reverting.
It is quite common to find uses of C4::Context->userenv in the Koha::* namespace, either as a plain requirement (if the borrowernumber is not set we bail out) or just as a fallback. I think it is wrong to expect such environment status in our logic. It complicates things in various ways. [2]
It is done this way because:
- We come from the CGI era, in which it made perfect sense
- Cookie auth is well established on the UI
- When migrating from C4:* into Koha:* it is easier to just keep the current behavior and then evolve.
I propose we leave them out, and add as a parameter whatever we are expecting to find in the userenv hashref. That way we could use this new libs in the API (which is not cookie-only).
Looking forward to hear your opinions
[1] https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14697
[2] CLI scripts are using Koha::* libraries, and we then need to 'emulate' that userenv or even add code to detect we don't have a userenv.
--
Tomás Cohen Arazi
Theke Solutions (http://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://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/
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org https://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/
-- Michael Hafen Washington County School District Technology Department Systems Analyst
participants (5)
-
dcook＠prosentient.com.au -
Marcel de Rooy -
Michael Hafen -
Renvoize, Martin -
Tomas Cohen Arazi