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.
1 Like
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:
- Get a list of subscribed folders
- If user clicks on -None- it supposed to get a list of all folders they have access to
- the user should be able to ctrl-click to remove folders they don’t want to see
- when clicking done it should save the current list of subscribed folders.
- The admin should be able to do the same as above, except that the admin can see all group folders and give access to the user this way by selecting which ones the user can see.
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!).
1 Like
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.
1 Like
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! 
3 Likes
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:
- Branch hotfix is SuiteCRM 7.12
- Branch hotfix-7.13 is SuiteCRM 7.13
- SuiteCRM v8.x is in SuiteCRM-Core repo, branch hotfix
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!
4 Likes
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.
2 Likes
Here’s a video walkthrough of the functionality:
5 Likes
Excellent season finale !
1 Like
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.
1 Like
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.
1 Like