Ensure users with the perm "CiviEvent: access CiviEvent " can edit events they have created.

[alifrumin]

Overview

Users with the permission "CiviEvent: access CiviEvent " should be able to edit events they create (regardless of whether they have the permission "CiviEvent: edit all events").

Before

Users with the permission "CiviEvent: access CiviEvent" could create but not edit their events.

After

Users with the permission "CiviEvent: access CiviEvent" can create AND edit their events.

Technical Details

It looks like this commit 5836c35#diff-57f730edc03717d188e539c92cd1a3d9L2047 changed the way that the $createdEvents array was formatted so that the key was the event id and the value was the title of the event while this line https://github.com/civicrm/civicrm-core/blob/5.2.1/CRM/Event/BAO/Event.php#L2126 expects the value to be the event id.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@alifrumin Are you able to add a test?

[alifrumin]

absolutely

[colemanw]

@alifrumin I've checked the code over and your logic looks sound. I see that the function \CRM_ACL_BAO_ACL::group() requires an array of ids not an array keyed by id, so that looks to be the right fix. Once the test is working ping me and I'm happy to merge it.

[eileenmcnaughton]

this may relate #11716

[eileenmcnaughton]

@alifrumin failing test is related

[alifrumin]

Yeah I know, I got distracted by some other work but I hope to work on this test this week

[alifrumin]

When I ran just this test locally, it passed but when I ran all the tests in the tests/phpunit/CRM/Event/BAO folder this test failed because the static $permissions in the CRM_Event_BAO_Event:CheckPermissions function was throwing things off. We changed the static $permissions to use the Civi::$statics[CLASS]['permissions'] as documented by Tim here and unset it in the test here and now the test is passing.

@colemanw could you take a look at this again? It is ready to be merged from our perspective.

[alifrumin]

@eileenmcnaughton thank you for your diligence.

[mattwire]

Hi @alifrumin I've just had a look at this - agree that it is an improvement. However with the introduction of this there are a couple of other issues that come up:

1. Navigation menu links "New Event" and "Manage Events" are only visible if you have "Edit All Events" permission - I think that needs to be changed as part of this PR so they are visible.
2. On the manage events page Event Links->Register Participant requires "CiviEvent: edit event participants" but is not disabled if the user does not have that permission. Can it be disabled please?

[mattwire]

On second thoughts, I think we should merge this now. I'm working on an extension that enhances events functionality and this checkPermission function is a real pain as the caching does not work properly - basically it should be two separate functions as currently if you call it first without an eventId it populates all events, but if you call it first with an eventId it only populates one event and will not cache all events if called a second time as it assumes that the cache has been built - see https://github.com/mattwire/civicrm-core/tree/civievent_permissions for a possible improvement which builds on this PR.

1. and 2. could be done as follow-up PRs.

@eileenmcnaughton @colemanw Are one of you happy to merge this?

[eileenmcnaughton]

Ok merged - any tickets to update?

[alifrumin]

Thank you @mattwire and @eileenmcnaughton! No tickets to update that I know of.

@mattwire 1 and 2 are very good points. If you start pr's for them let me know and I am happy to test them/ If I come into some extra time I will give it a go.