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

Accessing Group Mailboxes Does Not Work #9028

Open
greenlanegreb opened this issue Feb 8, 2021 · 28 comments
Open

Accessing Group Mailboxes Does Not Work #9028

greenlanegreb opened this issue Feb 8, 2021 · 28 comments
Labels
Area: Emails:Config Issues & PRs related to email configuration Priority:Critical Issues & PRs that are critical; broken core functionality, fatal errors - there are no workarounds Type: Bug Bugs within the core SuiteCRM codebase

Comments

@greenlanegreb
Copy link

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 johnM2401 added Area: Emails:Config Issues & PRs related to email configuration Priority:Critical Issues & PRs that are critical; broken core functionality, fatal errors - there are no workarounds Type: Bug Bugs within the core SuiteCRM codebase labels Feb 9, 2021
@johnM2401
Copy link
Contributor

@tsmgeek
Copy link
Contributor

tsmgeek commented Feb 11, 2021

Might be fixed by #8702

@greenlanegreb
Copy link
Author

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
Copy link
Author

Might be fixed by #8702

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

@greenlanegreb
Copy link
Author

@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
Copy link
Author

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

@tsmgeek
Copy link
Contributor

tsmgeek commented Feb 19, 2021

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
Copy link
Author

greenlanegreb commented Feb 21, 2021

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
Copy link

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
Copy link
Contributor

pstevens71 commented May 4, 2023

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
Copy link
Contributor

pstevens71 commented May 10, 2023

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
Copy link

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
Copy link
Contributor

chris001 commented May 10, 2023

@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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

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

public function isGroupEmailAccount()

'isGroupEmailAccount' => $isGroupEmailAccount,

if ($requestedInboundEmail->isGroupEmailAccount()) {

$isGroupEmailAccount = $inboundEmail->isGroupEmailAccount();

@pstevens71
Copy link
Contributor

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
Copy link
Contributor

pstevens71 commented May 12, 2023

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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 added a commit to pstevens71/SuiteCRM that referenced this issue May 30, 2023
When group email accounts are deleted the associated folders don't get marked as deleted=1.  This means they show up for users in folder selection.  They should be marked as deleted when the inbound group email account is deleted.  There is in fact, functionality to do this in SugarFolders.php but is is not called when an inbound group email is deleted.  This additional code just calls the existing function delete() when an inbound email account is deleted.
@pstevens71
Copy link
Contributor

pstevens71 commented Jun 5, 2023

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
Copy link
Contributor

chris001 commented Jun 6, 2023

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Emails:Config Issues & PRs related to email configuration Priority:Critical Issues & PRs that are critical; broken core functionality, fatal errors - there are no workarounds Type: Bug Bugs within the core SuiteCRM codebase
Projects
None yet
Development

No branches or pull requests

6 participants