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

Email Addresses Automatically Deleted By Workflows Relating to Contacts #7842

Closed
deuks opened this issue Sep 10, 2019 · 8 comments
Closed

Email Addresses Automatically Deleted By Workflows Relating to Contacts #7842

deuks opened this issue Sep 10, 2019 · 8 comments
Assignees
Labels
Area: Emails Issues & PRs related to all things regarding emails & email module Priority:Important Issues & PRs that are important; broken functions, errors - there are workarounds Type:Bug Bugs within the core SuiteCRM codebase

Comments

@deuks
Copy link

deuks commented Sep 10, 2019

If a workflow runs and makes changes to a contact, if the contact has more than 2 emails, all but 2 emails are deleted

There are some issues (specifically issue #6983, although I don't know if the cause is related) that are similar, but nothing that specifically relates to workflow. This has been tested on demo.

Issue

When a workflow is created, and modifies a contact record, if the contact has more than 2 emails, all but 2 emails are deleted. This is not related to only one type of workflow and is reproducible on save, on scheduler, on all, and doesn't matter which module the workflow is related to, as long as the workflow ends up modifying some field on contact.

I have pictures detailing the contact before and after.

Screen Shot 09-10-19 at 08 46 AM

Screen Shot 09-10-19 at 08 48 AM

Expected Behavior

Workflow doesn't delete email addresses by itself

Actual Behavior

What is occurring is the SugarEmailAddress.php file is setting one out of x > 2 addresses to primary, one as a reply to (?) and then deleting any others left in the $check_list array. You can see this all in the logs when debug mode is turned on.

Possible Fix

I don't have an exact fix but this is as close as I got:
It would seem that all addresses are placed in the $check_list array in SugarEmailAddress.php, then at line 698 they enter an if statement where a primary address is selected, and also a reply to address (This part I'm not sure about, but I'm guessing that's what happens at 701). These addresses are unset at line 704, leaving all the rest of the addresses in the $current_links array. When the array reaches 730, the array is not a conversion, and is not empty, so it passes the conditionals and those links are then deleted. Issue #6983 seemed to believe the if statement at 730 shouldn't always pass, but I don't know enough to about the intended functionality to determine whether this is the issue, or if the issue is that these addresses were supposed to be removed from $current_links before it reached that point.

$upd_eabr = "";
if (isset($current_links[$emailId])) {
if (!$isConversion) { // do not update anything if this is for lead conversion
if ($address['primary_address'] != $current_links[$emailId]['primary_address'] or $address['reply_to_address'] != $current_links[$emailId]['reply_to_address']) {
$upd_eabr = "UPDATE email_addr_bean_rel SET primary_address='" . $this->db->quote($address['primary_address']) . "', reply_to_address='" . $this->db->quote($address['reply_to_address']) . "' WHERE id='" . $this->db->quote($current_links[$emailId]['id']) . "'";
}
unset($current_links[$emailId]);
}
} else {
$primary = $address['primary_address'];
if (!empty($current_links) && $isConversion) {
foreach ($current_links as $eabr) {
if ($eabr['primary_address'] == 1) {
// for lead conversion, if there is already a primary email, do not insert another primary email
$primary = 0;
break;
}
}
}
$now = $this->db->now();
$upd_eabr = "INSERT INTO email_addr_bean_rel (id, email_address_id,bean_id, bean_module,primary_address,reply_to_address,date_created,date_modified,deleted) VALUES('" . $this->db->quote($guid) . "', '" . $this->db->quote($emailId) . "', '" . $this->db->quote($id) . "', '" . $this->db->quote($module) . "', " . intval($primary) . ", " . intval($address['reply_to_address']) . ", $now, $now, 0)";
}
if (!empty($upd_eabr)) {
$r2 = $this->db->query($upd_eabr);
}
}
}
}
//delete link to dropped email address.
// for lead conversion, do not delete email addresses
if (!empty($current_links) && !$isConversion) {
$delete = "";
foreach ($current_links as $eabr) {
$delete .= empty($delete) ? "'" . $this->db->quote($eabr['id']) . "' " : ",'" . $this->db->quote($eabr['id']) . "'";
}
$eabr_unlink = "update email_addr_bean_rel set deleted=1 where id in ({$delete})";
$this->db->query($eabr_unlink);
}

Steps to Reproduce

  1. Create a contact with more than 2 emails (or use an existing contact with more than 2 emails)
  2. Create a workflow relating to anything besides contacts, and modify any field in contacts as one of the actions (in my case I used Tasks and updated the relatives Contact's Primary Street Address, although any combination should work)
  3. Create a Task that meets the conditions of your new workflow
  4. Check the associated contact after workflow runs.

Context

This bug has ended up deleting about a hundred contact emails for my company. We did not realize the leak, as most contacts don't have 3 or more emails. I personally from that standpoint would say that this is high priority due to the potential for data loss. In the meantime, we will have to strip most workflow functionality.

Your Environment

  • SuiteCRM Version used: Admin Demo, 7.10.18
  • Browser name and version (e.g. Chrome Version 51.0.2704.63 (64-bit)): Chrome newest
  • Environment name and version (e.g. MySQL, PHP 7): MariaDB, PHP 7.3
  • Operating System and version (e.g Ubuntu 16.04): Ubuntu 18.04 LTS
@Mac-Rae
Copy link
Contributor

Mac-Rae commented Sep 20, 2019

Hi @deuks,

Would you be able to provide screenshots of the workflow that you were using to replicate this issue?

@Mac-Rae Mac-Rae added the Status:Requires Updates Issues & PRs which requires input or update from the author label Sep 20, 2019
@deuks
Copy link
Author

deuks commented Sep 20, 2019

Shouldn't be hard to replicate. As I already stated, it can be any module, doesn't matter if it is based on new or modified records. Doesn't matter if the trigger is all the time, on save, or on scheduler. Doesn't Matter which Contact field it modifies. I just made one on the demo moments ago, different configuration than the ones I had on my system, and different than the one I used when testing the demo before making this post. Here it is:
Screen Shot 09-20-19 at 02 11 PM

@fcorluka
Copy link

fcorluka commented Sep 14, 2020

This should not be closed because it is not solved.

I have the same issue, only difference is I am using MassUpdate.
Tested on: 7.10.27, 7.10.22., 7.10.14

I opened one oldddd CRM 7.8.8 and did not have this issue.

@deuks
Copy link
Author

deuks commented Feb 11, 2021

This issue is much bigger than I previously thought. Now I have realized that Inline editing on any field on the contact module will also delete email addresses down to 2 email addresses! Why are there literally zero attempts to address this?

This was tested on a fresh install of 7.10.29. This was also tested on the SuiteCRM demo and the same issue was prevelant, This is massive data loss and frankly makes inline editing useless if it will cause fields to unrelate/delete.

There obviously needs to be major time invested into making sure this bug isn't in other areas of the system. I would assume leads has similar issues, and perhaps even accounts.

@pribeiro42
Copy link
Contributor

Just tested on a clean install of 7.10.29 and 7.11.18.. The issue is the same on accounts !

@pribeiro42
Copy link
Contributor

For some reason, on the saveEmail method of the include/SugarEmailAddress/SugarEmailAddresses.php file, only the first two email addresses are included in the addresses property, so, all the others get deleted ate the end of this method...

@khan01
Copy link

khan01 commented Mar 28, 2021

I was able to fix it by changing

if (!empty($current_links) && !$isConversion) {
to

if (!empty($current_links) && $isConversion) {

@johnM2401 johnM2401 added Area: Emails Issues & PRs related to all things regarding emails & email module Priority:Important Issues & PRs that are important; broken functions, errors - there are workarounds Type:Bug Bugs within the core SuiteCRM codebase and removed Status:Requires Updates Issues & PRs which requires input or update from the author labels Jul 2, 2021
@clemente-raposo
Copy link
Contributor

clemente-raposo commented Dec 7, 2021

@deuks @JanSiero @fcorluka @pribeiro42

After analysing the problem the issue seem to have been caused by the code mentioned on this comment: #5203 (comment)

It was re-inializing the list of email addresses just adding email1 and email2.

Created MR to fix revert code and fix this issue
#9391

clemente-raposo added a commit that referenced this issue Dec 10, 2021
- Revert 41ee520
- Fixes issue in workflows, where email addresses where being removed.
-- this happened if record had more than 2 email addresses
clemente-raposo added a commit that referenced this issue May 17, 2022
- Revert 41ee520
- Fixes issue in workflows, where email addresses where being removed.
-- this happened if record had more than 2 email addresses

(cherry picked from commit 23f6a0d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Emails Issues & PRs related to all things regarding emails & email module Priority:Important Issues & PRs that are important; broken functions, errors - there are workarounds Type:Bug Bugs within the core SuiteCRM codebase
Projects
None yet
Development

No branches or pull requests

8 participants