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

WIP: Group roles UI #196

Open
wants to merge 32 commits into
base: 8.x-1.x
Choose a base branch
from
Open

WIP: Group roles UI #196

wants to merge 32 commits into from

Conversation

pfrenssen
Copy link
Collaborator

Creating the group roles UI, which in D7 looks like this:

group-roles-d7

The scope of this PR includes:

  • Displaying the roles in an overview.
  • Adding new roles and editing existing ones.
  • The default roles 'non-member' and 'member' are detected as being special and are not editable.
  • Linking to the route to edit permissions, but not implementing this - that is for a separate PR.

This is blocked by #190 and #192 since it builds on the code of these two PRs.

@pfrenssen pfrenssen self-assigned this May 14, 2016
@pfrenssen
Copy link
Collaborator Author

When I try to edit a role I'm getting the following error:

Drupal\og\Exception\OgRoleException: The id cannot be changed. in Drupal\og\Entity\OgRole->set() (line 237 of modules/contrib/og/src/Entity/OgRole.php).

This is likely to be caused by the ID prefixing. See #216.

I also need to look into why the "Delete" link is not showing up. For some reason I don't have the relevant permission, which is weird since I am logged in as UID1.

@pfrenssen
Copy link
Collaborator Author

I fixed the error. Still need to look into the deletion.

@amitaibu
Copy link
Owner

@pfrenssen will you be able to re-pick it up, or prefer to assign to me?

@pfrenssen
Copy link
Collaborator Author

This was something I was doing in my spare time. Once #217 is in this can be completed. I am still planning on completing this, but if you are anxious to work on it, go ahead :)

It will need to be rerolled though, this is ancient code by now.

@amitaibu
Copy link
Owner

Spare time is the best time :)

Go ahead and keep this one, I'll be working on other cool stuff.

@pfrenssen
Copy link
Collaborator Author

#217 is close to completion now, so this can be picked up again soon. It will be a real joy to merge in 2 months of changes :)

@amitaibu
Copy link
Owner

It will be a real joy to merge in 2 months of changes :)

I took the liberty and git merge 8.x-1.x and there were actually not to many merge conflicts - which is probably a good sign.

I'd love to get this in :)

@ArchieDoe
Copy link
Collaborator

Hey guys
Just started dev on that PR.
Do we implementing group-specific roles functionality here, or is this a separate PR?

Thanks!

@ArchieDoe
Copy link
Collaborator

Also, @amitaibu @pfrenssen I got a question - why group admin routes, like, member list, are not part of UI module? Should we move it there for consistency?

@MPParsley
Copy link
Collaborator

We should close this in favor of Gizra#243

@damienmckenna
Copy link
Collaborator

I resolved some merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants