grafana/grafana — pull request #428 by Tetha (2014-05-20 09:27:02+00:00)
Title: Refactored filterSrv into own object. 

Hi,

sorry for this huge pull request, and there are 1 or 2 more to come :)

So, what am I doing here? I did two things:

First, I reworked the filterSrv to not be a singleton object, but rather a factory which returns a filter object. This makes it easier to reason about the code, since the live cycle of the object is easier to comprehend - the controller creates it, puts it into the scope and once the scope dies, the object dies with it. Furthermore, this enabled the second refactoring and a number of further cleanups down the road.

Second, I moved the filter object from dashboard.current.services.filter to $scope.filter. I was mostly forced to do so, because I needed a reference to the current dashboard in the filter service, but the dashboard itself referred to the filter service and as such, angular.copy crashed with a stack overflow - what a bummer.

Looking at the code further, this made more and more sense as I was doing it, though: Many, many things depended on the dashboard just for the filter service, and moving the filter service into an own object in the scope allowed me to rework a number of manual event handling to angular-js driven scope-watching, which is nice in my book. And saving doesn't even break, because I keep the time object from the dashboard around and modify it in place.

This still has a question looming over this request, though: Why am I ultimately doing this? Shoving code around in this magnitude shouldn't be done on a whim. I do this, because I need stronger scripted dashboards than the current scripted dashboards. As such, my goal is to do the same refactoring with the dashboard service - turn it from a singleton with a current dashboard into a factory which places a dashboard object in the scope.

This will decouple the loading mechanism of the dashboards from the actual displaying of the dashboards, since the loader just outputs a dashboard and that's it. This in turn will allow me to implement selection of dashboard loading (or generation!) mechanisms based on the actual AngularJS-routing, which will open up a great extension API inside grafana :)