search

ripienaar / gdash

A dashboard for Graphite
http://www.devco.net/
Apache License 2.0
754 stars 117 forks source link

Fix NoMethodError when graph_columns is set to 1 #79

Closed bookest closed 11 years ago

bookest commented 11 years ago

Setting :graph_columns: to 1 in gdash.yaml caused a NoMethodError to be thrown when viewing a dashboard. The problem is that the monkey patched Array#in_groups_of method returns the unmodified array when called as array.in_group_of(1), while clients are expecting to get an array of 1 element arrays.

keymon commented 11 years ago

+1 I agree we should merge this patch, as the function should be consistent in return type.

But I was not able to reproduce the exception, can you paste it? Which ruby version are you using?

Also, I found that the function in_groups_of is not used anymore in the main views (dashboard.erb and detailed_dashboard.erb) since the commit c424a587027f000910a8262ae46dfc05557ec79d.

It is still being used in the print and full views. I think we should refactor all of them to keep only one unique .erb.

bookest commented 11 years ago

Seems I hadn't pulled in the latest master, so I was missing c424a58. I'm no longer able to reproduce the Exception with the updated dashboard.erb. Sorry about that, I thought I had verified this on the latest master before opening the PR, but obviously I screwed up somewhere.

keymon commented 11 years ago

It is OK, actually thanks for the PR. I think that there is a bug, but the code that was being affected not used anymore. There were a lot of merges last days.

bwhaley commented 11 years ago

I refactored the print and full views to remove the dependency on in_groups_of, then removed in_groups_of from monkey patches. bwhaley/gdash@aa50334f0015438f4708a0cc19cbf0047040c405

keymon commented 11 years ago

@bwhaley nice, but can you add this css in the print*dashboard.erb. If not it will show only one column (class=row and this stuff)

<link rel="stylesheet" type="text/css" href="<%= @prefix %>/bootstrap/bootstrap.css">

Thx!

bwhaley commented 11 years ago

Done and submitted as a new PR https://github.com/ripienaar/gdash/pull/82. Suggest we close this PR since it is no longer relevant. (Thanks though, bookest!)