salesagility / SuiteCRM

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
4.39k stars 2.06k forks source link

Accessing Group Mailboxes Does Not Work #9028

Open greenlanegreb opened 3 years ago

greenlanegreb commented 3 years ago

There is no ability to assign access to Group Mailboxes to users via user admin or the group mailbox itself.

If I am not mistaken (and one of the Devs John, Maintainer) hasn't had any luck either, then a CRM without an ability to communicate and interact is a very serious issue and, I would propose that whatever issues there are with the functionality needs to be resolved as a priority. I am running the latest version of Suite CRM on a Linux Server with MySQL (although not obviously relevant).

Thanks and Happy New Year.

johnM2401 commented 3 years ago

Links to relevant threads: https://community.suitecrm.com/t/new-cases-not-being-created-from-email-and-return-emails-not-being-sent/76254/17 https://community.suitecrm.com/t/how-to-assign-inbox-group-email-account-to-users/70220/

tsmgeek commented 3 years ago

Might be fixed by #8702

greenlanegreb commented 3 years ago

Please see my notes added to bug #8745 - I think CSS as a possible hidden assign button may be central to the issue. Cheers.

greenlanegreb commented 3 years ago

Might be fixed by #8702

@tsmgeek good day Ricardo, has this fix been rolled out in the latest update please? Thank you.

greenlanegreb commented 3 years ago

@tsmgeek - i've updated #8702 as your code doesn't fix this but thanks so much for your hard work. Is your code OK to leave in situ on a long term basis? I have backed up the original files but I again shout CSS issue as explained just a moment ago on #8702

Thanks and Kind Regards,

Beanie.

greenlanegreb commented 3 years ago

Any further exploration of this issue please or temporary fixes? Thanks.

tsmgeek commented 3 years ago

I use all my patches in production where I work, problem is that the Email module IMO is deeply flawed after it was all reworked and even with my patches it still needs more work to fix it properly.

greenlanegreb commented 3 years ago

Thanks tsmgeek - it's categorised as critical, it's been open 13 days and nobody is keeping us up to date with whether anything is going on. I'm probably going to leave SuiteCRM once and for all. It's been an absolute disaster in one way or another for so long now. As I write, 1000+ issues. If your anyone other than a software Engineer or you enjoy coding (and even then I would say that the resources would be used better elsewhere) then you'd be out of your mind (like me) to have been patient over the last 12 months. This is only going to get worse with people leaving en masse. My contact is apparently away for three months. I've tried and the probject just has been so badly run it's beyond saving. Whilst there will always be the brigade that say that you should expect nothing from open source and you'd not be disappointed, it rather begs the question why they are there in the first place, what need they are actually trying to fulfill or anything else. The amount of coding involved in fixing these issues would justify those same coders regrouping and abandoning this altogether.

SuiteBot commented 2 years ago

This issue has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/basic-setup-with-group-email/77776/3

pstevens71 commented 1 year ago

I can confirm this is still an issue in 7.12.8. I've looked everywhere for a solution or at least a clue to what is going on. This seems to be the only open ticket regarding this issue, others have been closed that documented it better because they were "duplicates". Anyway, I made a video of the problem. I think this demonstrates what's going on. https://vimeo.com/823530686/6ebb9faee5?share=copy

pstevens71 commented 1 year ago

Ok found a bunch of issues, trying to solve. I have group inbound email working. (kind of). Here is one major problem:

In SugarFolders.php in the function retrieveFoldersForProcessing…

1) Line 790 is the problem "isgroup" should be "is_group":
if ($item->isgroup === 1 || $item['created_by'] === $user->id || is_admin($user)) {

2) Line 790 is_group ===1 never returns true should be $item['is_group'] === '1' like this:

