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

Fixes #20230 - Add time ended to error tasks widget #263

Merged
merged 1 commit into from
Jul 11, 2017

Conversation

tbrisker
Copy link
Member

@tbrisker tbrisker commented Jul 6, 2017

Also made the table fixed with tooltips on overflow.

<th class="col-md-5"><%= _("Name") %></th>
<th class="col-md-2"><%= _("State") %></th>
<th class="col-md-2"><%= _("Result") %></th>
<th class="col-md-3"><%= _("Ended at") %></th>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to drop the 'at' from the header or reword it some other way? Ended at: 5 minutes ago feels weird, but maybe it's just me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just Ended should work fine, or?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to Ended

Copy link
Contributor

@adamruzicka adamruzicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, APJ

@iNecas iNecas merged commit 5e4a79b into theforeman:master Jul 11, 2017
@iNecas
Copy link
Member

iNecas commented Jul 11, 2017

Thanks @tbrisker

@iNecas
Copy link
Member

iNecas commented Jul 11, 2017

and @adamruzicka for review

@beav
Copy link

beav commented Jul 11, 2017

(sorry for the post-merge review, I didn't catch the issue til just now 🎣 )

If a task is in paused state, it won't have an ended_at and the widget will error. I had to use this line instead:

<td><%= task.ended_at ? _('%s ago') % time_ago_in_words(task.ended_at) : '' %></td>

Other than that, the change looks good.

@tbrisker
Copy link
Member Author

@beav thanks for the catch, opened #266

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

Successfully merging this pull request may close these issues.

5 participants