Make WordPress Core

Opened 8 years ago

Last modified 5 years ago

#33967 assigned enhancement

MS Sites: content of the users column should be by choice, number is not too informative

Reported by: katazina's profile katazina Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.3
Component: Networks and Sites Keywords: has-patch 2nd-opinion
Focuses: multisite Cc:

Description

I sadly noticed, that on network admin -> sites, the users column only shows numbers now.
It looks nicer with the less data, but on the other hand we used that information.

I ask, it is possible to make it choosable, or at least, list the users in the excerpt listing mode?

Why I ask this:
We have a multisite install with many blogs and open registration. Because of that we sometimes have spam blogs.
We frequently delete those spam blogs with its user(s), but now it's more complicated, because we don't see the email (user) at first. We have to open it on a new window (by clicking on the number of users).
That slows down the process.
And in some cases, just from the registered user (email) we saw if it was a spam blog or not.

Attachments (8)

33967.1.patch (4.2 KB) - added by Mista-Flo 7 years ago.
Display first username and link to all users of a site in sites admin page
33967.2.patch (7.5 KB) - added by Mista-Flo 7 years ago.
Properly get the first user of the blog (order by registered date)
33967.3.patch (13.6 KB) - added by Mista-Flo 7 years ago.
Fix count value and handle excerpt mode
33967.4.patch (5.3 KB) - added by Mista-Flo 7 years ago.
Just a single diff instead of all commits
33967.5.patch (5.1 KB) - added by Mista-Flo 7 years ago.
Refresh : Add consistency and <br> tags
33967.6.patch (5.3 KB) - added by Mista-Flo 7 years ago.
Add a notice if there is no user on the blog
33967.7.patch (2.5 KB) - added by PieWP 7 years ago.
Improved performance
33967.8.patch (5.4 KB) - added by Mista-Flo 7 years ago.
Fix patch with enhanced performance

Download all attachments as: .zip

Change History (35)

#1 @katazina
8 years ago

Related ticket: #32434

Last edited 8 years ago by johnbillion (previous) (diff)

#2 @johnbillion
8 years ago

  • Keywords needs-patch ui-feedback added
  • Milestone changed from Awaiting Review to 4.4
  • Type changed from feature request to enhancement

I noticed this too. This is actually a detrimental change.

#3 @jeremyfelt
8 years ago

For more context, the before image originally posted on #22383 when this work started.

I think the summary of the decision via Slack was that listing only the first 5 users wasn't great info to have.

I'd be in favor of having the expanded view return additional data. Should we go back to just first 5 or would something else be more useful?

#4 follow-up: @katazina
8 years ago

I would show only one user, who's registered the blog and if there's more, then show it like that:

username
(6 more)

1 + 6 can be easily add together :) so that way we will see the user and the number of the users if there is more than one.
Win win.

For me it is not important for the username to be a link. I don't know what others think about that.

#5 @katazina
8 years ago

But that could be done in the list mode, it's not much data and either whay the row will be at least three line tall, because of the links which are pop up if you hover over the url column.

#6 in reply to: ↑ 4 ; follow-up: @helen
8 years ago

Replying to katazina:

I would show only one user, who's registered the blog and if there's more, then show it like that:

username
(6 more)

I like the general idea - not 100% sure about the (6 more) part, perhaps some more specific language would work better.

#7 in reply to: ↑ 6 ; follow-up: @katazina
8 years ago

Replying to helen:
I think if someone uses the ms sites page, than she/he will know what that (6 more) means.
But we also could use a title attribute or js hint (bubble) on mouseover.

#8 in reply to: ↑ 7 @helen
8 years ago

Replying to katazina:

But we also could use a title attribute or js hint (bubble) on mouseover.

Neither of those are options - touch devices, people using other navigation methods. I meant a very brief + 6 more users kind of string so it has context without having to refer all the way back to the header.

#9 @katazina
8 years ago

That's good, I tought you thinking about a more longer string.

#10 @johnbillion
8 years ago

Showing the first five users is actually really handy on smaller networks, especially while multisite doesn't have a means of bulk adding a user to many sites.

