Joomla! Issue Tracker | Joomla! CMS #9685 - [Send Test Mail] Use json MimeType, send data via POST method and catch json response errors

? ?

Fixed in Code Base
12 Apr 2016
Medium
Build: staging
# 9685
Diff
andrepereiradasilva:patch-6

Success

Success continuous-integration/travis-ci/pr The Travis CI build passed Details
Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details

User tests: Successful: Unsuccessful:

andrepereiradasilva
31 Mar 2016

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).

8a8b535 31 Mar 2016

use correct mimetype for json

andrepereiradasilva - open - 31 Mar 2016

andrepereiradasilva - change - 31 Mar 2016

Status

New

⇒

Pending

joomla-cms-bot - change - 31 Mar 2016

Labels

Added: ?

brianteeman - comment - 31 Mar 2016

is #5238 related?

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

andrepereiradasilva - comment - 31 Mar 2016

i think that's unrelated to this issue.

brianteeman - comment - 31 Mar 2016

ok - fair enough ;)

mbabker - comment - 31 Mar 2016

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).

dff1296 31 Mar 2016

follow recomendations. thanks @mbabker

814ed4c 31 Mar 2016

handle json data in js

962998d 31 Mar 2016

handle json data in js (mimified)

andrepereiradasilva - comment - 31 Mar 2016

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 - change - 31 Mar 2016

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

c304fd8 31 Mar 2016

process HTTP or json ajax failures too

0f4958f 31 Mar 2016

process HTTP or json ajax failures too (mimified)

6d22f55 31 Mar 2016

remove extra space and extra identing

andrepereiradasilva - comment - 31 Mar 2016

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

Don't forgot to clear the browser cache.

andrepereiradasilva - comment - 31 Mar 2016

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 - comment - 31 Mar 2016

I will test it again

andrepereiradasilva - comment - 31 Mar 2016

ok, don't forget to clear the browser cache

andrepereiradasilva - change - 31 Mar 2016

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

richard67 - test_item - 31 Mar 2016 - Tested successfully

richard67 - comment - 31 Mar 2016

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 - comment - 31 Mar 2016

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)

ff8da28 31 Mar 2016

better error reporting

joomla-cms-bot - comment - 31 Mar 2016

This PR has received new commits.

CC: @richard67

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

69bb1de 31 Mar 2016

better error reporting (mimified)

joomla-cms-bot - comment - 31 Mar 2016

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 - comment - 31 Mar 2016

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

richard67 - comment - 31 Mar 2016

If you explain me what "mimified" means

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

andrepereiradasilva - comment - 31 Mar 2016

If you explain me what "mimified" means

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

richard67 - comment - 31 Mar 2016

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.}

richard67 - comment - 31 Mar 2016

@andrepereiradasilva The error message I get in the test looks as follows:

A parse error as occured while processing the following JSON data:
Warning: aaa in /homepages/38/d391032706/htdocs/test7/administrator/components/com_config/controller/application/sendtestmail.php on line 43
{"success":true,"message":null,"messages":{"success":["The email was sent successfully to admin@richard-fath.de<\/strong> using PHP Mail<\/strong>. You should check that you've received the test email."]},"data":true}

Was that intentional, that the json part is in the message?

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

brianteeman - comment - 31 Mar 2016

Who is mimi and who gave her a massage?

andrepereiradasilva - comment - 31 Mar 2016

@richard67 yes it's intentional to show why the parse error.

andrepereiradasilva - comment - 31 Mar 2016

Who is mimi and who gave her a massage?

The question is: who got a massage from mimi ;)

richard67 - test_item - 31 Mar 2016 - Tested successfully

richard67 - comment - 31 Mar 2016

I have tested this item successfully on 69bb1de

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

andrepereiradasilva - comment - 2 Apr 2016

@piotrmocko can you test this so it can be RTC for 3.5.2 too?

brianteeman - change - 2 Apr 2016

Category

⇒

JavaScript

piotrmocko - comment - 4 Apr 2016

I will test it today
02.04.2016 13:37 "andrepereiradasilva" notifications@github.com
napisał(a):

@piotrmocko https://github.com/piotrmocko can you test this so it can
be RTC for 3.5.2 too?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9685 (comment)

mikeveeckmans - test_item - 5 Apr 2016 - Tested successfully

mikeveeckmans - comment - 5 Apr 2016

I have tested this item successfully on 69bb1de

TEST OK

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

piotrmocko - comment - 5 Apr 2016

I have tested this item successfully on 69bb1de

TEST OK

brianteeman - change - 5 Apr 2016

Status

Pending

⇒

Ready to Commit

brianteeman - comment - 5 Apr 2016

RTC - thanks

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

joomla-cms-bot - change - 5 Apr 2016

Labels

Added: ?

brianteeman - change - 5 Apr 2016

Milestone

Added:

wilsonge - comment - 6 Apr 2016

Sorry to be a pain as this is already RTC but is there a reason why these javascript errors can't be made lang strings?

andrepereiradasilva - comment - 6 Apr 2016

is it possible to merge this and them i will take care of the messages in a new PR?

Just saying this because @anibalsanchez wants to make a PR too (see anibalsanchez#1) to correct a bug.
But since this has a minified js it gets conflicts and we have to redo the js.

andrepereiradasilva - comment - 7 Apr 2016

ok, will made the changes needed.

cf3b304 7 Apr 2016

add new text strings for sendmail js

joomla-cms-bot - comment - 7 Apr 2016

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 - change - 7 Apr 2016

Labels

Added: ?

4d7c329 7 Apr 2016