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

Replace admin/people with a View #149

Closed
quicksketch opened this issue Jan 5, 2014 · 12 comments · Fixed by backdrop/backdrop#615
Closed

Replace admin/people with a View #149

quicksketch opened this issue Jan 5, 2014 · 12 comments · Fixed by backdrop/backdrop#615

Comments

@quicksketch
Copy link
Member

This admin listing will be dependent upon #142.

Relevant drupal.org issue: https://drupal.org/node/1851086

@tarekdj
Copy link
Member

tarekdj commented Sep 17, 2014

@quicksketch what is the strategie for changing "admin/people" page callback in user_menu() ? We force views module to be required by user module? Or what if views is disabled ?

@rudiedirkx
Copy link

Views isn't mandatory enabled? I never got that in D8... Why do Views in core but still have backups? Who would disable it?

@tarekdj
Copy link
Member

tarekdj commented Sep 17, 2014

In "minimal" install profile Views is not enabled by default. And nothing can prevent user from disabling it since it's not required.

@quicksketch
Copy link
Member Author

We made Views module required (and therefor enabled on all profiles) in #270. The Views UI module is not enabled by default in minimal, but it should still be enabled in the same way that other required modules like File are. If this isn't the case we need to make sure that Views is enabled in these other profiles.

So Views should already be required. Paths like "admin/people" should be provided by a View only, bundled with user.module in core/modules/user/config/views.view.user_admin.json. We can remove the entry from hook_menu() and let the view configuration provide the entry.

@quicksketch
Copy link
Member Author

The PR at backdrop/backdrop#466 looks like a great start to this issue. There have been some changes to VBO/actions to allow us to convert these administrative listing to Views. In particular we've made it so that $context is passed by reference and $context['redirect'] may be set to redirect the user to another page. We used this for mass node deletion, and we'll need to use it for user account cancellation as well in this issue. Overall, it's looking great though!

@quicksketch
Copy link
Member Author

I'm going to pick this up to rebase it and add the missing update hook.

@quicksketch
Copy link
Member Author

We'll also need to add the ability to bulk cancel user accounts, reusing the existing form for choosing cancellation method.

@quicksketch
Copy link
Member Author

I've got this coming along nicely now, rebased and added an action for user cancellation. We don't yet have an action for adding/removing roles, so I'm working on adding that.

@quicksketch
Copy link
Member Author

I think the PR at backdrop/backdrop#615 is now ready to go. I've tested it on clean installs and upgrades. All automated tests updated.

  • Added new actions for cancel, add roles, and remove roles.
  • Added update hook.
  • Moved the form for bulk user cancellation from user.module to user.admin.inc.
  • Moved actions into a separate user.actions.inc.
  • The "file" option for hook_action_info() wasn't working, so I corrected it.
  • The list of actions was unsorted, so I added a call to backdrop_sort() to order by weight.
  • Removed the option to filter by permission name.
  • Added an option to filter/search by user name.

After adding the username search, the list of fields was getting pretty cluttered. The permission filter I've never found useful, so I removed it from the view to clean up the UI. @jenlampton mentioned to me that another problem with the permission filter is that in many cases the people responsible for administering user accounts are not responsible for configuring site permissions, so the permission filter doesn't have much utility to them because they don't know what the permissions do relative to their site. The exposed filter for permission search can still be added by the end-user if they require that functionality, but removing it gives us a cleaner UI with the most common options available.

@klonos
Copy link
Member

klonos commented Dec 30, 2014

...the people responsible for administering user accounts are not responsible for configuring site permissions

I hear you. Can't wait for #313

@quicksketch
Copy link
Member Author

PR is passing now! It took me a while to rework the test for the bulk operations, since VBO uses the row number and not the UID for its checkbox element names.

@klonos
Copy link
Member

klonos commented Jan 4, 2015

One more admin listing converted. Yeahee!!!

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

Successfully merging a pull request may close this issue.

5 participants