Failure

User tests: Successful: Unsuccessful:

avatar rcarey
rcarey
22 Feb 2013

Added feature that allows multiple email addresses to be added as recipients. The additional recipients are specified per contact form through a new parameter added to Email Options. This feature has been listed at the Joomlacode tracker: #30151.

avatar rcarey rcarey - open - 22 Feb 2013
avatar nicksavov
nicksavov - comment - 23 Mar 2013

Thanks for coding this, Randy!

For anyone interested, here's the direct link to the tracker item on JoomlaCode:
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=30151

avatar rob-van-baal
rob-van-baal - comment - 4 Jun 2014

Installed Patchmanager on a 3.3.0 installation and installed patch #734. In the General Settings the "Allways copy to" and "Additional Recipients" appear, but after filling them in, the input isn't stored, no error is given and probably since they aren't stored, no additional emails are send. Strange!

Tried modifying the contact itself. Added two extra emailadresses. First an error was issues because I used a space after the comma... Seperator recognition should be made more user friendly here! Then without the space, and both values were stored now! Victory! Could that be the problem also of my first strange behaviour???

So I went back to the General settings and tried to enter two entries at Additional Recipients again with space: No error was given and input field was cleared. That's a bug. Without space: Succes! Input was saved!

Tried to enter these two emailadresses in the (general) Allways copy to field with space: No error was given and extra space was taken out by software and value was stored! A miracle! Without space: Value was stored.

So the validation behaviour is far from robust... on both the General part and the individual contact part.

Then about sending mail: That's a part I cannot test quite well from my Mac, since mailprovidors keep sending back annoying messages that I'm sending mail from localhost... To test that part, I should have to create a testserver at my providers server, but that's too much of a hassle right now.

But at least, someone could pick up the findings of the not so robust input validation. When a new fix is available I might do some additional testing.

Cheers!

avatar rcarey
rcarey - comment - 4 Jun 2014

Rob, Those changes were from nearly 16 months ago. At that time I thoroughly tested them and they worked. But no one attempted to test them until months lathe master branch changed enough to undermine my changes - and the tests failed. I

avatar rob-van-baal
rob-van-baal - comment - 4 Jun 2014

Well... this RfC should be brought back to life, since its gives extra functionality which a lot of users will appriciate!

avatar rcarey
rcarey - comment - 4 Jun 2014

I'm willing to work with someone who wants to add these changes. I've worked through the issues. The solution was not complex, but it did require that I touch a lot of files as com_contact has several views that needed to be adjusted. So if anyone wants to run with this, feel free to ask me for how I solved it and I will collaborate and even be a tester to help get it approved.

avatar rob-van-baal
rob-van-baal - comment - 4 Jun 2014

How a about Thomas Hunziker... He led me to this request! I'll send him a mail. I can only help you guys out here doing testwork. I'm not able to write any code (ok, maybe some simple translation work).

avatar Bakual
Bakual - comment - 4 Jun 2014

@rob-van-baal No need to send me an email, I get notifications here :smile:
@rcarey From what I see this PR is a bit dated and thus against master. It would be great if you could update it and redo against staging branch.
Chad also mentioned in the JoomlaCode tracker that he got some errors. Those would have to be solved as well.
If you need help with that, feel free to ping me on Skype or by email.

avatar rcarey
rcarey - comment - 4 Jun 2014

@Bakual. I thoroughly tested it through every view it touched and scenario I could imagine, and there were no errors. Chad tested this PR months after I submitted it. By that time the master had changed, and I suspect that was the cause of the errors. To be candid, that experience deflated my incentive to continue this pull request in specific and future ones in general. But since I'm hearing some request for this feature, I'll reconsider.

avatar Bakual
Bakual - comment - 5 Jun 2014

@rcarey I know that it can be frustrating sometimes to get PRs accepted and understand your pain. I have several PRs waiting to be tested myself. We need far more people testing things.
On the other hand it's also frustrating for testers when they spend time to test a proposal only to find out that the branch is outdated and contains errors.

I'm interested in feedback how that process could be improved. Feel free to send me an email :smile:

avatar phproberto phproberto - reference | - 11 Jun 14
avatar phproberto
phproberto - comment - 11 Jun 2014

Hey @rcarey I totally understand your frustration. I've tried to help creating a new PR against the staging branch here #3757

Let's try to get it merged. I'm closing this in favour of the new PR.

Thanks for contributing!

avatar phproberto phproberto - close - 11 Jun 2014

Add a Comment

Login with GitHub to post a comment