Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.2] Notification email on admin registration approval #39650

Open
wants to merge 31 commits into
base: 5.2-dev
Choose a base branch
from

Conversation

carlitorweb
Copy link
Member

@carlitorweb carlitorweb commented Jan 16, 2023

This is a rework for j4 of this PR #20282

Summary of Changes

The user who are manually actived will receive a notification by email informing his account has been activated.
There is also the option of being able to manually send a reminder email to the user even though the account is already activated (in case the user do not login in the page after X time)
The mail template used is com_users.registration.user.admin_activated

Testing Instructions

Apply the PR and after run npm run build:js -- build/media_source/com_users/js

  1. Open the frontend and create a new user account
  2. Open the User edit view inside the administration, and you will notice a new button in the toolbar
  3. Use the button and check the user was actived and also a email sent to the email of the new user
  • A email will be sent also if the user is actived in the Users list view using the Actions -> Active option

Actual result BEFORE applying this Pull Request

You can active a user, but not notification is sent.

Expected result AFTER applying this Pull Request

You can active a user, but a notification is sent.

Accept suggestions for:

  • Button text
  • Button icon
  • Button color

user

Link to documentations

Please select:

  • Documentation link for docs.joomla.org
    Page: Users:_Edit_Profile -> Document the new toolbar button
    Page: Users -> Here just need mention that active the user will send a email with the notification.

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.3-dev labels Jan 16, 2023
@brianteeman
Copy link
Contributor

/me dancing with joy

@brianteeman
Copy link
Contributor

Looking good - two small issues

  1. When using the new button I expected to get a message saying "the email has been sent" or something like that
  2. The email that is sent is just the language keys not the values
    image

Guessing that you need to copy the language strings from the site language file to the admin language file

@brianteeman
Copy link
Contributor

or maybe the issue is that its not using the mail template

@carlitorweb
Copy link
Member Author

My bad, I totally forgot the "mail sent" notification.

The other issue is weird, that worked for me in local. I will do a fresh Joomla install and check from there, maybe I had some changes made in the repo I was using...but still weird, somehow the mailer is not loading the strings..

I will check later this

@carlitorweb
Copy link
Member Author

carlitorweb commented Jan 17, 2023

@brianteeman Okay you was right, the com_users.ini language loaded was only from the administrator part, as is expected.

So adding the follow:

// Load com_users site language strings
$language = $app->getLanguage();
$language->load('com_users', JPATH_SITE);

Fix the problem. But I really not like this solution. The best approch is just copy the strings to the administrator .ini language file. Is okay do it like this?

@brianteeman
Copy link
Contributor

How do the other com_users admin emails do it?
Is it perhaps because you are not using the new mail templates?

@carlitorweb
Copy link
Member Author

carlitorweb commented Jan 17, 2023

Is it perhaps because you are not using the new mail templates?

I using it, yes:

$mailer = new \Joomla\CMS\Mail\MailTemplate('com_users.registration.user.admin_activated', $app->getLanguage()->getTag());
$mailer->addTemplateData($mailData);
$mailer->addRecipient($userMail);

How do the other com_users admin emails do it?

They have all the string in the administrator .ini file

@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@HLeithner HLeithner changed the title Notification email on admin registration approval [5.2] Notification email on admin registration approval Apr 24, 2024
@Sieger66
Copy link
Contributor

Sieger66 commented May 4, 2024

I have tested this item ✅ successfully on f5cf34a

Works,
but the activated-email to the new user comes in the backend-language of the "Super User" who
click to the button"Activate & Send Email" or "Send Activation Reminder"-button,
and not in the default-language of the website as the registration-emails.

The registration-emails comes with this PR and without this PR in the default-language of the website.

default-language:
https://help.joomla.org/proxy?keyref=Help51:Languages:_Installed


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 4, 2024
@richard67 richard67 removed the RTC This Pull Request is Ready To Commit label May 4, 2024
@richard67
Copy link
Member

Back to pending due to merge conflicts.


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 4, 2024
@brianteeman
Copy link
Contributor

@richard67 how can this be RTC when we have this comment #39650 (comment)

@richard67
Copy link
Member

@richard67 how can this be RTC when we have this comment #39650 (comment)

@brianteeman Why does @Sieger66 then post a successful test result if it is not successful?

@carlitorweb Is it intended that the email goes out in the backend language of the admin user?

@richard67 richard67 removed the RTC This Pull Request is Ready To Commit label May 4, 2024
@richard67
Copy link
Member

Back to pending due to necessary clarification.


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

@brianteeman
Copy link
Contributor

I can't speak for @Sieger66

What I have observed in other test reports is -" i tested this according to the instructions and it did what it said" but I also think

  • its a bad idea
  • its missing x
  • y happens now

The current system doesnt really allow for that.

@carlitorweb
Copy link
Member Author

carlitorweb commented May 5, 2024

Note: Remember when you install a new language and you want to use the mail template, is neccessary go to the Mail Template section, enter to the template you want to use and save it. Doing this, then the proper tag is generated in the language column of the mail_templates table. If not, always will fallback to en-GB

image

@Sieger66
Copy link
Contributor

Sieger66 commented May 5, 2024

I have tested this item 🔴 unsuccessfully on b13a47a

After your last commit the com_users.registration.user.admin_activated - email comes only with the two language-Strings in the email:
in subject is only "COM_USERS_EMAIL_ACTIVATED_BY_ADMIN_ACTIVATION_SUBJECT"
in body is only "COM_USERS_EMAIL_ACTIVATED_BY_ADMIN_ACTIVATION_BODY"

Then i go to the Mail Template section to "com_users.registration.user.admin_activated" email-template
and click to save in german and english language version.(Only this two languages are installed in the website)
Now the com_users.registration.user.admin_activated - email comes with the correct text but ever in the english language in the email!
The default-language of the website and the "administrator backend-language" have no affect to the language in this email.
I think that is not correct.

All other "registration-emails" for example:
com_users.registration.user.admin_activation
com_users.registration.admin.verification_request
comes in the default-language of the website.

default-language:
https://help.joomla.org/proxy?keyref=Help51:Languages:_Installed


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

@carlitorweb
Copy link
Member Author

carlitorweb commented May 5, 2024

@Sieger66 We have 2 default language param, one is for the administrator and the other is for the frontend. Right now, the email go out with the default language of the administrator.

If I understand well, what you looking for is, the email go out with the default language of the frontend, is this correct?

@Sieger66
Copy link
Contributor

Sieger66 commented May 5, 2024

Yes, this is correct.
Ohh! I now see the default-language param for the administrator and
the email go out with the default language of the administrator.
In detail the "com_users.registration.user.admin_activated" email comes allways with this language with your last commit.
But the other emails comes in the language of the default-language param for the site.

I dont know is it intended and is it correct usecase.

Other small issue:
In the sent notification message in backend comes only the language-key:
COM_USERS_REGISTRATION_ACTIVATION_NOTIFY_SEND_MAIL_SUCCESS
and not the value(text).


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

@carlitorweb
Copy link
Member Author

@Sieger66 issue fixed

Now the email will go out with the site default language.

@Sieger66
Copy link
Contributor

Sieger66 commented May 5, 2024

I have tested this item ✅ successfully on c375030

Works as aspected.
@brianteeman : Now we need a second test.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Required Feature Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet