salesagility / SuiteCRM

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

BUG: Incorrect DB field name used in SugarFolders class prevents viewing of emails imported from group inbound email accounts #10333

Open MatthewHana opened 8 months ago

MatthewHana commented 8 months ago

Issue

A typo in an array representing the results returned by a DB query mishandles group inbound email accounts.

Expected Behavior

Group accounts should be returned by retrieveFoldersForProcessing function in the SugarFolders class.

Actual Behavior

The typo prevents them from being returned.

Possible Fix

Original code:

$isGroup = $item['isgroup'] ?? '';
if ($isGroup === 1) {
     $secureReturn[] = $item;
}

Correct code:

$isGroup = $item['is_group'] ?? '';
if ($isGroup == 1) {
     $secureReturn[] = $item;
}

Steps to Reproduce

N/A

Context

It does prevent emails imported from group Inbound Email Accounts being displayed for users who have authority to view them. Low priority.

Your Environment

MatthewHana commented 8 months ago

Will make PR to fix soon.

Edit: see below

chris001 commented 8 months ago

Fantastic find! This bug has a few pending PR's: https://github.com/salesagility/SuiteCRM/pull/8702 https://github.com/salesagility/SuiteCRM/pull/10059 https://github.com/salesagility/SuiteCRM/commit/5132dc54f7199f8f60ee510183a2f8320325eb8b @pstevens71 has about 5-6 PRs and commits to fix group inbox. As soon as they're in one PR, they'll be easy to test.

MatthewHana commented 8 months ago

Fantastic find! This bug has a few pending PR's: https://github.com/salesagility/SuiteCRM/pull/8702 https://github.com/salesagility/SuiteCRM/pull/10059 https://github.com/salesagility/SuiteCRM/commit/5132dc54f7199f8f60ee510183a2f8320325eb8b @pstevens71 has about 5-6 PRs and commits to fix group inbox. As soon as they're in one PR, they'll be easy to test.

Ahhh. I should have looked through the existing issues more thoroughly! 😅

pstevens71 commented 8 months ago

I'm hoping to get to that this weekend (combining all the PR's). There are numerous issues around this one. Basically, the group inboxes never get assigned "1" is the first problem, then when they do, this bug is an issue, then when this is fixed, the JSON output to the popup to select a users inbox fails because it contains two arrays instead of one (group emails are separated from personal emails in the JSON), then the JS fails in the popup because it can't handle the JSON. So it's a whole chain of problems.

chris001 commented 8 months ago

@pstevens71 IMHO it'd be nice to get feedback on your fix from the original SA implementer of group inbound email.

pstevens71 commented 8 months ago

@chris001 Yes, me too! I kind of think the original dev opened this can of worms and just removed the whole is_group =1 thing to avoid it. Fixing that one line led to all the rest LOL!

MatthewHana commented 8 months ago

That is a possibility, though the UI still allows you to create a group inbound email account. Surely it would have been easier and cleaner to disable group accounts by simply removing thr UI option.

Regardless of the original reasoning, if any, the implementation in the repo has gotten messy.

@pstevens71 I'm happy to help out however I can if you need it. Just let me know!

chris001 commented 8 months ago

@pstevens71 The fastest and easiest way would be to use an IDE such as VS Code, with add-on GitLens Supercharge to "rebase" all of your commits related to this group inbound email issue, on to your branch with the first part of your fix: https://github.com/pstevens71/SuiteCRM/tree/patch-1 Then, your PR coming from that branch, would be the one PR that would solve the issue, be the candidate to merge into the app, and users would easily run two commands to download and patch their test servers running Suite 7.14.1, try it out, and give you feedback. As it is now, with the scattered commits and branches, it's too cumbersome of a task to expect anyone to spend significant time editing code by hand to apply these fixes and test it. Let the tools do that heavy work quick.

pstevens71 commented 8 months ago

Hey @chris001 I'm not a developer like that. I mostly build WordPress sites with alot of custom PHP, and I can customize SuiteCRM pretty well, but Github, and pushing changes is out of my scope of confidence at least for the time being.

chris001 commented 8 months ago

This git terminology can be confusing until you get the hang of what they really mean. Can you provide a list of links to all the PRs and commits that, when all applied, make up this complete Fix for Group Inbound email?