Show "reporting user" on "issues" screen

[nertc]

This PR addresses "Show 'reporting user' on 'issues' screen" issue mentioned in the #2273

On the issues page, there was added a new column called "Reporter Users". It will show unique reporters of the item. If there are more than reporters for the given item than the size of the block can handle, ellipsis will be shown at the end of the line. Each of user display name will have a link to the user. Fixes #2273

Update:
Added fixed max-width for reporters' column. If the text can't be fitted, ellipsis will be shown. On hover, box shows all the reporters.

Screenshot:

[tomhughes]

Although this is described as showing the "last three unique reporters" there isn't actually any ordering that I can see, and ordering on time is hard to combine with uniqification...

[AntonKhorev]

Currently the ignored issues page takes 10 seconds to load and the resolved issues won't load at all for me. If the queries are to be any heavier, it's going to get even worse.

[nertc]

@tomhughes Thanks for the review. I've updated according to your feedback. Tooltip is a good idea for giving more information, but I have a question about, if users get the information correctly and won't be confused, because when a user hovers on a profile link, tooltip will show all the reporters (current solution) and he might be confused if the link redirects him to the specific user or some list of users. If we add a tooltip only to the places with no link, user will have to hover either semicolons or ellipsis, which are hard to target because of their small size.

[nertc]

@AntonKhorev If the issues page is already slow, we can temporarily archive this PR and reopen it after optimization.

[nertc]

@AntonKhorev About the issues page loading time, I am interested, what is the average number of issues displayed, when the page is loaded. If the loading problem arises because of the great number of issues, we can add infinite scroll (load-on-scroll) functionality or pagination to resolve it. If either solution is okay, I'll create a separate PR for it.

[AntonKhorev]

what is the average number of issues displayed, when the page is loaded.

There's an acceptable number of open issues because they are being dealt with, but once that happens they become either resolved or ignored, those numbers keep growing. Total number of issues is currently at ~30000, that means 2000..20000 issues on ignored/resolved pages for moderator/administrator queues.

If the loading problem arises because of the great number of issues, we can add infinite scroll (load-on-scroll) functionality or pagination to resolve it.

We can, if we want to trade it for another problem of making searches more difficult. See #3105 for a similar issue.

[AntonKhorev]

Intermediate solution would be showing reporting users only on open issues pages.

Or maybe on any page but only if the number of issues is below some threshold.

[nertc]

@AntonKhorev What do you mean with making searches more difficult? From the user experience perspective, infinite scroll doesn't add any difficulties or additional clicks to see issues. If you mean searching with a browser built-in search functionality (because it needs all the issues to be rendered to work) and admins/moderators are using it, then we have to change or enhance our search so that browser built-in search wouldn't be needed. This way we will both optimize the page and make the user experience more website oriented.

[nertc]

As #5057 is merged, this PR was updated to match pagination changes of the issues page.

