Skip to content
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

Create export mappings without admin perms #26262

Merged
merged 1 commit into from
May 19, 2023

Conversation

MegaphoneJon
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/core/-/issues/2435
Import mappings, and AFAICT export mappings before ang/exportui, don't require admin permissions to create. However, the Mapping API required admin permissions, but that's a default that I imagine no one ever changed.

With the advent of ang/exportui, one could no longer save an export mapping without admin permissions. This corrects that.

Note that Coleman created #19733 to make the UI match the permission, intentionally sidestepping the permission question. I can't see any reason not to change the permission though.

Before

Non-admin users don't have a "Save Fields" button in the Export UI.

After

"Save Fields" appears.

Comments

My only "real" change is in CRM/Core/Permissions.php. The other files are reverting Colemans' alternate fix.

@civibot
Copy link

civibot bot commented May 18, 2023

(Standard links)

@civibot civibot bot added the master label May 18, 2023
@eileenmcnaughton
Copy link
Contributor

@MegaphoneJon I agree with this change - but should we target the rc?

@colemanw
Copy link
Member

I'm good with this as well

@MegaphoneJon
Copy link
Contributor Author

I mean - we could target the rc, but this regressed in...5.37? It's not recent at all. I manually confirmed it happened in 5.53.

@eileenmcnaughton
Copy link
Contributor

@MegaphoneJon so I guess my general principle would be to target the rc for anything that is
a) safe and
b) at least somewhat regressive

  • on the principle that we should always be trying to get the least buggy version out

Having said that this is against master & it exists & is mergeable with the click of a button so I'm all good with not creating more work here given the lack of recency

@eileenmcnaughton eileenmcnaughton merged commit 91b4e7b into civicrm:master May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants