-
Notifications
You must be signed in to change notification settings - Fork 358
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
Use the browser's native printing for asynchronously generated saved reports #4201
Conversation
@skateman Cannot apply the following label because they are not recognized: reporting |
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.
Tested. Looks good
ce6e8ae
to
9b25322
Compare
@report = @result.report | ||
@data = @result.get_generated_result(@sb[:render_type]) | ||
# We need the last_run_on time from the original result | ||
last_run_on = MiqReportResult.find(session[:report_result_id]).last_run_on |
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.
does this fetch the whole record and then take a single columnt or does it fetch just the single column from the db?
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.
nice catch, fixed
9b25322
to
2da362c
Compare
Checked commits skateman/manageiq-ui-classic@e4f2ead~...2da362c with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Saved report PDFs are being generated asynchronously due to their size, the most time-consuming operation (except of the PDF generation itself) was the joining the sub-reports together. As we're moving the PDF generation to the client, I modified the asynchronous task to return with the
html_rows
only. This is later being used in the rendering the print view using the@data
instance variable.To have a new window for printing, the
:popup => true
was not effective due to thewait_for_task
so I also had to add an exception for opening PDF reports in a new window. I know that the code is ugly, but it is now consistent with the other printing views in the UI. I will refactor this in the future in a separate PR.@miq-bot add_label pending core, gaprindashvili/no, reporting, enhancement
@miq-bot add_reviewer @martinpovolny
@miq-bot add_reviewer @epwinchell
Depends on: ManageIQ/manageiq#17632
Closes: #4113
https://bugzilla.redhat.com/show_bug.cgi?id=1588072