[Send Test Mail] Use json MimeType, send data via POST method and catch json response errors #9685

andrepereiradasilva · 2016-03-31T11:48:25Z

Summary of Changes

Adds the correct HTTP header mimetype for json in sendmail test response and changes the HTTP method from GET to POST so the smtp server details (host, port, user, password) doesn't appear in the server/proxies log files.
Catch HTTP or json failure errors and display error message when that happens.

Testing Instructions

Correct HTTP header mimetype and HTTP method

Go to global config in the admin panel
Open the console in the chrome (F12)
Select "Network" tab, "XHR" separator in Chrome console
Click the "Send Test Mail" button. You will see a new URI in the Chrome console, click on it. Check in the "Response headers" the "Content type" HTTP header is content-type:text/html; charset=UTF-8 and the HTTP method is GET
Apply patch
Repeat steps 1 (clear javascript cache Ctrl+F5) to 4, you will see in the "Response headers" the content type HTTP header is content-type:application/json; charset=utf-8 and the HTTP method is POST

Catch HTTP or json failure errors

With the patch previously applied.
Add in https://github.com/andrepereiradasilva/joomla-cms/blob/patch-6/administrator/components/com_config/controller/application/sendtestmail.php#L42

trigger_error('aaa', E_USER_WARNING);

Go to global config in the admin panel and send a test mail
You will get an error message (without this patch you get no message - javascript error).

brianteeman · 2016-03-31T11:49:47Z

is #5238 related?

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

andrepereiradasilva · 2016-03-31T11:56:17Z

i think that's unrelated to this issue.

brianteeman · 2016-03-31T11:59:43Z

ok - fair enough ;)

mbabker · 2016-03-31T13:22:57Z

Being completely honest with you, the last time I tried working with JDocumentJson it just felt completely broken (like it doesn't even JSON encode the data buffer for starters). Also seems pretty stupid if it forces downloads (which it didn't in that case where I was using it in Chrome, and this was last month I was working with it).

BTW, this is just me being nit-picky for the sake of being nit-picky, but headers should be consistently set and sent through the application like https://github.com/joomla/joomla-cms/blob/staging/components/com_finder/controllers/suggestions.json.php does (the day components ever get real automated tests written for them using PHP's header() function will just make things act up).

andrepereiradasilva · 2016-03-31T14:39:15Z

ok just changed jquery javascript to handle the json object data without the need to use parseJSON (it's already fetched as a json object).

ready for testing.

NOTE: i also changed the HTTP method to POST so the smtp server password doesn't appear in the server/proxies log files.

andrepereiradasilva · 2016-03-31T19:32:46Z

@piotrmocko please try now i have added some ajax fail treatments in this PR.

Don't forgot to clear the browser cache.

andrepereiradasilva · 2016-03-31T19:53:23Z

no it also works on malformed json , just tested.

to trown a warning i added

trigger_error('aaa', E_USER_WARNING);

in https://github.com/joomla/joomla-cms/pull/9685/files#diff-f0600c10281461980c2770b1951d453eR42

Result:

HTTP status 200. Page:

Warning: aaa in /path/to/joomla-staging/administrator/components/com_config/controller/application/sendtestmail.php on line 42
{"success":true,"message":null,"messages":{"success":["The email was sent successfully to <strong>test@test.com<\/strong> using <strong>SMTP<\/strong>. You should check that you've received the test email."]},"data":true}

piotrmocko · 2016-03-31T19:55:15Z

I will test it again

andrepereiradasilva · 2016-03-31T19:55:33Z

ok, don't forget to clear the browser cache

richard67 · 2016-03-31T20:05:51Z

I have tested this item ✅ successfully on 6d22f55

Tested as described in the instructions, but with Firefox/Firebug and not Chrome console 👅

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

piotrmocko · 2016-03-31T20:10:31Z

Finally it helped :) It looks like it has changed in jQuery in last years.
You may consider to display in the message:
(jqXHR.responseText.indexOf('<html') === -1 ? jqXHR.responseText : error)

joomla-cms-bot · 2016-03-31T21:28:34Z

This PR has received new commits.

CC: @richard67

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

joomla-cms-bot · 2016-03-31T21:28:50Z

This PR has received new commits.

CC: @richard67

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

andrepereiradasilva · 2016-03-31T21:29:37Z

@richard67 @piotrmocko
Added better error reporting,Test instructions updated. Please test now.

richard67 · 2016-03-31T21:31:30Z

If you explain me what "mimified" means 😄

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

andrepereiradasilva · 2016-03-31T21:33:03Z

If you explain me what "mimified" means 😄

copy pasting ... errors... and lack of sleep

richard67 · 2016-03-31T21:34:34Z

Ah, I thought maybe it has to do with a girl named "Mimi"

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

joomla-cms-bot · 2016-04-07T09:51:06Z

This PR has received new commits.

CC: @mikeveeckmans, @richard67

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

…ponse

joomla-cms-bot · 2016-04-07T09:52:17Z

This PR has received new commits.

CC: @mikeveeckmans, @richard67

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

joomla-cms-bot · 2016-04-07T09:53:02Z

This PR has received new commits.

CC: @mikeveeckmans, @richard67

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

andrepereiradasilva · 2016-04-07T09:54:03Z

ok. language variables added.

brianteeman · 2016-04-07T09:55:38Z

administrator/language/en-GB/en-GB.com_config.ini

@@ -229,6 +229,11 @@ COM_CONFIG_SAVE_SUCCESS="Configuration successfully saved."
 COM_CONFIG_SENDMAIL_ACTION_BUTTON="Send Test Mail"
 COM_CONFIG_SENDMAIL_BODY="This is a test mail sent using "_QQ_"%s"_QQ_". If you receive it, then your email settings are correct!"
 COM_CONFIG_SENDMAIL_ERROR="Test mail could not be sent."
+COM_CONFIG_SENDMAIL_JS_ERROR_CONNECTION_ABORT="A connection abort as occured while fetching the JSON data."


all corrected

brianteeman · 2016-04-07T09:57:26Z

Removed the RTC label and made some comments on the language strings

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

joomla-cms-bot · 2016-04-07T09:59:36Z

This PR has received new commits.

CC: @mikeveeckmans, @richard67

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

andrepereiradasilva · 2016-04-07T10:03:17Z

The latest change was adding javascript language vars as @wilsonge requested.

@mikeveeckmans, @richard67, @brianteeman can you retest to get this RTC again?

wilsonge · 2016-04-07T10:20:58Z

This looks MUCH better - thankyou!

richard67 · 2016-04-07T11:37:12Z

I have tested this item ✅ successfully on f56e8c8

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

anibalsanchez · 2016-04-08T08:39:36Z

I have tested this item ✅ successfully on f56e8c8

Test OK

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

brianteeman · 2016-04-08T08:54:17Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

anibalsanchez · 2016-04-08T08:57:39Z

I have updated #9768 with the code from this issue.

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9685.}

use correct mimetype for json

8a8b535

joomla-cms-bot added the PR-staging label Mar 31, 2016

andrepereiradasilva added 3 commits March 31, 2016 15:19

follow recomendations. thanks @mbabker

dff1296

handle json data in js

814ed4c

handle json data in js (mimified)

962998d

andrepereiradasilva changed the title ~~Use correct mimetype in HTTP headers for the json result in send mail test~~ [Send Test Mail] Use json MimeType and send data via POST method Mar 31, 2016

andrepereiradasilva mentioned this pull request Mar 31, 2016

Handle PHP warnings in send test email JSON response #9676

Closed

andrepereiradasilva added 3 commits March 31, 2016 20:28

process HTTP or json ajax failures too

c304fd8

process HTTP or json ajax failures too (mimified)

