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

Add group members page to templates #844

Merged
merged 6 commits into from Jun 26, 2013
Merged

Add group members page to templates #844

merged 6 commits into from Jun 26, 2013

Conversation

johnmartin
Copy link
Contributor

To edit a group's members you have to go to /group/members/my-group, but there's no link to this anywhere.

@frank0051
Copy link

I can confirm this - I thought they were trying to do away with group permissions when orgs were introduced, but it doesn't seem like it.

@johnmartin
Copy link
Contributor

FYI: I know that the buttons overlap on this... but this will be answered with #890

@ghost ghost assigned seanh Jun 4, 2013
@seanh
Copy link
Contributor Author

seanh commented Jun 13, 2013

This seems wrong to me, because it's different from how the organization page works.

  1. The members tab is shown on /group/foobar, but on an organization's page I have to click on the Admin button first and then the members tab is shown on /organization/edit/foobar.
  2. On the groups page the members tab (including edit and delete buttons) is shown to users who aren't a member of the group
  3. With organizations, if I'm not a member of an org I cannot see a list of who the members are (because I don't get the Admin button). Maybe I can get it via the API, not sure, but not via the UI anyway. I don't know why this is this way for organizations but presumably the decision was made for a reason and it may as well be the same for groups.

@ghost ghost assigned johnmartin Jun 13, 2013
@johnmartin
Copy link
Contributor

@seanh Aha! Good point. I think I misunderstood that point of this issue. I was assuming that we simply wanted the members page back in as of 1.8. But in reality we want a replica of the org admin pages. I'll re-write this, thanks for the feedback.

@johnmartin
Copy link
Contributor

I'm moving this to 2.2 as it's unlikely that'll be able to get this complete for the tomorrows deadline to branch from master. I'll most likely get to this on Tuesday, so we might be able to squeeze into 2.1 anyways, but I don't want this to block 2.1.

Makes group member style pages within the edit section of a group and adds tabs
to all edit pages.
@ghost ghost assigned seanh Jun 25, 2013
@johnmartin
Copy link
Contributor

OK, well... I had time in the end to get this ready for 2.1, so I've tweaked the pull request based on the feedback.

- When editing a member (as opposed to adding a new one):
  - Prefill username field, and make it non-editable
  - Prefill role field with user's current role
  - Add a delete button to the form
  - Label the submit button 'Save' rather than 'Add'
@seanh
Copy link
Contributor Author

seanh commented Jun 26, 2013

I've edited the new group member form to make it more similar to the one for organizations.

There's still a few more differences:

  1. I noticed that the button on the group read page is Edit but on the organization read page it's Admin, which is right?
  2. The organization page title is Edit Organization but the group one is Edit a Group
  3. The organization page title is Members - My Organization - CKAN but the groups one is just My Group - CKAN
  4. The group breadcrumbs are wrong, they're just "/ Group / Edit Group" should be "/ Group / My Group / Edit Group" (or in the case of organization it's Admin not Edit Organization, these should be consistent)
  5. Organization's admin page has a datasets tab group one doesn't

@johnmartin Up to you if you want to fix any of the above in this pull request for 2.1 or not, I'm happy for new 2.2 issues to be made out of them, especially because it looks like the groups templates (and the groups feature generally) need a bit of an overhaul.

I also noticed that the organization read page shows the number of datasets in the sidebar, but the group one doesn't (not related to this pull request, I guess).

It looks like there are a lot of very similar template files for groups and organizations, and by duplicating them random differences like this get in. I wonder if they should be combined into single template files?

@ghost ghost assigned johnmartin Jun 26, 2013
@johnmartin
Copy link
Contributor

@seanh

  1. I noticed that the button on the group read page is Edit but on the organization read page it's Admin, which is right?

"Edit" is right. But #1030 will answer this problem.

  1. The organization page title is Edit Organization but the group one is Edit a Group

Agreed. Fixed.

  1. The organization page title is Members - My Organization - CKAN but the groups one is just My Group - CKAN

Agreed, odd. I've made groups like orgs now.

  1. The group breadcrumbs are wrong, they're just "/ Group / Edit Group" should be "/ Group / My Group / Edit Group" (or in the case of organization it's Admin not Edit Organization, these should be consistent)

Again, like 3.

  1. Organization's admin page has a datasets tab group one doesn't

I've made a 2.2 issue #1049 that incorporates this (and some of the other elements you mentioned). As a side note: I think maybe having a generic template for orgs/groups might be a nice idea.

I've also merged master into this (since there were merge conflicts)

@seanh
Copy link
Contributor Author

seanh commented Jun 26, 2013

@johnmartin The tabs on the group edit page seem broken now? Merge conflict?

@johnmartin
Copy link
Contributor

@seanh Yes, the template block names had changed in master. It's fixed now.

@seanh seanh merged commit 1b22212 into master Jun 26, 2013
@johnmartin johnmartin mentioned this pull request Nov 14, 2013
@vitorbaptista vitorbaptista deleted the 844-group-members branch April 28, 2014 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants