Fixes #34515, #34516 - Add host owner and comment on host list page #9182

pkoprda · 2022-04-07T12:41:08Z

Add two columns in host list page - owner of the host and host comment

theforeman-bot · 2022-04-07T12:41:10Z

ares · 2022-04-08T15:51:11Z

I've tried to apply this to get the screenshot from the instance with real data. I encountered the following issue though, let me know once fixed, I'm happy to retest.

2022-04-08T17:49:33 [W|app|b191f852] undefined method `truncate' for nil:NilClass
2022-04-08T17:49:33 [I|app|b191f852] Backtrace for 'undefined method `truncate' for nil:NilClass' error (ActionView::Template::Error): undefined method `truncate' for nil:
NilClass
 b191f852 | /usr/share/foreman/app/views/hosts/_list.html.erb:47:in `block in _cafb7050f0bc78f5d174aa704ac89300'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/relation/delegation.rb:87:in `each'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/activerecord-6.0.3.7/lib/active_record/relation/delegation.rb:87:in `each'
 b191f852 | /usr/share/foreman/app/views/hosts/_list.html.erb:23:in `_cafb7050f0bc78f5d174aa704ac89300'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/base.rb:274:in `_run'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/template.rb:185:in `block in render'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/notifications.rb:182:in `instrument'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/template.rb:385:in `instrument_render_template'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/template.rb:183:in `render'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/deface-1.5.3/lib/deface/action_view_extensions.rb:43:in `render'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/renderer/partial_renderer.rb:357:in `block in render_partial'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/renderer/abstract_renderer.rb:88:in `block in instrument'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/notifications.rb:180:in `block in instrument'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/activesupport-6.0.3.7/lib/active_support/notifications.rb:180:in `instrument'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/renderer/abstract_renderer.rb:87:in `instrument'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/renderer/partial_renderer.rb:346:in `render_partial'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/renderer/partial_renderer.rb:317:in `render'
 b191f852 | /opt/theforeman/tfm/root/usr/share/gems/gems/actionview-6.0.3.7/lib/action_view/renderer/renderer.rb:65:in `render_partial_to_object'

ofedoren

Thanks, @pkoprda! Some comments/suggestions inline. Also, I've noticed that a user doesn't have to actually hover over the icon to see the full comment, which is... just weird :) Moreover, when the full text is shown, I don't have much time to read it, so not sure if it's useful for long descriptions, maybe we could disable autofading? Here is a gif:

ofedoren · 2022-04-08T16:08:23Z

app/views/hosts/_list.html.erb

        <td class="hidden-tablet hidden-xs"><%= label_with_link host.hostgroup, 23, @hostgroup_authorizer %></td>
        <td class="hidden-tablet hidden-xs ellipsis"><%= last_report_column(host) %></td>
+        <td class="hidden-tablet hidden-xs" title="<%= host.comment.truncate(255) %>"><%= icon_text('comment', '') unless host.comment.empty? %></td>


Suggested change

<td class="hidden-tablet hidden-xs" title="<%= host.comment.truncate(255) %>"><%= icon_text('comment', '') unless host.comment.empty? %></td>

<td class="hidden-tablet hidden-xs" title="<%= host.comment&.truncate(255) %>"><%= icon_text('comment', '') unless host.comment.empty? %></td>

You can use safe navigation in Ruby, so in case there is no comment for the host, it won't raise an exception. Probably this why the tests are failing.

Thanks @ofedoren for suggestion. I am not sure how to fix that autofading, because in my environment seems like that it does not fade. So when I hover over the icon, the tooltip stays there until I move away my cursor.

ofedoren · 2022-04-08T16:25:38Z

app/views/hosts/_list.html.erb

        <td class="hidden-tablet hidden-xs"><%= label_with_link host.hostgroup, 23, @hostgroup_authorizer %></td>
        <td class="hidden-tablet hidden-xs ellipsis"><%= last_report_column(host) %></td>
+        <td class="hidden-tablet hidden-xs" title="<%= host.comment.truncate(255) %>"><%= icon_text('comment', '') unless host.comment.empty? %></td>


Since we use icon only and we cannot shrink the column a bit more, maybe it's better to actually put text there? Truncated, of course, and leave the full text to be shown on hover.

Or at least we could center the icon like with Power column.

app/views/hosts/_list.html.erb

ofedoren · 2022-04-11T18:30:20Z

Thanks for the updates. What do you think about moving comment to be the last column in the table? I think not so many hosts have comments, thus we could have an empty column between other relevant information. Moreover, this table is expandable, so having an empty column in the middle is not so... attractive. And the issue with autofading still persists for me :sad_panda:

Also, I've played a bit and I guess a simple centered icon looks better:

But I'd better ask more competent people regarding this: @MariaAga, @Ron-Lavi, @ares

Ron-Lavi

Nice addition @pkoprda !
I tend to agree with @ofedoren about moving the comment column to be last (after the render_pagelets_for extension points)

ares

Adding a screen of both versions presented here from the system with more data:

I like the second option more. I also agree we should do something about the auto-hiding of the tooltip. Ideally it should still until hover out. If that's not possible, I'd consider clicking the icon to open the modal.

@MariSvirik any thoughts? We're adding two new columns here, the host owner and the comment.

app/views/hosts/_list.html.erb

Ron-Lavi

Looks good to me, @ofedoren any other thoughts on this?

ofedoren · 2022-04-26T14:31:57Z

[test katello]

Ron-Lavi · 2022-04-28T12:38:05Z

Thanks @pkoprda @ofedoren !

theforeman-bot added Waiting on contributor Needs testing Not yet reviewed labels Apr 7, 2022

pkoprda changed the title ~~Fixes #34515, #34516 - Add host owner and comment on host list page~~ Fixes #34515, #34516 - Add host owner and comment on host list page Apr 7, 2022

pkoprda force-pushed the host-owner branch from 1b8cfda to f94b2fc Compare April 7, 2022 13:08

theforeman-bot removed the Waiting on contributor label Apr 7, 2022

ofedoren reviewed Apr 8, 2022

View reviewed changes

pkoprda force-pushed the host-owner branch from f94b2fc to c05cd96 Compare April 11, 2022 11:25

Ron-Lavi reviewed Apr 12, 2022

View reviewed changes

MariaAga assigned ofedoren Apr 13, 2022

ares reviewed Apr 13, 2022

View reviewed changes

app/views/hosts/_list.html.erb Outdated Show resolved Hide resolved

pkoprda added 2 commits April 13, 2022 16:22

Fixes #34515 - Add owner of the host on host list page

b5fa3a9

Fixes #34516 - Add host comment on host list page

6ee0023

pkoprda force-pushed the host-owner branch from c05cd96 to 6ee0023 Compare April 13, 2022 14:23

Ron-Lavi approved these changes Apr 18, 2022

View reviewed changes

theforeman-bot removed the Not yet reviewed label Apr 18, 2022

Ron-Lavi merged commit ba18463 into theforeman:develop Apr 28, 2022

pkoprda deleted the host-owner branch May 4, 2022 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #34515, #34516 - Add host owner and comment on host list page #9182

Fixes #34515, #34516 - Add host owner and comment on host list page #9182

pkoprda commented Apr 7, 2022

theforeman-bot commented Apr 7, 2022

ares commented Apr 8, 2022

ofedoren left a comment

ofedoren Apr 8, 2022 •

edited

Loading

pkoprda Apr 11, 2022

ofedoren Apr 8, 2022

ofedoren commented Apr 11, 2022

Ron-Lavi left a comment

ares left a comment

Ron-Lavi left a comment

ofedoren commented Apr 26, 2022

Ron-Lavi commented Apr 28, 2022

	<td class="hidden-tablet hidden-xs" title="<%= host.comment.truncate(255) %>"><%= icon_text('comment', '') unless host.comment.empty? %></td>
	<td class="hidden-tablet hidden-xs" title="<%= host.comment&.truncate(255) %>"><%= icon_text('comment', '') unless host.comment.empty? %></td>

Fixes #34515, #34516 - Add host owner and comment on host list page #9182

Fixes #34515, #34516 - Add host owner and comment on host list page #9182

Conversation

pkoprda commented Apr 7, 2022

theforeman-bot commented Apr 7, 2022

ares commented Apr 8, 2022

ofedoren left a comment

Choose a reason for hiding this comment

ofedoren Apr 8, 2022 • edited Loading

Choose a reason for hiding this comment

pkoprda Apr 11, 2022

Choose a reason for hiding this comment

ofedoren Apr 8, 2022

Choose a reason for hiding this comment

ofedoren commented Apr 11, 2022

Ron-Lavi left a comment

Choose a reason for hiding this comment

ares left a comment

Choose a reason for hiding this comment

Ron-Lavi left a comment

Choose a reason for hiding this comment

ofedoren commented Apr 26, 2022

Ron-Lavi commented Apr 28, 2022

ofedoren Apr 8, 2022 •

edited

Loading