-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
dev/core#4498 Ensure that users without administer civicrm permission… #27071
Conversation
… can use the Mailing Autocomplete
Thank you for contributing to CiviCRM! ❤️ We will need to test and review the PR. 👷 Introduction for new contributors
Quick links for reviewers |
The issue associated with the Pull Request can be viewed at https://lab.civicrm.org/dev/core/-/issues/4498 |
This looks right to me. @MegaphoneJon does this solve it on your end? |
@seamuslee001 @colemanw I just did an r-run and it fixes the issue. Though I did notice that the widget displays unsent mailings as an option, which is neither consistent with past behavior nor (imo) desirable. I'll see if I can write a patch. |
On review, it seems like a pretty intentional decision to include unsent mailings. @colemanw I'm not gonna do a patch unless you think otherwise. |
I don't have a strong opinion on it, but I can see valid cases for including unsent mailings:
You could just exclude the group, but if you're working with mailing recipients for some reason, sometimes people schedule all mailings, and send them all on the same day, so it can be useful to not exclude drafts (my example happened to have an easy workaround, but sometimes there isn't). (I remember a while back, it was not possible to clone/copy a draft mailing, for example; this seems similar) |
Also chiming in to say that excluding unsent mailings is desirable (and has been the past behaviour, as far as I know). |
I'm going to merge this based on the fact it fixes the hard fail but will open a gitlab in regards to the unsent mailings question |
… can use the Mailing Autocomplete
Overview
This fixes a problem where the mailing recipients autocomplete does an EntitySet.get apiv4 call and that requires administer civicrm because the permissions function doesn't correctly modify the get parameter
Before
Non Admins cannot use the CiviMail Recipients lookup
After
Non Admins can use the CiviMail Recipients look up
ping @MegaphoneJon @colemanw