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

Allow setting admin user notification email receiver #3722

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions app/code/core/Mage/Admin/Model/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -733,17 +733,21 @@ protected function _getDateNow($dayOnly = false)
* You can declare additional emails in Mage_Core general/additional_notification_emails/admin_user_create node.
*
* @param Mage_Admin_Model_User $user
* @param string|false $receiverEmail An optional receiver email address for the notification. Overrides default and additional emails.
* @return $this
*/
public function sendAdminNotification($user)
public function sendAdminNotification($user, $receiverEmail = false)
{
// define general contact Name and Email
$generalContactName = Mage::getStoreConfig('trans_email/ident_general/name');
$generalContactEmail = Mage::getStoreConfig('trans_email/ident_general/email');

// collect general and additional emails
$emails = $this->getUserCreateAdditionalEmail();
$emails[] = $generalContactEmail;
// collect general and additional emails unless overridden
if ($receiverEmail && filter_var($receiverEmail, FILTER_VALIDATE_EMAIL)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Zend_Validate::is($receiverEmail, 'EmailAddress') perhaps?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, because FILTER_VALIDATE_EMAIL is not used.

$emails = [$receiverEmail];
} else {
$emails = array_merge($this->getUserCreateAdditionalEmail(), [$generalContactEmail]);
}

/** @var Mage_Core_Model_Email_Template_Mailer $mailer */
$mailer = Mage::getModel('core/email_template_mailer');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php
/**
* OpenMage
*
* This source file is subject to the Open Software License (OSL 3.0)
* that is bundled with this package in the file LICENSE.txt.
* It is also available at https://opensource.org/license/osl-3-0-php
*
* @category Mage
* @package Mage_Adminhtml
* @copyright Copyright (c) 2006-2020 Magento, Inc. (https://www.magento.com)
* @copyright Copyright (c) 2022-2023 The OpenMage Contributors (https://www.openmage.org)
* @license https://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
*/

/**
* System config email field backend model that also allows empty address
*
* @category Mage
* @package Mage_Adminhtml
*/
class Mage_Adminhtml_Model_System_Config_Backend_Email_Addressorempty extends Mage_Core_Model_Config_Data
{
protected function _beforeSave()
{
$value = $this->getValue();
if ($value && !Zend_Validate::is($value, 'EmailAddress')) {
Mage::throwException(Mage::helper('adminhtml')->__('Invalid email address "%s".', $value));
}
return $this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,9 @@ public function saveAction()
try {
$model->save();
// Send notification to General and additional contacts (if declared) that a new admin user was created.
if (Mage::getStoreConfigFlag('admin/security/crate_admin_user_notification') && $isNew) {
Mage::getModel('admin/user')->sendAdminNotification($model);
if (Mage::getStoreConfigFlag('admin/security/create_admin_user_notification') && $isNew) {
$contactEmail = Mage::getStoreConfig('admin/security/create_admin_user_notification_email');
Mage::getModel('admin/user')->sendAdminNotification($model, $contactEmail);
}
if ($uRoles = $this->getRequest()->getParam('roles', false)) {
if (is_array($uRoles) && (count($uRoles) >= 1)) {
Expand Down
2 changes: 1 addition & 1 deletion app/code/core/Mage/Core/etc/config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<config>
<modules>
<Mage_Core>
<version>1.6.0.10</version>
<version>1.6.0.11</version>
</Mage_Core>
</modules>
<global>
Expand Down
18 changes: 15 additions & 3 deletions app/code/core/Mage/Core/etc/system.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1256,16 +1256,28 @@
<show_in_website>0</show_in_website>
<show_in_store>0</show_in_store>
</extensions_compatibility_mode>
<crate_admin_user_notification translate="label comment">
<create_admin_user_notification translate="label comment">
Copy link
Contributor

Choose a reason for hiding this comment

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

I would leave the typo as-is. Yes, you move the value to the new setting, but legacy code may still use the old xpath.

Copy link
Contributor

Choose a reason for hiding this comment

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

For legacy code, this can be used, if I remember correctly:

<crate_admin_user_notification deprecated="true">
    <config_path>admin/security/create_admin_user_notification</config_path>
</crate_admin_user_notification>

Copy link
Contributor

Choose a reason for hiding this comment

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

Where is that @luigifab ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here:

<account translate="label" module="paypal">
<label>Merchant Location</label>
<fieldset_css>paypal-config</fieldset_css>
<expanded>1</expanded>
<sort_order>5</sort_order>
<show_in_default>1</show_in_default>
<show_in_website>1</show_in_website>
<frontend_model>paypal/adminhtml_system_config_fieldset_location</frontend_model>
<fields>
<merchant_country translate="label comment">
<label>Merchant Country</label>
<config_path>paypal/general/merchant_country</config_path>

Copy link
Contributor

Choose a reason for hiding this comment

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

In #3721 was decided not to create a breaking change to fix a typo, let's please address the comments made by other users if possible

<label>New Admin User Create Notification</label>
<comment>This setting enable notification when new admin user created.</comment>
<comment>This setting enables notification when new admin user is created.</comment>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is ok to change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Before changing the comment, please check if there is a translation for that phrase. If exists you should change only the CSV file.

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, needs to be changed in Mage_Core.csv instead of here, so that we don't break language packs

<frontend_type>select</frontend_type>
<sort_order>10</sort_order>
<source_model>adminhtml/system_config_source_enabledisable</source_model>
<show_in_default>1</show_in_default>
<show_in_website>0</show_in_website>
<show_in_store>0</show_in_store>
</crate_admin_user_notification>
</create_admin_user_notification>
<create_admin_user_notification_email translate="label comment">
<label>Notification Email Receiver</label>
<comment>Email address to receive notifications when a new admin user is created. Defaults to the General Store email address if empty.</comment>
<depends><create_admin_user_notification>1</create_admin_user_notification></depends>
<frontend_type>text</frontend_type>
<validate>validate-email</validate>
<backend_model>adminhtml/system_config_backend_email_addressorempty</backend_model>
<sort_order>11</sort_order>
<show_in_default>1</show_in_default>
<show_in_website>0</show_in_website>
<show_in_store>0</show_in_store>
</create_admin_user_notification_email>
<reprocess_image_quality translate="label comment">
<label>Image Reprocess Quality</label>
<comment>The recommended value is 85, a higher value will increase the file size. You can set the value to 0 to disable image processing, but it may cause security risks.</comment>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php
/**
* OpenMage
*
* This source file is subject to the Open Software License (OSL 3.0)
* that is bundled with this package in the file LICENSE.txt.
* It is also available at https://opensource.org/license/osl-3-0-php
*
* @category Mage
* @package Mage_Core
* @copyright Copyright (c) 2006-2020 Magento, Inc. (https://www.magento.com)
* @copyright Copyright (c) 2022 The OpenMage Contributors (https://www.openmage.org)
* @license https://opensource.org/licenses/osl-3.0.php Open Software License (OSL 3.0)
*/

/** @var Mage_Core_Model_Resource_Setup $installer */
$installer = $this;
$installer->startSetup();

$table = $installer->getTable('core/config_data');

if ($installer->getConnection()->isTableExists($table)) {
$oldPath = 'admin/security/crate_admin_user_notification';
$newPath = 'admin/security/create_admin_user_notification';

$select = $installer->getConnection()->select()
->from($table, ['config_id', 'path'])
->where('path = ?', $oldPath);

$rows = $installer->getConnection()->fetchAll($select);

foreach ($rows as $row) {
$installer->getConnection()->update(
$table,
['path' => $newPath],
['config_id = ?' => $row['config_id']]
);
}
}

$installer->endSetup();
Loading