if ($item['is_group'] === '1' || $item['created_by'] === $user->id || is_admin($user)) {

That works and shows folders that are group folders that are subscribed to by the user and lets admin see all the folders.

However...... One other problem I'll look into next is in the "folders" table, "is_group" is not getting populated when a group email account is added. That's tomorrows problem. Anyway, I manually made "is_group" =1 for the group folders in the "folders" table and it works.

SuiteBot commented 1 year ago

This issue has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/does-anyone-have-group-emails-working-for-users/88944/7

chris001 commented 1 year ago

@pstevens71 Excellent find! Keep going!

EDIT: Does #8702 fix this issue, show folders that are group folders, and write to the is_group column in the folders table, when you add or remove a group account?

pstevens71 commented 1 year ago

@chris001 sort of the fix proposes as part of it changing this:

if ($item->isgroup === 1 || $item['created_by'] === $user->id || is_admin($user)) { to:

if ($item['is_group'] == 1 || $item['created_by'] === $user->id || is_admin($user)) {

That won't work, I tried: the 1 needs to be in quotes like this:

if ($item['is_group'] === '1' || $item['created_by'] === $user->id || is_admin($user)) {

The other proposed fixes don't address this specific problem.

The second part of the problem is that when an Inbound email account is saved, the "is_group" field is not getting populated. I'm thinking something like this around line 221 of modules/InboundEmail/Save.php to check if the inbound_email table has "is_personal=0", in which case, it's a group account and the folder records should be updated where is_group = 1. I haven't tested yet, but here's what I'm thinking.... Add:

$isGroupFolder = 0;
if ($focus->is_personal === '0') {
    $isGroupFolder = 1;
}

then in the record creation like this (the is_group I added):

"inbound" => array(
            'name' => $focus->mailbox . ' ('.$focus->name.')',
            'folder_type' => "inbound",
            'has_child' => 1,
            'dynamic_query' => '',
            'is_dynamic' => 1,
            'created_by' => $focusUser->id,
            'modified_by' => $focusUser->id,
            'is_group' => $isGroupFolder,
        ),
pstevens71 commented 1 year ago

actually, fixing the one line: if ($item['is_group'] === '1' || $item['created_by'] === $user->id || is_admin($user)) {

Fixes the group emails on the front end, but in email settings, it crashes out the subscribed folders selection. @chriss001 I'll try those other two changes in conjunction with this one tomorrow and see if they fix the folder selection timing out. Thanks for suggesting that. Fingers crossed.

chris001 commented 1 year ago

@pstevens71 Here's some more places of interest in the code, in case these help:

https://github.com/salesagility/SuiteCRM/blob/d0f078b7b75179bcad1cf5f8e7c2d9f5c52df2c0/modules/InboundEmail/InboundEmail.php#L381

https://github.com/salesagility/SuiteCRM/blob/d0f078b7b75179bcad1cf5f8e7c2d9f5c52df2c0/modules/Emails/EmailsDataAddress.php#L96

https://github.com/salesagility/SuiteCRM/blob/d0f078b7b75179bcad1cf5f8e7c2d9f5c52df2c0/modules/Emails/EmailsController.php#L954

https://github.com/salesagility/SuiteCRM/blob/d0f078b7b75179bcad1cf5f8e7c2d9f5c52df2c0/modules/Emails/EmailsDataAddressCollector.php#L152

pstevens71 commented 1 year ago

Thanks @chris001 I solved my problem with the user profile crashing out. Thanks to @pgorod he suggested my === was too restrictive. So I changed it to the following (at least I have group emails working!!). Next step is to solve the write is_group =1 to the folders table on save of an inbound email account and we'll be almost there. The third problem is that the folder selection in the user account doesn't actually work. It does write to the folder_subscriptions table, however, the user gets access to ALL group accounts, not just the ones selected, so that needs to be fixed too. Here's the modified line. I've been doing alot of testing and this seems so far to make everything work smoothly in terms of the user seeing the folders and user profile not crashing out.

if ($item['is_group'] == 1 || $item['created_by'] === $user->id || is_admin($user)) {

pstevens71 commented 1 year ago

OK I have it working. Problem number 2 solved. Still have some testing to do. I modified the public function save($addSubscriptions = true) to check the inbound email account for is_personal and then set is_group based on the opposite of that. I've tested for updated records and works. I've also tested for adding new inbound email and it works. However, while testing I'm finding that folders from deleted inbound email accounts are not getting removed. That seems to be another issue to chase down. Should I open another issue for that or keep all this here?

Here's what I've done:

public function save($addSubscriptions = true)
    {
         $this->dynamic_query = $this->db->quote($this->dynamic_query);
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////CHANGE     
// Retrieve the value of 'is_personal' for the current folder
 // Check if the current folder is a parent folder
if (!empty($this->parent_folder)) {
    $query2 = "SELECT is_personal FROM inbound_email WHERE id = " . $this->db->quoted($this->parent_folder);
} else {
    $query2 = "SELECT is_personal FROM inbound_email WHERE id = " . $this->db->quoted($this->id);
}

$r2 = $this->db->query($query2);
$is_personal = $this->db->fetchByAssoc($r2)['is_personal'];

// Set the value of 'is_group' based on the value of 'is_personal'
if ($is_personal == 1) {
    $is_group = 0;
} else {
    $is_group = 1;
}

/////////////////////////////////////////////////////////////////////////////////////////////////////END  CHANGES

        if ((!empty($this->id) && $this->new_with_id == false) || (empty($this->id) && $this->new_with_id == true)) {
            if (empty($this->id) && $this->new_with_id == true) {
                $guid = create_guid();
                $this->id = $guid;
            }

            $query = "INSERT INTO folders (id, name, folder_type, parent_folder, has_child, is_group, " .
                 "is_dynamic, dynamic_query, assign_to_id, created_by, modified_by, deleted) VALUES (" .
                    $this->db->quoted($this->id) . ", " .
                    $this->db->quoted($this->name) . ", " .
                    $this->db->quoted($this->folder_type) . ", " .
                    $this->db->quoted($this->parent_folder) . ", " .
                    $this->db->quoted($this->has_child) . ", " .
                    ////////////////////////////////////////////////////////CHANGE
                //    $this->db->quoted($this->is_group) . ", " .//remove
                  $this->db->quoted($is_group) . ", " . // set the value of is_group here
                //////////////////////////////////////////////////////////////END CHANGE    
                    $this->db->quoted($this->is_dynamic) . ", " .
                    $this->db->quoted($this->dynamic_query) . ", " .
                    $this->db->quoted($this->assign_to_id) . ", " .
                    $this->db->quoted($this->currentUser->id) . ", " .
                    $this->db->quoted($this->currentUser->id) . ", 0)";

            if ($addSubscriptions) {
                // create default subscription
                $this->addSubscriptionsToGroupFolder();
            }

            // if parent_id is set, update parent's has_child flag
            if (!empty($this->parent_folder)) {
                $query3 = "UPDATE folders SET has_child = 1 WHERE id = " . $this->db->quoted($this->parent_folder);
                $r3 = $this->db->query($query3);
            }
        } else {
            $query = "UPDATE folders SET " .
                "name = " . $this->db->quoted($this->name) . ", " .
                "parent_folder = " . $this->db->quoted($this->parent_folder) . ", " .
                "dynamic_query = " . $this->db->quoted($this->dynamic_query) . ", " .
                "assign_to_id = " . $this->db->quoted($this->assign_to_id) . ", " .
                "modified_by = " . $this->db->quoted($this->currentUser->id) . ", " .
                ///////////////////////////////////////////////////////////////CHANGE
                 "is_group = " . $is_group . " " .
                 //////////////////////////////////////////////////////////////END CHANGE
                "WHERE id = " . $this->db->quoted($this->id);
        }

        return $this->db->query($query, true);
    }
chris001 commented 1 year ago

Good work!

However, while testing I'm finding that folders from deleted inbound email accounts are not getting removed.

I looked over the code, when it deletes inbound email accounts, there's an option or function to remove the folders.

pstevens71 commented 1 year ago

@chris001 I know there is a function that supposed to do that. It doesn't seem to be working. When I delete an Inbound account it doesn't remove the folders. I'll troubleshoot that one next.

pstevens71 commented 1 year ago

I've done some more testing and when a inbound email account is deleted, it does indeed mark the inbound email account "deleted = 1" However, the "public function delete()" doesn't seem to be being called at al in SugarFolders.php. It looks like it's designed to do the job, it's just not being triggered. I tried to log the variables in the function on delete and nothing was outputted. So at least that's a start!

pstevens71 commented 1 year ago

OK I added a PR for the code to update is_group value in group folders: https://github.com/salesagility/SuiteCRM/pull/10059/commits

pstevens71 commented 1 year ago

Ok problem #3 Solved. I'll be adding a PR. Problem #3 is that when an inbound group email account is deleted, it doesn't mark the folders as deleted=1 and users still have access to them. This functionality is already built in SuiteCRM but is never called. I've managed to call it from /modules/IndboundEmail/Delete.php successfully. Here is what needs to be added to Delete.php

//delete related folders by calling delete() in SugarFolders.php
// Retrieve the ID of the inbound email account being deleted
// Create an instance of the SugarFolders class
require_once('include/SugarFolders/SugarFolders.php');

$inboundEmailId = $focus->id;
$sugarFolder = new SugarFolder();

if (!empty($inboundEmailId)) {
    if ($sugarFolder->retrieve($inboundEmailId)) {
        $sugarFolder->delete();
    } else {
        // Write a fatal error to the log indicating retrieval failure
        error_log('Failed to retrieve the sugar folder for inbound email ID: ' . $inboundEmailId);
    }
} else {
    // Write a fatal error to the log indicating null inbound email ID
    error_log('Inbound email ID is null. Cannot proceed.');
}
pstevens71 commented 1 year ago

Ok I've solved all the problems and have group emails working 100%! There's lots of little changes to the JS required to get this to work with folder subscriptions as well as, I had to use the existing SecurityGroups, so it will only work in 7.13+ where Inbound Emails module has a relationship to SecurityGroups. Might try to figure out how to get the last part working in 7.12 later.

The major problems in addition to the ones I just fixed are that the JSON output to the user profile where the user selects the folders doesn't account for a second array in the JSON {"groupFolders"}... Also, due to various other functions groupFolders were appearing twice in the JSON. So had to fix those and fixt the JS to deal with the new JSON structure that now includes a groupFolders array. In addition the existing JS had to be modified not to pull in the child folders and only list the parent folders in the select HTML element. Also I ran into the click and select functionality wasn't working, which I fixed and also the first item in each JSON array is --None-- so I had to deal with the second one and not show it.

I've tested and I can add group folders as admin. I can see all folders and select/deselect ones I don't want to see in my personal folder settings. I can also create a security group, add a user to the group, and the user can see the group folders in addition to any personal folders they may have. The user can also select/deselect folders they want to see in their personal folder settings (assuming they have access).

So my question if anyone can help, is that some of these changes apply to both 7.12 and 7.13+(core) and some only apply to 7.13+(core). Should I do separate (duplicate) pull requests for each version?

chris001 commented 1 year ago

@pstevens71 Excellent work. You can do one PR, and for each "commit" in the PR, comment whether the commit applies to 7.12 and 7.13+ (core), or only 7.13+(core). The maintainers team will read your comments on each commit, and "cherry-pick" each commit, and apply the commit to the relevant branch/release version you say it's made to fix.

pstevens71 commented 1 year ago

thanks @chris001 will do. I just spent most of today testing and adding the changes to another installation for testing. Got it working there too, so I'm pretty confident at this point the changes work.

pstevens71 commented 1 year ago

OK I created a new issue in the "core" branch where all the changes and explanations are linked. You will need 7.13+ to make all this work because prior, there is no relationship between security groups and inbound emails.

Complete Group Folders Solution

SuiteBot commented 1 month ago

This issue has been mentioned on SuiteCRM. There might be relevant details there:

https://community.suitecrm.com/t/problem-with-outgoing-mail-account/93445/2