0f4958f

remove extra space and extra identing

6d22f55

andrepereiradasilva changed the title ~~[Send Test Mail] Use json MimeType and send data via POST method~~ [Send Test Mail] Use json MimeType, send data via POST method and catch json response errors Mar 31, 2016

better error reporting

ff8da28

better error reporting (mimified)

69bb1de

load text string from vars and do html entitiy encode to the json res…

6dedd7b

…ponse

same changes in the compressed js version

d739166

brianteeman reviewed Apr 7, 2016
View reviewed changes

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 7, 2016

correct language - thanks again @brianteeman

f56e8c8

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 8, 2016

anibalsanchez mentioned this pull request Apr 8, 2016

Send Test Mail button - Ajax call has always smtpauth = 1 #9768

Closed

rdeutz merged commit d5a0c4b into joomla:staging Apr 12, 2016

andrepereiradasilva deleted the patch-6 branch April 12, 2016 21:12

rdeutz modified the milestones: Joomla 3.5.2, Joomla! 3.6.0 May 1, 2016

brianteeman removed the RTC This Pull Request is Ready To Commit label May 11, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Send Test Mail] Use json MimeType, send data via POST method and catch json response errors #9685

[Send Test Mail] Use json MimeType, send data via POST method and catch json response errors #9685

andrepereiradasilva commented Mar 31, 2016

brianteeman commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

brianteeman commented Mar 31, 2016 via email

mbabker commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

piotrmocko commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

richard67 commented Mar 31, 2016

piotrmocko commented Mar 31, 2016

joomla-cms-bot commented Mar 31, 2016

joomla-cms-bot commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

richard67 commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

richard67 commented Mar 31, 2016

joomla-cms-bot commented Apr 7, 2016

joomla-cms-bot commented Apr 7, 2016

joomla-cms-bot commented Apr 7, 2016

andrepereiradasilva commented Apr 7, 2016

brianteeman Apr 7, 2016

andrepereiradasilva Apr 7, 2016

brianteeman commented Apr 7, 2016

joomla-cms-bot commented Apr 7, 2016

andrepereiradasilva commented Apr 7, 2016

wilsonge commented Apr 7, 2016

richard67 commented Apr 7, 2016

anibalsanchez commented Apr 8, 2016

brianteeman commented Apr 8, 2016

anibalsanchez commented Apr 8, 2016

[Send Test Mail] Use json MimeType, send data via POST method and catch json response errors #9685

[Send Test Mail] Use json MimeType, send data via POST method and catch json response errors #9685

Conversation

andrepereiradasilva commented Mar 31, 2016

Summary of Changes

Testing Instructions

Correct HTTP header mimetype and HTTP method

Catch HTTP or json failure errors

brianteeman commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

brianteeman commented Mar 31, 2016 via email

mbabker commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

piotrmocko commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

richard67 commented Mar 31, 2016

piotrmocko commented Mar 31, 2016

joomla-cms-bot commented Mar 31, 2016

joomla-cms-bot commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

richard67 commented Mar 31, 2016

andrepereiradasilva commented Mar 31, 2016

richard67 commented Mar 31, 2016

joomla-cms-bot commented Apr 7, 2016

joomla-cms-bot commented Apr 7, 2016

joomla-cms-bot commented Apr 7, 2016

andrepereiradasilva commented Apr 7, 2016

brianteeman Apr 7, 2016

Choose a reason for hiding this comment

andrepereiradasilva Apr 7, 2016

Choose a reason for hiding this comment

brianteeman commented Apr 7, 2016

joomla-cms-bot commented Apr 7, 2016

andrepereiradasilva commented Apr 7, 2016

wilsonge commented Apr 7, 2016

richard67 commented Apr 7, 2016

anibalsanchez commented Apr 8, 2016

brianteeman commented Apr 8, 2016

anibalsanchez commented Apr 8, 2016