-
-
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
GL-44 admin price field options on event info page #11923
Conversation
Not sure how we're linking gitlab issues to PRs, so here is the issue: https://lab.civicrm.org/dev/core/issues/44 |
@lcdservices Brian i notice the code is almost the same as https://github.com/civicrm/civicrm-core/pull/11923/files#diff-ca9ad334affe5bb2bf94f5bc1c9b78f5R133 is this being duplicated or is this not working as its too far up the code line? (sorry Brian wrong link before) |
@seamuslee001 visibility can be set at both the field and option level. The code you linked to addresses it at the field level. It was not being handled at the option level. The code added is similar but addresses the option value loop. |
@lcdservices ah ok, have you checked if contribution pages are correctly dealing with both elements the option and the field level? |
@seamuslee001 This issue only addresses events. I did not investigate contribution pages. The event registration form was already being handled properly -- only the event info page was incorrect. |
Since disclosing info we don't want to disclose (discounts for special groups, etc), adding a test would be very wise, would you be able to integrate that in the current tests? (Thinking of this also regarding non-public custom fields, will open up an issue for that) |
@jitendrapurohit seems a little related to one you raised - perhaps you could review this? I think it would be a hard one to write a test for so despite agreeing the benefit I think we could let that slip |
This seems pretty trivial and only affects the display of a page. @seamuslee001 @eileenmcnaughton I think we should merge this. |
Thanks @mattwire |
Overview
Suppress admin visibility price field options in event info page.
Before
Price set field option set to admin visibility were exposed on event info page to unauthenticated users.
After
Admin price field options are suppressed.