{"action":"created","issue":{"url":"https://api.github.com/repos/SalesforceFoundation/Cumulus/issues/1479","labels_url":"https://api.github.com/repos/SalesforceFoundation/Cumulus/issues/1479/labels{/name}","comments_url":"https://api.github.com/repos/SalesforceFoundation/Cumulus/issues/1479/comments","events_url":"https://api.github.com/repos/SalesforceFoundation/Cumulus/issues/1479/events","html_url":"https://github.com/SalesforceFoundation/Cumulus/issues/1479","id":64106228,"number":1479,"title":"remove \"with sharing\" from core classes that don't need it","user":{"login":"mscholtz","id":931031,"avatar_url":"https://avatars.githubusercontent.com/u/931031?v=3","gravatar_id":"","url":"https://api.github.com/users/mscholtz","html_url":"https://github.com/mscholtz","followers_url":"https://api.github.com/users/mscholtz/followers","following_url":"https://api.github.com/users/mscholtz/following{/other_user}","gists_url":"https://api.github.com/users/mscholtz/gists{/gist_id}","starred_url":"https://api.github.com/users/mscholtz/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mscholtz/subscriptions","organizations_url":"https://api.github.com/users/mscholtz/orgs","repos_url":"https://api.github.com/users/mscholtz/repos","events_url":"https://api.github.com/users/mscholtz/events{/privacy}","received_events_url":"https://api.github.com/users/mscholtz/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":7,"created_at":"2015-03-24T21:36:25Z","updated_at":"2015-03-27T21:12:15Z","closed_at":null,"body":"There are many classes in NPSP that have \"with sharing\" declaration that IMHO don't warrant it, and I humbly suggest that you review where this is really needed (VF controllers, etc)  and where it's not (most trigger code IMHO).  \n\nThis has caused me several problems with writing code that will work w/ NPSP3.  In every case this is code that worked fine w/ NPSP2, and we made expected adjustments, but hard-to-debug permissions errors have gotten in the way (hard to debug because occurring on managed NPSP code).\n\nOne situation was posted on the PUB here (although discussion petered out because I didn't have it diagnosed quite properly): https://powerofus.force.com/0D58000001PuE6G\n\nAnother that just arose was this:\n\n* client has Sharing Settings on opp set to Read Only\n* we have a custom Before Insert trigger on opp that resets the owner\n* an integration (IATS Brickwork) inserts opps from donation forms via a Sites guest user account\n* worked fine in NPSP2\n* in NPSP3, was throwing a \"insufficient access rights on cross-reference ID\" error\n* through copious testing, was able to figure out that the error was happening here: https://github.com/SalesforceFoundation/Cumulus/blob/2d8fc54af04102c9aa84b68380eedd5587d6b95e/src/classes/TDTM_TriggerHandler.cls#L149\n* my understanding is that after the before trigger changes the owner of the opp, the Guest User account no longer has permissions to edit it.  when your trigger fires and calls this class (declared as \"with sharing\") a DML of some sort that touches opps is no longer allowed and throws this err\n* If I comment out the resetting of Owner, or if I change the Sharing Settings to Read/Write, the error doesn't occur\n\nMy understanding of the TDTM_TriggerHandler class is limited, but it doesn't seem like the sort of thing that needs to be \"with sharing\".  Most triggers should just run when the conditions are met, regardless of which user put the data in.\n\nFurthermore, it leads to the kind of errors and difficult debugging that I've been dealing with today.  I think you don't want your users to have to assign additional permissions to user accounts simply to prevent core NPSP  triggers from throwing errors.\n\nThanks! \n@davidhabib @ceiroa "},"comment":{"url":"https://api.github.com/repos/SalesforceFoundation/Cumulus/issues/comments/87093229","html_url":"https://github.com/SalesforceFoundation/Cumulus/issues/1479#issuecomment-87093229","issue_url":"https://api.github.com/repos/SalesforceFoundation/Cumulus/issues/1479","id":87093229,"user":{"login":"ceiroa","id":1894238,"avatar_url":"https://avatars.githubusercontent.com/u/1894238?v=3","gravatar_id":"","url":"https://api.github.com/users/ceiroa","html_url":"https://github.com/ceiroa","followers_url":"https://api.github.com/users/ceiroa/followers","following_url":"https://api.github.com/users/ceiroa/following{/other_user}","gists_url":"https://api.github.com/users/ceiroa/gists{/gist_id}","starred_url":"https://api.github.com/users/ceiroa/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ceiroa/subscriptions","organizations_url":"https://api.github.com/users/ceiroa/orgs","repos_url":"https://api.github.com/users/ceiroa/repos","events_url":"https://api.github.com/users/ceiroa/events{/privacy}","received_events_url":"https://api.github.com/users/ceiroa/received_events","type":"User","site_admin":false},"created_at":"2015-03-27T21:12:15Z","updated_at":"2015-03-27T21:12:15Z","body":"Well, not all. We should not remove them from controllers."}}