Nice!
For the PR you probably want to make that code a bit more resilient, by adding checks after each step that might fail. For example, if retrieve doesnāt retrieve, then delete wonāt work, and some proper-error handling is better than a FATALā¦
Hey @pgr I added some checks to the code and logs in case of failure as you suggested. I need some help with Github. Iām really struggling with it. So I went to the file on github and committed a change. However, I think I missed a step somewhere, it didnāt create a PR just a commit? Does one of the moderators have to change it to a PR or is there something I need to do? Or did I just enter it wrong completely?
https://github.com/pstevens71/SuiteCRM/commit/2774e435529aaf8dfa3264fe2e11b36bbaafea78
Normally, you start the edit from SalesAgility repo, not your fork. Then Github tells you that you canāt edit it there and takes you to your fork, where you create the commit. BUT then it will take you to a facilitated ācreate PR back to SAās repoā screen. Right after saving the commit, you would see that create PR screen.
But worry not, if you have the commit, just start a new PR on your repo. It will give you a chance to change both repo and branch, so you can āpoint itā at salesagilityās repo.
Thanks @pgr I think I got it.
Now on to problem #4. This is a doosey! I started investigating last night. Basically, the issue is that in the user profile, when folders are selected, a few things are supposed to happen:
None of this works very well, it doesnāt save, the user can see all the folders, in some cases the ctrl-click doesnāt work, itās kind of a mess.
Plusā¦ there a like half a dozen or so different functions that govern the functionality, not including the front end JavaScript.
From what I understand so far, there is a function to get a list of subscribed folders, another to get a list of all folders, another to clear out all subscribed folders, and then a final one to update the subscribed folders with whatever was just selected. So Iāve got a lot of troubleshooting to follow it through!
OK hot on the trail of a solution!
In SugarFolders.php clearSubscriptions function is clearing subscriptions for $this->currentUser, which in the case of admin trying to assign access to a user, is always admin, even if you are updating a users folder subscriptions. I think that is why itās not updating the users folders.
It gets called from EmailUIAjax.php in the case of āupdateSubscriptionsā, but itās not passing the user id of the user being edited. (I think!) and as a fall back itās currentUser which may be the root of the problem. (I think!).
Ok so hereās where weāre at. Iāve tested ALL the functions in SugarFolders.php (did find some errors). However, for the most part itās working properly.
Iāve also tested most of the functions in EmailUIAjax.php that have to do with folder subscriptions and updates. So far most are working as Iād expect.
However,ā¦ when you fix the code in SugarFolders.php to actually mark Group Folders as is_group=1 then in the User popup to select folders (in user profile), it causes problems with the JS that creates the list of available and subscribed folders.
The list outputs first as JSON
{āuserFoldersā:ā¦ folder1, folder2, folder3}ā¦{āgroupFoldersā: folder1, folder2, folder3}
When group folders were incorrectly marked (before I fixed it), they end up in the userFolder array in JSON. Which gives everyone access to add them to their subscriptions. Now that that issue is fixed, the JS currently does not handle the second JSON array {āgroupFoldersā}.
Soā¦ problem #1
in EmailUI.js retreiveGroupFolderSubscriptions only puts out the ID. So Iāve modified that to put out an array to output the folder name along with the ID and this can be used to create the list.
This should be used in ajax.js in settingsFoldersRefresh: function (and probably more) to output BOTH the results of āuserFoldersā and āgroupFoldersā from the JSON.
Problem #2: The initial output to list doesnāt handle the {groupFolders} either. Need to find exactly where that is too.
So far Iāve also managed to get it to output to the list of folders to subscribe to when one clicks on --none-- to update the folder list.
So once I get them all in the list properly (Iām close), I have to troubleshoot the JS that grabs the selections from the list and does an ajax call to a PHP function in SugarFolders to update the subscribed_folders in the database.
Wow, thought this would be a good PHP project to learn, now Iām having to learn JS, Ajax calls, and JSON.
The thing making this slow is that I have to flush and rebuild all the JS files every time I make a change!! Is there some kind of trick (Iām new to this) to avoid that? Iāve already put SuiteCRM in developer mode.
If you use ādeveloper modeā I think it changes the way JS is handled, though I am not sure what changes. Maybe it doesnāt minify; maybe it always rebuilds JS in every request. Try it.
Any way, while developing, you can make the changes in the destination JS file, not in jssource, until you get things right, and then you make the changes in jssource to put them in the PR.
Ok if anyone is following along my sagaā¦ todayās progress, had to edit function getFoldersForSettings so that it doesnāt put out duplicate group folders in the JSON for group folders array thatās used in folder selection, problem solved!
had to modify: updateSubscriptions: function () in EmailUI.js to make the select options work in folder selection. (it kept refreshing every time I clicked!) Problem solved!
had to modify settingsFolderRefresh: function in ajax.js in order to get the group folders from JSON add them to the pick list. Problem solved!
So now Iām kind of back where I started. Everything works, but the users still have access to all folders. In 7.13 there is now security group logic. So hoping with a few tweaks I can get that working! Last problem I hope to get this working in 7.13 and on ward.
Group Emails Season 1 starring Paul Stevens is the coolest thing on Netflix this year!
And the good thing is that the SuiteCRM emails code has material for a dozen more seasons, this wonāt end anytime soon! 
Thanks @pgr one more quick question. If I have changes that apply to 7.12 and 7.13+ do I need to create a PR on both the suitecrm and the core branch?
That question is better for the SalesAgility folks, I am not sureā¦
But I think this is what is currently in place:
The legacy code in the v7 repos is still going into the v8 package, under public/legacy. Thatās likely coming from the hotfix-7.13 branch.
@clemente.raposo can you please confirm?
Todayās update! I have Group Emails completely working.
Admin can create group accounts, admin can see all group accounts. User can only see group accounts if they are in the security group that inbound group account is in.
One last small thing to fix, is that group emails show up twice in the listing. Iām sure I can get that fixed too.
Thanks @pgr for all your encouragement!
Incredible work @pstevens and assists by @pgr. Your work shows that these issues are fixable. Hopefully, the PRs get implemented.
@pjdm Just spent all day testing it. I installed the changes on another installation just to be sure. There were a couple of hickups, but Iāve got it worked out. I spent a month trying to figure it all out and there are about half a dozen files that need to change. I kept decent notes of what I changed, but wanted to do a clean test just to make sure. It does work. The only catch is it needs to be 7.13.3+ where Inbound Email Accounts were added to security groups. My solution relies on this. I donāt know how it would ever work without it!
Plan to add the PRās tomorrow.
The answer to the original question is, YES I DO!!!
This was like a months worth of work to track down multiple problems throughout SuiteCRM that caused the general functionality to not work.
Iāve added all the PRās and all the info from my notes about what is required here:
Iāve tested on two different installations and it works. I would love for someone else to give it a try and make sure there are no issues with my PRās. I checked very carefully, but Github for me is a bit of a challenge. So hopefully I did everything mostly right.
This solution will only work in 7.13+ as 7.12 doesnāt have a relationship between security groups and inbound email accounts.
Hereās a video walkthrough of the functionality:
Excellent season finale !
Hey @pgr, one more quick question. Iām just doing a double check on my PRās by adding them to a fresh install of Version 8 for testing. I realized I missed one line in one PR.
For example here: Update SugarFolders.php Re:277 by pstevens71 Ā· Pull Request #281 Ā· salesagility/SuiteCRM-Core Ā· GitHub
I need to add one line in between: 888/889
if (!in_array($a['id'], $groupFolderIds)) {
How do I do that with creating the least amount of mess. (Sorry, Iām still struggling with navigating around Github).
Do I just āedit fileā from the commit?
If you update the file in your own repo, that new commit will appear also in the PR you already created.
The PR relates one branch to another branch, whatever is there, will be included in its latest version.
Great news!!! Just tested and it works on SuiteCRM 8. However, the cache is kind of tricky. I had to put in developer mode, do the bin/cache/clear thing, and then rebuild all the JS stuff in repair and rebuild and flush my browser cache to get it to work.
