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

Make including pw in mail to user optional when created by admin #10462

Closed
wants to merge 8 commits into from

Conversation

sovainfo
Copy link
Contributor

@sovainfo sovainfo commented May 14, 2016

Pull Request for Issue #10376 .

Summary of Changes

When creating user accounts as administrator and sending an email, it is now optional to include the password in the mail.
This is configurable on the plugin. Sending the email was already optional.
When sending emails, including the password in the email is now optional.

Testing Instructions

Apply this patch
As administrator create an user account.
Verify that the password is included in the mail.
Configure plugin User - Joomla to exlude the password.
Create an user account.
Verify that the password in not in the mail.

This closes issue #10376

…nistrator.

When creating user accounts as administrator and sending an email, it is now optional to include the password in the mail.
This is configurable on the plugin. Sending the email was already optional. 
When sending emails, including the password in the email is now optional.

This closes issue joomla#10376
@wojsmol wojsmol mentioned this pull request May 14, 2016
@wojsmol
Copy link
Contributor

wojsmol commented May 14, 2016

@sovainfo see sovainfo#3

@@ -8,11 +8,14 @@ PLG_USER_JOOMLA_FIELD_AUTOREGISTER_DESC="Automatically create Registered Users w
PLG_USER_JOOMLA_FIELD_AUTOREGISTER_LABEL="Auto-create Users"
PLG_USER_JOOMLA_FIELD_FORCELOGOUT_DESC="Set to No to disable this."
PLG_USER_JOOMLA_FIELD_FORCELOGOUT_LABEL="Force Logout for all Sessions?"
PLG_USER_JOOMLA_FIELD_INCLUDEPASSWORD_DESC="Inlude the password when sending a mail to the created user."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Include password in the email sent to the created user."?

@sovainfo
Copy link
Contributor Author

Thank you, @wojsmol @andrepereiradasilva

@BurtNL
Copy link
Contributor

BurtNL commented May 15, 2016

I have tested this item ✅ successfully on dd4e457

Tested successfully. All works as described.
Without the patch: username and password will be send by email.
With the patch applied: send password is optional and will not be send if ticked 'No'.


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

@brianteeman
Copy link
Contributor

I dont understand why you did it this way in the plugin and didnt use the existing option

screen shot 2016-05-17 at 04 11 08


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

@sovainfo
Copy link
Contributor Author

Disagree on using the option meant for the registration process. The registration process is different from the account creation process by an administrator. Consider that you should be able to configure that setting for each process. The settings can be the same or different. Setting it for one process has no bearing on the other. That is the functionality I think should be provided.

@infograf768
Copy link
Member

I agree this should be done in the plugin as it is already there that sending mail is defined.
@sovainfo Can you correct the conflicts?

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @BurtNL


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

@joomla-cms-bot
Copy link

This PR has received new commits.

CC: @BurtNL


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

@sovainfo
Copy link
Contributor Author

Tried to fix the conflicts but apparently failed:

  • copy content staging/joomla.xml
  • add field
  • edit patch-5/joomla.xml
    Same thing for joomla.php

Maybe somebody else has more luck!

@wojsmol
Copy link
Contributor

wojsmol commented May 20, 2016

@sovainfo Please revert to dd4e457. Then rebase will be simple

@sovainfo
Copy link
Contributor Author

sovainfo commented May 20, 2016

@wojsmol Thanks for trying to help. Using GitHub for Firefox exclusively, haven't found proper way to resolve conflicts, revert or rebase. The only way I know is to close and create a new PR.

Very disappointing if that is what needs to be done. Will wait for feedback before giving up and closing!

@wojsmol
Copy link
Contributor

wojsmol commented May 20, 2016

@wojsmol
Copy link
Contributor

wojsmol commented May 20, 2016

@sovainfo See staging...wojsmol:10462-rebase - current staging with thIs PR.

@sovainfo
Copy link
Contributor Author

@wojsmol Thanks, but neither of them help. The procedure is incorrect and can't do anything with the rebase you are pointing to!

Unbelievable for a product that is supposed to make these things easier!

@roland-d
Copy link
Contributor

@sovainfo Is this PR to be redone or can you fix the conflicts?

@sovainfo
Copy link
Contributor Author

Can't fix the conflicts!

@sovainfo sovainfo closed this Jun 24, 2016
@wojsmol
Copy link
Contributor

wojsmol commented Jun 25, 2016

@sovainfo Are you planning redo this PR based on the current staging?

@sovainfo
Copy link
Contributor Author

Yes, probably after the release of J360.

@brianteeman
Copy link
Contributor

Please see #10990

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants