-
Notifications
You must be signed in to change notification settings - Fork 112
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
More job details in pm #3700
More job details in pm #3700
Changes from 6 commits
5e28419
99639f4
1ff4142
0850f1c
4b14bd4
f57f02c
f7b5f4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,8 +5,25 @@ | |
|
||
<turbo-stream action="replace" target="<%= id %>"> | ||
<template> | ||
<div> | ||
<%= job.id %> is currently <%= job.status.to_s %>. | ||
<button class="badge <%= status_class(job.status.to_s) %> rounded-pill border-0 btn" type="button" data-bs-toggle="collapse" data-bs-target="#<%= id %>Data"> | ||
<span> | ||
<%= job.id %> <%= status_text(job.status.to_s) %> | ||
</span> | ||
</button> | ||
<div class="collapse" id="<%= id %>Data"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should keep html ids |
||
<div class="card card-body"> | ||
<% readable_job_data(job).each do |category_name, category| %> | ||
<table class="table table-bordered table-sm m-0 mb-2"> | ||
<thead><%= category_name %></thead> | ||
<% category.each do |name, value| %> | ||
<tr> | ||
<td><strong><%= name %></strong></td> | ||
<td><%= value %></td> | ||
</tr> | ||
<% end %> | ||
<% end %> | ||
</table> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK - I think I've got this now. I think this should just be 1 table instead of 3. I think that'll simplify the computation for this object as well. |
||
</div> | ||
</div> | ||
</template> | ||
</turbo-stream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With just 1 table, this should simplify a lot. But in any case, I'm not in love with the giant case statement. Maybe try a lookup table to map the key
job_name
to the new keyJob Name
. You may even get by with a simplemap
tokey.humanize
.Also I'd say put this somewhere as a member of the
HpcJob
class so we can just dojob.to_human_display
or something similar.