RTC Unit/System Tests Language Change PR-5.2-dev Pending

User tests: Successful: 0 Unsuccessful: 0

avatar brianteeman
brianteeman
27 Jan 2025

When mail templates are set to HTML it is not possible to SEE the senders email address as it sees the wrapping < > as html markup

This PR simply removes the < > from the string so that the email address is displayed.

The alternative would be to create a more complicated option that uses a different language string for the HTML version of the mail template

Pull Request for Issue #44791 .

Testing Instructions

Set the mail templates to use html and then send a test mail using a contact form

Actual result BEFORE applying this Pull Request

image

Expected result AFTER applying this Pull Request

image

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar brianteeman brianteeman - open - 27 Jan 2025
avatar brianteeman brianteeman - change - 27 Jan 2025
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 27 Jan 2025
Category Language & Strings
avatar coolcat-creations coolcat-creations - test_item - 27 Jan 2025 - Tested successfully
avatar coolcat-creations
coolcat-creations - comment - 27 Jan 2025

I have tested this item ✅ successfully on 592eaf9

Thank you @brianteeman for this quick fix. I tested it and now it works as expected.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44792.
avatar drmenzelit drmenzelit - test_item - 27 Jan 2025 - Tested successfully
avatar drmenzelit
drmenzelit - comment - 27 Jan 2025

I have tested this item ✅ successfully on 592eaf9


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44792.

avatar drmenzelit drmenzelit - change - 27 Jan 2025
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 27 Jan 2025

System tests are failing for both MySQL and PostgreSQL, and it seems related:

  Running:  api/com_contact/Contacts.cy.js                                               (93 of 129)

  Test that contacts API endpoint
    ✓ can deliver a list of contacts (887ms)
    ✓ can deliver a single contact (754ms)
    ✓ can create a contact (503ms)
    ✓ can update a contact (1177ms)
    ✓ can delete a contact (379ms)

    1) can submit a contact form

  5 passing (9s)
  1 failing

  1) Test that contacts API endpoint
       can submit a contact form:
     AssertionError: Timed out retrying after 4000ms: expected 'This is an enquiry email via http://localhost/cmysql/api/ from:\njane doe admin@example.com\n\nautomated test message\n' to contain 'jane doe <admin@example.com>'
      at  (webpack://joomla/./tests/System/integration/api/com_contact/Contacts.cy.js:65:29)

It seems the system tests need some adjustment, too.

avatar richard67 richard67 - change - 27 Jan 2025
Status Ready to Commit Pending
Labels Added: Language Change RTC PR-5.2-dev
avatar richard67
richard67 - comment - 27 Jan 2025

Back to pending as system tests need to be adjusted to the changes in this PR.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44792.

avatar richard67
richard67 - comment - 27 Jan 2025

@LadySolveig Could you help here with adjusting the system tests (see my previous comment)? Not needed anymore, Brian has done that.

avatar alikon
alikon - comment - 27 Jan 2025

i'm unable to replicate the issue ##44791 without the pr

image

avatar brianteeman brianteeman - change - 27 Jan 2025
Labels Removed: RTC
avatar joomla-cms-bot joomla-cms-bot - change - 27 Jan 2025
Category Language & Strings Language & Strings JavaScript Unit Tests
avatar brianteeman
brianteeman - comment - 27 Jan 2025

System tests updated

avatar richard67 richard67 - change - 27 Jan 2025
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 27 Jan 2025

RTC as previous human tests are still valid, only system tests were adjusted meanwhile.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44792.

avatar QuyTon QuyTon - change - 29 Jan 2025
Labels Added: RTC Unit/System Tests
avatar Hackwar
Hackwar - comment - 2 Feb 2025

First of all thank you for working on this. Unfortunately I'm a bit unhappy with the approach you took, because with your approach we are relying on the transformation of the non-HTML text into HTML. At the same time we do have the possibility to create entirely different templates for HTML mails than for text mails.

What I'm trying to say here is, that I'd rather would want to see us introducing a new translation string and insert that into the htmlbody column of the template instead of modifying the existing templates. Could you please change this PR in that manner? Thank you! 😄

avatar brianteeman
brianteeman - comment - 2 Feb 2025

with your approach we are relying on the transformation of the non-HTML text into HTML.

what transformation?

avatar Hackwar
Hackwar - comment - 2 Feb 2025

If the HTML body is empty, it tries to convert the plain text content into an HTML content. See line 328 of /libraries/src/Mail/MailTemplate.php

avatar brianteeman
brianteeman - comment - 2 Feb 2025

yes but there is nothing special happening. the email address is just plain text now and its all it would be in an htmlbody template. So your reasoning is lost on me

avatar Hackwar
Hackwar - comment - 2 Feb 2025

When the email adress is wrapped in those brackets in a plaintext mail, at least in my program the content of the brackets is displayed as a link. With your change that is gone and I just get a plain text. There is a reason why we have the html body field and I would rather have a proper template in there than to convert the current plain text template into HTML, even if it is a simple conversion right now.

avatar brianteeman
brianteeman - comment - 2 Feb 2025

They're not a link in my email client.

Doing it your way has a lot of b/c issues for anyone that has customised their email and that's not something easily addressed and not something I am prepared to work on

avatar coolcat-creations
coolcat-creations - comment - 2 Feb 2025

I would make it a priority to get this fix, because websites can lose their leads without the fix. Afterward, you could discuss improving it.

avatar Hackwar
Hackwar - comment - 2 Feb 2025

UPDATE #__mail_templates SET htmlbody = 'COM_CONTACT_ENQUIRY_TEXT_HTML' WHERE template_id = 'com_contact.mail' AND htmlbody = '' AND language = '*'
That is fully b/c, doesn't break any existing modifications and is the correct solution to fixing this. I'm not going to merge this PR the way it is.

@coolcat-creations no, companies are not losing their leads. The data is still there. They can look at the source code of the mail and will see the address there. Yes, I know that this is cumbersome.

avatar brianteeman
brianteeman - comment - 3 Feb 2025

That is only b/c if the email has not been customised

Add a Comment

Login with GitHub to post a comment