[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382
--- Comment #7 from David Cook ---
(In reply to Jonathan Druart from comment #6)
> 1. carp is not needed indeed.
> 2. var you pass to the template need to be useful, otherwise what is the
> purpose?
As I said above, I was just copying the existing patterns of these scripts.
> There is plenty of places where we push @messages, { error => $error_code };
> then loop on the messages in the template.
Not for these scripts we don't.
> That's mandatory here IMO.
Rewriting these scripts and templates seems out of scope to me, but if that's
mandatory that's OK, as they're very reasonable changes to make. It's more work
than I'm willing to do at this point, so I'll put it on my TODO list for the
future.
This seemed like a faster/easier win than validating the input to Koha::Email
for all scripts, but if that's not the case I'll just maintain the changes
locally for now, and get back to it when I have more time.
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382
Jonathan Druart changed:
What|Removed |Added
CC||jonathan.dru...@bugs.koha-c
||ommunity.org
--- Comment #6 from Jonathan Druart
---
1. carp is not needed indeed.
2. var you pass to the template need to be useful, otherwise what is the
purpose?
There is plenty of places where we push @messages, { error => $error_code };
then loop on the messages in the template.
That's mandatory here IMO.
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382 Katrin Fischer changed: What|Removed |Added Status|Signed Off |Failed QA --- Comment #4 from Katrin Fischer --- Hi David, while this works, there are a few catches: 1) I feel like the unconditional carp here is not necessary. We usually don't ouput erros like this in the logs. 2) You pass parameters to the template - why not have a specific error message? At the moment we always have: There was an error sending the cart. For the user that could read like there is a server issue, not an issue with the entered information. 3) I think we should also add additional client side validation like we have on the email fields on the staff client or in the patron details form in OPAC. This way we can give even more specific feedback in a standard way. Failing for 1, but it would be great if you could consider 2 and 3 as well. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382 David Roberts changed: What|Removed |Added Attachment #104389|0 |1 is obsolete|| --- Comment #3 from David Roberts --- Created attachment 104626 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104626=edit Bug 25382: validate email addresses for opac-sendbasket.pl and opac-sendshelf.pl This patch adds email validation to these two OPAC scripts using Email::Valid Test plan: 0) Don't apply patch yet 1) Send a cart to "b...@blah.blah b...@blah.blah" 2) Note the cart says it has sent to both email addresses 3) Apply patch 4) Send a cart to "b...@blah.blah b...@blah.blah" 5) Note the cart says it has an error 6) Send a cart to "b...@blah.blah" 7) Note the cart says email was sent successfully 8) Repeat above process for OPAC List functionality Signed-off-by: David Roberts -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382 David Roberts changed: What|Removed |Added CC||david.roberts@ptfs-europe.c ||om Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382 --- Comment #5 from David Cook --- (In reply to Katrin Fischer from comment #4) > Hi David, > > while this works, there are a few catches: > > 1) I feel like the unconditional carp here is not necessary. We usually > don't ouput erros like this in the logs. > I was just copying the style used elsewhere in those same scripts. Happy not to include it if you don't want it. > 2) You pass parameters to the template - why not have a specific error > message? At the moment we always have: There was an error sending the cart. > For the user that could read like there is a server issue, not an issue with > the entered information. > I was just copying the style used elsewhere in those same scripts. I didn't think this was a good opportunity to change the error handling for the whole script. > 3) I think we should also add additional client side validation like we have > on the email fields on the staff client or in the patron details form in > OPAC. This way we can give even more specific feedback in a standard way. > That's a good idea. My patch is mostly to address security issues, but a better user experience is a great idea. > Failing for 1, but it would be great if you could consider 2 and 3 as well. I'm not sure these points deserve the patch to be failed, but I'd be willing to remove the carp to get it moving again. If points 2 and 3 are necessary, I'll just leave this patch for now. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382 --- Comment #2 from David Cook --- Instead of "b...@blah.blah b...@blah.blah" you could do "sagsadsadg b...@blah.blah" for your bad email input too. That might make it more obvious. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382 David Cook changed: What|Removed |Added See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=25371 Assignee|oleon...@myacpl.org |dc...@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382 --- Comment #1 from David Cook --- Created attachment 104389 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104389=edit Bug 25382: validate email addresses for opac-sendbasket.pl and opac-sendshelf.pl This patch adds email validation to these two OPAC scripts using Email::Valid Test plan: 0) Don't apply patch yet 1) Send a cart to "b...@blah.blah b...@blah.blah" 2) Note the cart says it has sent to both email addresses 3) Apply patch 4) Send a cart to "b...@blah.blah b...@blah.blah" 5) Note the cart says it has an error 6) Send a cart to "b...@blah.blah" 7) Note the cart says email was sent successfully 8) Repeat above process for OPAC List functionality -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 25382] opac-sendbasket.pl and opac-sendshelf.pl don't validate email addresses
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25382 David Cook changed: What|Removed |Added Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