I wonder if we can re-purpose excerpt mode (see #29536) to display more information on this screen.

#11 @DrewAPicture
8 years ago

@jeremyfelt Thoughts on this getting to a ready state by 4.4 Beta 1 (Oct 21)?

#12 @jeremyfelt
8 years ago

It seems like we have a direction, just need a patch to play with.

My read:

  • Keep list view as is for # of sites.
  • Add an excerpt view displaying first X users and a link to "+ # more users" similar to what the list view was before 4.3.
Last edited 8 years ago by jeremyfelt (previous) (diff)

#13 @katazina
8 years ago

I think it's a good way.
List mode stays simple, and the excerpt mode will have more information.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


8 years ago

#15 @wonderboymusic
8 years ago

  • Owner set to jeremyfelt
  • Status changed from new to assigned

#16 @jeremyfelt
8 years ago

  • Milestone changed from 4.4 to Future Release
  • Owner jeremyfelt deleted

We need a patch to progress on this. Bumping to future release, but we can pick this up again in 4.5.

@Mista-Flo
7 years ago

Display first username and link to all users of a site in sites admin page

#17 @Mista-Flo
7 years ago

  • Keywords dev-feedback has-patch added; needs-patch removed

Hi,

I have made a patch for this ticket, it does those things :
-Display the username of the first user of the site
-If there are other users in this site, display a link to those users and the number of users
-in excerpt mode : Display username of 10 first user of the site and a see more users link

My patch is not finished yet, it needs some help :
-First, about the changes I have made in the code, have we got a better option for performance purpose about the get_sites func call I made every request instead of the previous behavior with 12 hours of cache ?
-Second, the wording "+ %s more users" (where %s is the number of users) is still good for you ?

Last edited 7 years ago by Mista-Flo (previous) (diff)

@Mista-Flo
7 years ago

Properly get the first user of the blog (order by registered date)

@Mista-Flo
7 years ago

Fix count value and handle excerpt mode

#18 @Mista-Flo
7 years ago

  • Keywords 2nd-opinion added

Hey @jeremyfelt I will be happy to have your opinion about my patch. I wrote some questions in this comment : https://core.trac.wordpress.org/ticket/33967#comment:17

@Mista-Flo
7 years ago

Just a single diff instead of all commits

@Mista-Flo
7 years ago

Refresh : Add consistency and <br> tags

@Mista-Flo
7 years ago

Add a notice if there is no user on the blog

This ticket was mentioned in Slack in #core-multisite by florian-tiar. View the logs.


7 years ago

@PieWP
7 years ago

Improved performance

#20 @PieWP
7 years ago

Performance would be killing on major blogs with many users. Thats why the transient was included.
Suggested patch .7 to show how you can improve the database fetching by only requesting minimal required fields.

Don't know why the core developers didn't use the count_total paramater in the first place.

@Mista-Flo How would you suggest switching the $mode from the UI?

#21 @Mista-Flo
7 years ago

@PieWP Thank you for your enhancement :)

The call of get_users can't be cached or put in a transient ? It's nice to have selected only required fields.

I see in your patch that you don't handle case of list mode, the else statement only return the number count, we have at least to display the first user of the site.

By the way, I prefere to remove complexity by stop execution of the function if there are no users in the site :

<?php
if ( empty ( $user_count ) ) { 
    printf( __( 'There are no users in this site' ) ); 
    return;
} 

// Do something

instead of a else if and else statement.

Last edited 7 years ago by Mista-Flo (previous) (diff)

#22 @PieWP
7 years ago

The call of get_users can't be cached or put in a transient ? It's nice to have selected only required fields.

Well you can but that would probably be overkill. After all you always want to show the last registered users; using fresh/live data.

I see in your patch that you don't handle case of list mode, the else statement only return the number count, we have at least to display the first user of the site.

Is that logic behaviour when you only want to display a count? (so specifically in count modus)

By the way, I prefere to remove complexity by stop execution of the function if there are no users in the site. Instead of a else if and else statement.

Depending on the code structure I totally agree with you. In this case I though I have no preferred way; either way is fine.

@Mista-Flo
7 years ago

Fix patch with enhanced performance

#23 follow-up: @Mista-Flo
7 years ago

  • Keywords ui-feedback dev-feedback removed

@PieWP I fixed your patch, thanks for your help. I guess we have a ready patch now :)

Last edited 7 years ago by Mista-Flo (previous) (diff)

#24 in reply to: ↑ 23 @PieWP
7 years ago

Replying to Mista-Flo:

@PieWP I fixed your patch, thanks for your help. I guess we have a ready patch now :)

Yeah looks fine :) Although I still do not quite get how the $mode global gets set. Should an additional screen option be included in the patch file as well?

#25 follow-up: @Mista-Flo
7 years ago

@PieWP For the $mode global, I have just looked at the other columns that use it, I was previously checking the $_GET var, but I prefere to follow existing code.

I don't understand your question about additional screen option ? What do you have in mind ?

#26 in reply to: ↑ 25 @PieWP
7 years ago

Replying to Mista-Flo:

@PieWP For the $mode global, I have just looked at the other columns that use it, I was previously checking the $_GET var, but I prefere to follow existing code.

I don't understand your question about additional screen option ? What do you have in mind ?

Sorry thought you introduced the global (due to newline in the patch file). Taking a closer look I now see the 'excerpt' switch mode on the right side of the table (between the site search and the table)

This ticket was mentioned in Slack in #core-multisite by florian-tiar. View the logs.


7 years ago

Note: See TracTickets for help on using tickets.