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.

Sign up for GitHub

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

Jump to bottom

Hotfix scrm 569 bounce email handling broken #3777

Merged
merged 9 commits into from Jun 29, 2017
Merged

Hotfix scrm 569 bounce email handling broken #3777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7f998fa
fix for: PHP Notice: Undefined property: EmailTemplate::
gymad Jun 23, 2017
f530088
fix php notice: EmailTemplate Plain text cleanup fix
gymad Jun 23, 2017
01b235e
fix typo in a log
gymad Jun 23, 2017
a9b8766
fix duplicate check
gymad Jun 23, 2017
a96f2a9
do not use deprecated func
gymad Jun 23, 2017
b499e9d
add a comment
gymad Jun 26, 2017
1115567
change var names more meaningful
gymad Jun 27, 2017
dcaaad5
replace a deprecated function
gymad Jun 28, 2017
b3829f5
revert test changes
gymad Jun 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/EmailMan/EmailMan.php
View file
Open in desktop
Expand Up @@ -629,7 +629,7 @@ function sendEmail($mail,$save_emails=1,$testmode=false){

if (!$this->valid_email_address($module->email1)) {
$this->set_as_sent($module->email1, true,null,null,'invalid email');
$GLOBALS['log']->fatal('Encountered invalid email address' . $module->email1 . " Emailman id=$this->id");
$GLOBALS['log']->fatal('Encountered invalid email address: ' . $module->email1 . " Emailman id=$this->id");
return true;
}

Expand Down
13 changes: 12 additions & 1 deletion modules/EmailTemplates/EmailTemplate.php
View file
Open in desktop
Expand Up @@ -300,7 +300,18 @@ function fill_in_additional_detail_fields()
{
if (empty($this->body) && !empty($this->body_html)) {
global $sugar_config;
$this->body = strip_tags(html_entity_decode($this->body_html, ENT_COMPAT, $sugar_config['default_charset']));

$bodyCleanup = html_entity_decode($this->body_html, ENT_COMPAT, $sugar_config['default_charset']);

// Template contents should contains at least one
// white space character at after the variable names
// to recognise it when parsing and replacing variables

$bodyCleanup = preg_replace('/(\$\w+\b)([^\s])/', '$1 $2', $bodyCleanup);

$bodyCleanup = strip_tags($bodyCleanup);

$this->body = $bodyCleanup;
}
$this->created_by_name = get_assigned_user_name($this->created_by);
$this->modified_by_name = get_assigned_user_name($this->modified_user_id);
Expand Down
2 changes: 1 addition & 1 deletion modules/EmailTemplates/Save.php
View file
Open in desktop
Expand Up @@ -53,7 +53,7 @@

require_once('modules/EmailTemplates/EmailTemplateFormBase.php');
$form = new EmailTemplateFormBase();
sugar_cache_clear('select_array:'.$focus->object_name.'namebase_module=\''.$focus->base_module.'\'name');
sugar_cache_clear('select_array:'.$focus->object_name.'namebase_module=\''. (isset($focus->base_module) ? $focus->base_module : null).'\'name');
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use '' instead of null to make the intent clear.

Copy link
Contributor Author

@gymad gymad Jun 26, 2017 •
edited

Choose a reason for hiding this comment

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

It doesn't matter by functional behavior.
In PHP at string concatenation the (string)null value cast to string evaluated as an empty string '' .
Also in PHP the undefined variables and object properties evaluated as null and indicates a php notice.
so if it's null that clearly means it is an undefined variable which evaluated as an empty string without php notice.

if(isset($_REQUEST['inpopupwindow']) and $_REQUEST['inpopupwindow'] == true) {
$focus=$form->handleSave('',false, false); //do not redirect.
$body1 = "
Expand Down
2 changes: 1 addition & 1 deletion modules/Emails/EmailUI.php
View file
Open in desktop
Expand Up @@ -1697,7 +1697,7 @@ function doAssignment($distributeMethod, $ieid, $folder, $uids, $users) {
}

if(!empty($msgNo)) {
if ($ie->importOneEmail($msgNo, $uid)) {
if ($ie->returnImportedEmail($msgNo, $uid)) {
$emailIds[] = $ie->email->id;
$ie->deleteMessageOnMailServer($uid);
//$ie->retrieve($ieid);
Expand Down
10 changes: 5 additions & 5 deletions modules/Emails/EmailUIAjax.php
View file
Open in desktop
Expand Up @@ -407,7 +407,7 @@ function handleSubs($subs, $email, $json) {
$_REQUEST['uid'] = $ie->getCorrectMessageNoForPop3($_REQUEST['uid']);
}

if (!$ie->importOneEmail($_REQUEST['uid'], $uid)) {
if (!$ie->returnImportedEmail($_REQUEST['uid'], $uid)) {
$ie->getDuplicateEmailId($_REQUEST['uid'], $uid);
} // id
$ie->email->retrieve($ie->email->id);
Expand Down Expand Up @@ -696,9 +696,9 @@ function handleSubs($subs, $email, $json) {
$uid = $msgNo;
if($ie->protocol == 'imap') {
$msgNo = imap_msgno($ie->conn, $msgNo);
$status = $ie->importOneEmail($msgNo, $uid);
$status = $ie->returnImportedEmail($msgNo, $uid);
} else {
$status = $ie->importOneEmail($ie->getCorrectMessageNoForPop3($msgNo), $uid);
$status = $ie->returnImportedEmail($ie->getCorrectMessageNoForPop3($msgNo), $uid);
} // else
$return[] = $app_strings['LBL_EMAIL_MESSAGE_NO'] . " " . $count . ", " . $app_strings['LBL_STATUS'] . " " . ($status ? $app_strings['LBL_EMAIL_IMPORT_SUCCESS'] : $app_strings['LBL_EMAIL_IMPORT_FAIL']);
$count++;
Expand All @@ -711,9 +711,9 @@ function handleSubs($subs, $email, $json) {
$msgNo = $_REQUEST['uid'];
if($ie->protocol == 'imap') {
$msgNo = imap_msgno($ie->conn, $_REQUEST['uid']);
$status = $ie->importOneEmail($msgNo, $_REQUEST['uid']);
$status = $ie->returnImportedEmail($msgNo, $_REQUEST['uid']);
} else {
$status = $ie->importOneEmail($ie->getCorrectMessageNoForPop3($msgNo), $_REQUEST['uid']);
$status = $ie->returnImportedEmail($ie->getCorrectMessageNoForPop3($msgNo), $_REQUEST['uid']);
} // else
$return[] = $app_strings['LBL_EMAIL_MESSAGE_NO'] . " " . $count . ", " . $app_strings['LBL_STATUS'] . " " . ($status ? $app_strings['LBL_EMAIL_IMPORT_SUCCESS'] : $app_strings['LBL_EMAIL_IMPORT_FAIL']);

Expand Down
28 changes: 14 additions & 14 deletions modules/InboundEmail/InboundEmail.php
View file
Open in desktop
Expand Up @@ -4404,10 +4404,10 @@ public function importDupeCheck($message_id, $header, $textHeader)
$this->compoundMessageId = md5($this->compoundMessageId);

$query = 'SELECT count(emails.id) AS c FROM emails WHERE emails.message_id = \'' . $this->compoundMessageId . '\' and emails.deleted = 0';
$r = $this->db->query($query, true);
$a = $this->db->fetchByAssoc($r);
$results = $this->db->query($query, true);
$row = $this->db->fetchByAssoc($results);

if ($a['c'] > 0) {
if ($row['c'] > 0) {
$GLOBALS['log']->debug('InboundEmail found a duplicate email with ID (' . $this->compoundMessageId . ')');

return false; // we have a dupe and don't want to import the email'
Expand Down Expand Up @@ -4602,13 +4602,13 @@ public function getDuplicateEmailId($msgNo, $uid)
if (!$dupeCheckResult && !empty($this->compoundMessageId)) {
// we have a duplicate email
$query = 'SELECT id FROM emails WHERE emails.message_id = \'' . $this->compoundMessageId . '\' and emails.deleted = 0';
$r = $this->db->query($query, true);
$a = $this->db->fetchByAssoc($r);
$results = $this->db->query($query, true);
$row = $this->db->fetchByAssoc($results);

$this->email = new Email();
$this->email->id = $a['id'];
$this->email->id = $row['id'];

return $a['id'];
return $row['id'];
} // if
return "";
} // else
Expand Down Expand Up @@ -5665,10 +5665,10 @@ public function getCaseIdFromCaseNumber($emailName, $aCase)
$query = 'SELECT id FROM cases WHERE case_number = '
. $this->db->quoted($sub3)
. ' and deleted = 0';
$r = $this->db->query($query, true);
$a = $this->db->fetchByAssoc($r);
if (!empty($a['id'])) {
return $a['id'];
$results = $this->db->query($query, true);
$row = $this->db->fetchByAssoc($results);
if (!empty($row['id'])) {
return $row['id'];
}
}
}
Expand Down Expand Up @@ -6414,7 +6414,7 @@ public function moveEmails($fromIe, $fromFolder, $toIe, $toFolder, $uids, $copy
}

if (!empty($msgNo)) {
$importStatus = $this->importOneEmail($msgNo, $uid);
$importStatus = $this->returnImportedEmail($msgNo, $uid);
// add to folder
if ($importStatus) {
$sugarFolder->addBean($this->email);
Expand Down Expand Up @@ -6689,7 +6689,7 @@ public function setEmailForDisplay($uid, $isMsgNo = false, $setRead = false, $fo

}

$this->importOneEmail($msgNo, $uid, true);
$this->returnImportedEmail($msgNo, $uid, true);
$this->email->id = '';
$this->email->new_with_id = false;
$ret = 'import';
Expand Down Expand Up @@ -7606,7 +7606,7 @@ protected function importMailboxMessages($protocol, $mailbox = null)
foreach ($msgNumbers as $msgNumber) {
$uid = $this->getMessageUID($msgNumber, $protocol);
$GLOBALS['log']->info('Importing message no: ' . $msgNumber);
$this->importOneEmail($msgNumber, $uid, false, false);
$this->returnImportedEmail($msgNumber, $uid, false, false);
}
}

Expand Down
6 changes: 3 additions & 3 deletions modules/Schedulers/_AddJobsHere.php
View file
Open in desktop
Expand Up @@ -162,7 +162,7 @@ function pollMonitoredInboxes()
$uid = imap_uid($ieX->conn, $msgNo);
} // else
if ($isGroupFolderExists) {
if ($ieX->importOneEmail($msgNo, $uid)) {
if ($ieX->returnImportedEmail($msgNo, $uid)) {
// add to folder
$sugarFolder->addBean($ieX->email);
if ($ieX->isPop3Protocol()) {
Expand Down Expand Up @@ -206,7 +206,7 @@ function pollMonitoredInboxes()
} // if
} else {
if ($ieX->isAutoImport()) {
$ieX->importOneEmail($msgNo, $uid);
$ieX->returnImportedEmail($msgNo, $uid);
} else {
/*If the group folder doesn't exist then download only those messages
which has caseid in message*/
Expand Down Expand Up @@ -612,7 +612,7 @@ function pollMonitoredInboxesAOP()
} // if
} else {
if ($aopInboundEmailX->isAutoImport()) {
$aopInboundEmailX->importOneEmail($msgNo, $uid);
$aopInboundEmailX->returnImportedEmail($msgNo, $uid);
} else {
/*If the group folder doesn't exist then download only those messages
which has caseid in message*/
Expand Down
2 changes: 1 addition & 1 deletion tests/tests/modules/InboundEmail/InboundEmailTest.php
View file
Open in desktop
Expand Up @@ -1793,7 +1793,7 @@ public function testimportOneEmail()

//execute the method and test if it works and does not throws an exception.
try {
$result = $inboundEmail->importOneEmail('1', '1');
$result = $inboundEmail->returnImportedEmail('1', '1');
$this->assertEquals(false, $result);
} catch (Exception $e) {
$this->fail();
Expand Down