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

Returning custom field view permission #13741

Merged
merged 3 commits into from
May 13, 2024

Conversation

escopecz
Copy link
Sponsor Member

@escopecz escopecz commented May 10, 2024

Q A
Bug fix? (use the a.b branch) [y]
New feature/enhancement? (use the a.x branch) [ ]
Deprecations? [ ]
BC breaks? (use the c.x branch) [ ]
Automated tests included? [y]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #13602

Description:

This line got removed in https://github.com/mautic/mautic/pull/13321/files#diff-fc1c6863009fa9dbb412ad96ef474537d4e3d72c2ccc90f126e7bd808b0e45a7L64 by accident. Probably due to extensive conflicts when updating that PR and its predecessor to updated target branch.

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Follow the steps from Missing Custom fields view permission #13602

@escopecz escopecz added bug Issues or PR's relating to bugs regression A bug that broke something in the last release labels May 10, 2024
@escopecz escopecz added this to the 5.1.0 milestone May 10, 2024
@escopecz escopecz added the roles Anything related to users and roles label May 10, 2024
Copy link
Sponsor Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked out this PR but I'm still not seeing the view permission for this.

screenshot-mautic ddev site-2024 05 10-14_02_37

@escopecz
Copy link
Sponsor Member Author

Oops, I was so sure I found the problem. There was another line missing. Added that and then updated the existing test that confirms there are 2 permissions for the contact fields now.

@escopecz escopecz requested a review from RCheesley May 10, 2024 13:25
@RCheesley
Copy link
Sponsor Member

RCheesley commented May 10, 2024

Thanks @escopecz - when I try to view the custom fields logged in with a user who has 'view' permission:

screenshot-mautic ddev site-2024 05 10-15_28_04

I don't actually see it in the settings menu at all. I think that I should at least be able to view them?

screenshot-mautic ddev site-2024 05 10-15_26_08

@jos0405
Copy link
Contributor

jos0405 commented May 13, 2024

Custom fields are not visible yet.
image

Copy link
Member

@patrykgruszka patrykgruszka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine 👍

Works well from API perspective - can get all custom fields with both - view or manage permissions.

The problem with accessing Mautic Panel custom fields with view-only permissions is also present in the 4.x version. So this issue is not a regression. In my opinion, it's good to go.

image

@escopecz
Copy link
Sponsor Member Author

Thanks for confirming. Since this fixes the regression described in #13602 I'm merging this to unblock the release of 5.1.0

@escopecz escopecz merged commit f13b8a4 into mautic:5.x May 13, 2024
16 checks passed
@escopecz escopecz deleted the returning-cf-view-permission branch May 13, 2024 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs regression A bug that broke something in the last release roles Anything related to users and roles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing Custom fields view permission
4 participants