In terms of performance, on the local machine I have 1000 users and 1000 issues, and pagination time was increased only by 150-200ms (it should be similar increase in the real environment, as before for the #5057 I tried to match up timing of the local and real environments).

All the Ruby variable initializations and calculations (from the previous solution) were moved from issues/index.html.erb file to issues_controller.rb to make html files less filled with calculation code.

Also, as we are using Bootstrap Tooltips in other places, turbo_tooltip.js file was created. Bootstrap Tooltips Documentation mentions:

... you must initialize them yourself

Tooltips must be hidden before their corresponding elements have been removed from the DOM.

Because of these limitations, it has some bugs when working with Turbo (tooltips are not closing, tooltips got frozen on the top-left side of the screen ...). turbo_tooltip.js initializes Bootstrap Tooltips, solves the problem that caused tooltips bugs and also is reusable to be used in the future everywhere, where Turbo will be used and tooltips will be needed (Diary Entries page, Diary Entry Comments, User Blocks page ...).

[AntonKhorev]

I don't think tooltips were a good idea. You can't interact with the usernames they show. And truncation works fine for individual names, but then you'd have to do it inside the tooltip too. Do you want to make html tooltips? And then there's Turbo cleanup.

[AntonKhorev]

I wouldn't try to do a comma-separated list

UserA, UserBBBBBBBBBBBBBBBBBB, UserC

I'd write them each on a new line and add truncation

UserA
UserBBBBBBBBB...
UserC

And if there's more than say 3, write something like

UserA
UserBBBBBBBBB...
UserC
and 2 more users

[nertc]

And if there's more than say 3, write something like

UserA
UserBBBBBBBBB...
UserC
and 2 more users

@AntonKhorev do you mean to make this kind of truncation in the tooltip?

[AntonKhorev]

I mean let's not have tooltips at all. This is what a table cell contents may look like:

UserA
UserBBBBBBBBB...
UserC
and 2 more users

"and 2 more users" initially can be just a passive text, later it can bechanged to some js expansion or turbo streams rewrite.

[nertc]

@AntonKhorev It will make Reporter Users cells too high which will cause additional whitespaces in other cells.

[AntonKhorev]

It will make Reporter Users cells too high which will cause additional whitespaces in other cells.

Is that additional whitespace going to be a problem?

[nertc]

It will make Reporter Users cells too high which will cause additional whitespaces in other cells.

Is that additional whitespace going to be a problem?

Sorry for the late answer. I think it will be a problem in terms of UI and UX:
UI - visually, additional whitespaces in the table make cells feel disconnected from each other and also create an impression that something is missing in the cell (something went wrong)
UX - as there will be additional whitespaces, they'll make page longer and, therefore, harder to scroll down. In addition to this, big whitespaces in cells generally make information harder to read (instead of regular left-to-right or right-to-left reading, user has to read every block of information top-to-bottom)

[AntonKhorev]

instead of regular left-to-right or right-to-left reading, user has to read every block of information top-to-bottom

Which blocks are "every"? Reporter user lists? Aren't lists regularly read top-to-bottom?

[nertc]

@AntonKhorev I'll update this PR to have vertical list of Reporting users. Do you also suggest to completely remove tooltips? I think tooltips are quite handy even if user can't click on the profile links (to just take a glance at reporting users' names), but it introduces additional complexities (which I don't know if are worth it, though, I think, as these complexities are created as reusable code, it will benefit website code in future too)

[AntonKhorev]

I think tooltips are quite handy even if user can't click on the profile links (to just take a glance at reporting users' names)

Can't you don this already by looking at the link location that the browse shows?

[nertc]

I think tooltips are quite handy even if user can't click on the profile links (to just take a glance at reporting users' names)

Can't you don this already by looking at the link location that the browse shows?

If there is no tooltip on the issues, which has more than 3 reporters, users won't be able to take a look at reporters full list without opening the issue page.

[AntonKhorev]

If there is no tooltip on the issues, which has more than 3 reporters, users won't be able to take a look at reporters full list without opening the issue page.

For now they won't. But then we'll make it clickable to show the rest of the users. This should work better than a tooltip because it's usable with touch interfaces too.

[nertc]

PR was updated. Bootstrap tooltips were removed. Instead of the horizontal view of reporters list, now it is vertical and after 3 users it shows "and X more". I tried aligning data of the table vertically middle, but top one was visually more appealing. This is screenshot of how it looks now:

[AntonKhorev]

Are reporters sorted? Judging by #4990 (review) there was an attempt to show last three unique reporters, but now looks like it's not happening.

If there's an issue that was resolved, it has read_reports. Then the same item may get reported again and get new unread_reports. If you look at the issues list, reporters from read_reports are not as important and maybe shouldn't be shown at all. Or maybe they should, but only if the status filter is set to resolved (otherwise there's no available reports because they all are read).

But all of those considerations could be skipped (at least initially) if we sort reporters by their last reports. Unread reports always come chronologically after read reports, therefore last reporters will come from unread reports first.

[nertc]

Sorting reporters by their last reports is a good idea. I'll do it and update PR accordingly. I think, after this PR is merged then, if needed, we can enhance "reporters" functionality by adding some filters to it. As reports and issues are different tables, adding filters to reports may cause some additional performance and UX difficulties. I think, it deserves a separate PR.

[nertc]

PR was updated. Reporters are now sorted according to their last report creation datetime in descending order. Tests were also added to test sorting functionality.

[AntonKhorev]

Suggested change

	1.upto(5).each do |n|
	issue.reports << create(:report, :user => create(:user, :display_name => "Test Name #{n}"))
	end
	create_list(:report, 5, :issue => issue)

because you're not checking if usernames match "Test Name #{n}".

And probably the same in the next test.

[nertc]

Thanks for the suggestion. Both tests were changed to match smaller list creation you suggested.

[AntonKhorev]

Although it can still be optimized, let's start with this.

[tomhughes]

The phrase "reporting user" as in the title of this PR makes more sense than "reporter user" that is actually used...