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

Add widget-report component and method to get data #1805

Merged
merged 6 commits into from
Aug 18, 2017

Conversation

ZitaNemeckova
Copy link
Contributor

https://www.pivotaltracker.com/story/show/147760695

@miq-bot add_label wip, dashboards, fine/no

Cloud Intel -> Dashboard -> widget with report

Before/After: Behavior and graphics will be same.

@miq-bot
Copy link
Member

miq-bot commented Aug 2, 2017

This pull request is not mergeable. Please rebase and repush.

@ZitaNemeckova ZitaNemeckova force-pushed the widget_report_component branch from a355ecb to 3c9d22a Compare August 3, 2017 08:22
@ZitaNemeckova ZitaNemeckova force-pushed the widget_report_component branch 2 times, most recently from 9a51552 to 2688ed7 Compare August 3, 2017 09:51
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot add_label enhancement

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Add widget-report component and method to get data Add widget-report component and method to get data Aug 9, 2017
@miq-bot miq-bot removed the wip label Aug 9, 2017

this.$onInit = function() {
$http.get('/dashboard/widget_report_data/' + vm.id)
.then(function(response) { vm.widgetReportModel.content = $sce.getTrustedHtml(response.data.content);})
Copy link
Contributor

@himdel himdel Aug 9, 2017

Choose a reason for hiding this comment

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

  .then(function(response) {
    vm.widgetReportModel.content = $sce.getTrustedHtml(response.data.content);
  })

But.. you're sure about getTrustedHtml, right? (vs trustAsHtml)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So far it wasn't problem. I like getTrustedHtml safety but trustAsHtml will work for "anything". What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, getTrustedHtml is definitely safer, it doesn't seem to remove colors from reports.. let's keep it until someone complains :)

@@ -127,6 +127,12 @@ def start_url
redirect_to start_url_for_user(nil)
end

def widget_report_data
widget = MiqWidget.find(params[:id])
Copy link
Contributor

Choose a reason for hiding this comment

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

rbac

@ZitaNemeckova ZitaNemeckova force-pushed the widget_report_component branch from 8f168da to 16e3979 Compare August 11, 2017 14:00
@miq-bot
Copy link
Member

miq-bot commented Aug 15, 2017

This pull request is not mergeable. Please rebase and repush.

getTrustedHTML returns value that is safe for HTML context or throws an exception
Add spaces to improve HTML readability
def widget_report_data
widget = find_record_with_rbac(MiqWidget, params[:id])
render :json => {:content => widget.contents_for_user(current_user).contents,
:minimized => @sb[:dashboards][@sb[:active_db]][:minimized].include?(params[:id])}
Copy link
Contributor

Choose a reason for hiding this comment

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

bad rebase?

Two methods begin, only one ends :)

@ZitaNemeckova ZitaNemeckova force-pushed the widget_report_component branch from 2af02cd to 9b29df6 Compare August 16, 2017 12:11
@ZitaNemeckova
Copy link
Contributor Author

@himdel Fixed :)

@miq-bot
Copy link
Member

miq-bot commented Aug 16, 2017

Checked commits ZitaNemeckova/manageiq-ui-classic@b175711~...9b29df6 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@himdel himdel merged commit c4ab6f5 into ManageIQ:master Aug 18, 2017
@himdel himdel added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 18, 2017
@ZitaNemeckova ZitaNemeckova deleted the widget_report_component branch September 12, 2017 14:15
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.

3 participants