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

Allow adding tags to ECK entities #33

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

jensschuppe
Copy link
Collaborator

@jensschuppe jensschuppe commented Mar 9, 2022

This allows adding tags to ECK entities through a tab on the entity's page.

For now, editing tag assignments is allowed for all users allowed to view the tags page (access CiviCRM permission), which is, of course, unsafe. There should be dedicated permissions for editing tags on entities for each ECK entity type. This PR is therefore a draft.

Also, this requires managed entities be reconciled and the menu router be rebuilt.

@jensschuppe jensschuppe added feature this issue represents a completely new feature status:needs work There is code, but it needs additional work before it should be reviewed labels Mar 9, 2022
@jensschuppe jensschuppe added this to the 1.0 milestone Mar 9, 2022
@colemanw
Copy link
Collaborator

colemanw commented Mar 9, 2022

There should be dedicated permissions for editing tags on entities for each ECK entity type

I don't think I agree with this. IMO the permission to view/edit tags on an entity type should be the same as the permission to view/edit entities of that type. So if you have permission to edit an entity of type Foo then you have permission to add tags to it as well. That's how it works with core entities.

But I'm not sure per-entity-type permissions have been implemented? I don't see where that's currently happening, and this is just a fixme:

/**
* @return array
*/
public static function permissions() {
return []; // FIXME: Add per-entity-type permissions
}

Also, this requires managed entities be reconciled and the menu router be rebuilt.

This happens during every upgrade, so it's not something you have to notify users of.

@jensschuppe
Copy link
Collaborator Author

You're correct, @colemanw, the extension does not yet define any permissions, but definitely should; see #34.

I'd be fine with the Edit Foo Entities permission also controlling access to editing tag assignments on entities of that type for now. But the more granular permissions the better, however 🤓. Adding more granular permissions later will cause larger update routines. Also, I can imagine use cases for not letting creators of entities assign them tags. But let's go with what Core does for now.

@jensschuppe
Copy link
Collaborator Author

This will have to be rebased onto #100.

@jensschuppe jensschuppe modified the milestones: 1.0, 1.1 Dec 12, 2023
@jensschuppe
Copy link
Collaborator Author

This needs a rebase now that permissions have been implemented (differently) with #100.

I'm also postponing this to after version 1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature this issue represents a completely new feature status:needs work There is code, but it needs additional work before it should be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants