Merge pull request #8 from aptivate/barchart_time_period

Barchart time period

django/website/dashboard/templatetags/render_widget.py

@@ -4,7 +4,7 @@ from django import template
 from django.template.loader import render_to_string
 from django.utils.translation import ugettext_lazy as _
 
-from dashboard.widget_pool import get_widget, MissingWidgetType
+from dashboard.widget_pool import get_widget, MissingWidgetType, WidgetError
 
 
 logger = logging.getLogger(__name__)
@@ -53,6 +53,10 @@ def render_widget(widget_instance):
             context = widget.get_context_data(**settings)
         except AttributeError:
             context = {}
+        except WidgetError as e:
+            template_name = 'dashboard/widget-error.html'
+            context = {}
+            context['error'] = str(e)
         except Exception as e:
             logger.exception('Error while fetching widget context data: %s', e)
             template_name = 'dashboard/widget-error.html'

django/website/dashboard/tests/widget_tests.py

@@ -3,7 +3,7 @@ from mock import patch
 from django.test import TestCase
 
 from dashboard.templatetags.render_widget import render_widget
-from dashboard.widget_pool import register_widget, get_widget
+from dashboard.widget_pool import register_widget, get_widget, WidgetError
 
 
 class TestWidget(object):
@@ -38,6 +38,26 @@ class TestWidgetNoTemplateName(object):
         return {}
 
 
+class TestWidgetRaisesException(object):
+    """ A test widget which raises a generic exception in
+        get_context_data
+    """
+    template_name = 'something.html'
+
+    def get_context_data(self, **kwargs):
+        raise Exception('message raised from get_context_data')
+
+
+class TestWidgetRaisesWidgetError(object):
+    """ A test widget which raises a WidgetError in
+        get_context_data
+    """
+    template_name = 'something.html'
+
+    def get_context_data(self, **kwargs):
+        raise WidgetError('message raised from get_context_data')
+
+
 class MockWidgetInstance(object):
     """ A Mock class to represent a widget instance
 
@@ -152,3 +172,48 @@ class WidgetPoolTestCase(TestCase):
         self.assertEqual(
             template_name, 'dashboard/widget-error.html'
         )
+
+    def test_render_widget_exception_includes_generic_message(self):
+        """ Test that a widget which raises a generic exception will
+            display a generic error message, not the content of the
+            exception
+        """
+        test_widget = TestWidgetRaisesException()
+        register_widget('test-widget', test_widget)
+        widget_instance = MockWidgetInstance('test-widget')
+        with patch(self.render_to_string_method) as mock:
+            render_widget(widget_instance)
+            template_name = self.get_mock_render_to_string_parameter(
+                mock, 'template_name'
+            )
+            context = self.get_mock_render_to_string_parameter(
+                mock, 'context'
+            )
+        self.assertEqual(
+            template_name, 'dashboard/widget-error.html'
+        )
+        self.assertEqual(
+            context['error'], 'Widget error. See error logs.'
+        )
+
+    def test_render_widget_widgeterror_exception_includes_error_message(self):
+        """ Test that a widget which raises a WidgetError exception will
+            display the error message provided in the exception.
+        """
+        test_widget = TestWidgetRaisesWidgetError()
+        register_widget('test-widget', test_widget)
+        widget_instance = MockWidgetInstance('test-widget')
+        with patch(self.render_to_string_method) as mock:
+            render_widget(widget_instance)
+            template_name = self.get_mock_render_to_string_parameter(
+                mock, 'template_name'
+            )
+            context = self.get_mock_render_to_string_parameter(
+                mock, 'context'
+            )
+        self.assertEqual(
+            template_name, 'dashboard/widget-error.html'
+        )
+        self.assertEqual(
+            str(context['error']), 'message raised from get_context_data'
+        )

django/website/dashboard/widget_pool.py

@@ -6,6 +6,17 @@ class MissingWidgetType(Exception):
     pass
 
 
+class WidgetError(Exception):
+    """ Exception that can be raised from widget types
+        in get_context_data.
+
+        The error message will be displayed to the end
+        user, so should not contain debug or sensisitve
+        information
+    """
+    pass
+
+
 def register_widget(name, widget):
     """ Register a new widget type
 

django/website/hid/static/hid/widgets/chart.js

@@ -39,11 +39,16 @@ FlotChart = {
 
         /* Render the chart */
         render: function() {
+            var options = this.chart.get('options');
+            var $legend_container = this.$el.siblings('.flot-legend');
+            if ($legend_container.length > 0 && options['legend']) {
+                options['legend']['container'] = $legend_container;
+            }
             this.resize();
             $.plot(
                 this.$el,
                 this.chart.get('data'), 
-                this.chart.get('options')
+                options
             );
 
             return this;
@@ -51,8 +56,14 @@ FlotChart = {
 
         /* Resize the chart */
         resize: function() {
-            this.$el.width(this.$container.width());
-            this.$el.height(this.$container.height());
+            if (this.$container.length == 0) {
+                return;
+            }
+            var new_width = this.$container.width();
+            var chart_parent_offset = this.$el.offset().top - this.$container.offset().top;
+            var new_height = this.$container.height() - chart_parent_offset;
+            this.$el.width(new_width);
+            this.$el.height(new_height);
         },
 
         /* Display tooltip */

django/website/hid/templates/hid/tests/chart.html

@@ -51,9 +51,11 @@
     </script>
 {% endblock %}
 {% block body_content %}
-    <div class='flot-chart' 
-         data-data='[[[23, 0], [10, 1], [150, 2], [50, 3]]]'
-         data-options='{"series": {"bars": {"show": true}}, "bars": {"horizontal": true}, "yaxis": {"ticks": [[0, "q1"], [1, "q2"], [2, "q3"], [3, "q4"]]}}'
-         style='width:300px; height:300px'
-    ></div>
+    <div class='panel-body'>
+        <div class='flot-chart'
+             data-data='[[[23, 0], [10, 1], [150, 2], [50, 3]]]'
+             data-options='{"series": {"bars": {"show": true}}, "bars": {"horizontal": true}, "yaxis": {"ticks": [[0, "q1"], [1, "q2"], [2, "q3"], [3, "q4"]]}}'
+             style='width:300px; height:300px'
+        ></div>
+    </div>
 {% endblock %}

django/website/hid/templates/hid/widgets/chart.html

@@ -5,7 +5,9 @@
         <h2>{{title}}</h2>
     </div>
     <div class='panel-body'>
-        <div class='flot-chart' 
+        <div class='flot-legend'>
+        </div>
+        <div class='flot-chart'
              data-options='{{ options|json_data }}'
              data-data='{{ data|json_data }}'
         >

django/website/hid/tests/term_count_widget_tests.py

+from datetime import datetime, timedelta
+from dateutil import parser
 from mock import patch
+from dashboard.widget_pool import WidgetError
 from django.test import TestCase
 from hid.widgets.term_count_chart import TermCountChartWidget
 
@@ -50,10 +53,80 @@ class TestTermCountChartWidget(TestCase):
                 title='test-name', taxonomy='tax'
             )
         self.assertEqual(
-            context_data['data'],
-            [[[345, 2], [782, 1]]]
+            context_data['data'][0]['data'],
+            [[345, 2], [782, 1]]
         )
 
+    def test_context_data_hides_legend_when_there_is_no_time_period(self):
+        widget = TermCountChartWidget()
+        with patch('hid.widgets.term_count_chart.term_itemcount') as itemcount:
+            itemcount.return_value = []
+            context_data = widget.get_context_data(
+                title='test-name', taxonomy='tax'
+            )
+        self.assertEqual(context_data['options']['legend']['show'], False)
+
+    def test_context_data_includes_legend_when_there_is_a_time_period(self):
+        widget = TermCountChartWidget()
+        with patch('hid.widgets.term_count_chart.term_itemcount') as itemcount:
+            itemcount.return_value = []
+            periods = [{
+                'start_time': '2015-01-01',
+                'end_time': '2015-02-02'
+            }]
+            context_data = widget.get_context_data(
+                title='test-name', taxonomy='tax', periods=periods
+            )
+        self.assertEqual(context_data['options']['legend']['show'], True)
+
+    def test_context_data_raises_widgeterror_when_more_than_one_period(self):
+        widget = TermCountChartWidget()
+        with patch('hid.widgets.term_count_chart.term_itemcount') as itemcount:
+            itemcount.return_value = []
+            periods = [{
+                'start_time': '2015-01-01',
+                'end_time': '2015-02-02'
+            }, {
+                'start-time': '2015-07-08',
+                'end-time': '2015-07-09'
+            }]
+            with self.assertRaises(WidgetError):
+                widget.get_context_data(
+                    title='test-name', taxonomy='tax', periods=periods
+                )
+
+    def test_context_data_raises_widgeterror_when_date_is_not_parseable(self):
+        widget = TermCountChartWidget()
+        with patch('hid.widgets.term_count_chart.term_itemcount') as itemcount:
+            itemcount.return_value = []
+            periods = [{
+                'start_time': '!!!',
+                'end_time': '2015-02-02'
+            }, {
+                'start-time': '2015-07-08',
+                'end-time': '2015-07-09'
+            }]
+            with self.assertRaises(WidgetError):
+                widget.get_context_data(
+                    title='test-name', taxonomy='tax', periods=periods
+                )
+
+    def test_get_context_data_parses_dates(self):
+        widget = TermCountChartWidget()
+        with patch('hid.widgets.term_count_chart.term_itemcount') as itemcount:
+            itemcount.return_value = []
+            periods = [{
+                'start_time': '2015-01-01',
+                'end_time': '2015-02-02'
+            }]
+            widget.get_context_data(
+                title='test-name', taxonomy='tax', periods=periods
+            )
+            kwargs = itemcount.call_args[1]
+
+        self.assertEqual(kwargs['start_time'], parser.parse('2015-01-01'))
+        self.assertEqual(kwargs['end_time'], parser.parse('2015-02-02'))
+
     def test_chart_questions_are_set_as_yaxis_value_labels(self):
         widget = TermCountChartWidget()
         with patch('hid.widgets.term_count_chart.term_itemcount') as itemcount:
@@ -93,7 +166,7 @@ class TestTermCountChartWidget(TestCase):
                     'count': 1000
                 },
             ]
-            counts = widget._fetch_counts('tax', 0, 'Others')
+            counts = widget._fetch_counts('tax', 0, None, None, 'Others')
 
         self.assertEqual(
             counts.items(),
@@ -127,7 +200,7 @@ class TestTermCountChartWidget(TestCase):
                 },
 
             ]
-            counts = widget._fetch_counts('tax', 3, 'Others')
+            counts = widget._fetch_counts('tax', 3, None, None, 'Others')
 
         self.assertEqual(
             counts.items(),
@@ -136,3 +209,24 @@ class TestTermCountChartWidget(TestCase):
                 ('Others', 11)
             ]
         )
+
+    def test_fetch_count_ignores_missing_start_and_end_time(self):
+        widget = TermCountChartWidget()
+
+        with patch('hid.widgets.term_count_chart.term_itemcount') as itemcount:
+            widget._fetch_counts('tax', 3, None, None, 'Others')
+            itemcount_kwargs = itemcount.call_args[1]
+
+        self.assertNotIn('start_time', itemcount_kwargs)
+        self.assertNotIn('end_time', itemcount_kwargs)
+
+    def test_fetch_count_uses_start_and_end_time(self):
+        widget = TermCountChartWidget()
+        t1 = datetime.now()
+        t2 = t1 + timedelta(days=4)
+        with patch('hid.widgets.term_count_chart.term_itemcount') as itemcount:
+            widget._fetch_counts('tax', 3, t1, t2, 'Others')
+            itemcount_kwargs = itemcount.call_args[1]
+
+        self.assertEqual(t1, itemcount_kwargs['start_time'])
+        self.assertEqual(t2, itemcount_kwargs['end_time'])

django/website/hid/widgets/term_count_chart.py

 from collections import OrderedDict
+from dashboard.widget_pool import WidgetError
+from datetime import timedelta
+from dateutil import parser
+from django.conf import settings
+from django.utils import dateformat
 from django.utils.translation import ugettext_lazy as _
 from transport.taxonomies import term_itemcount
 
@@ -23,17 +28,27 @@ class TermCountChartWidget(object):
         'hid/widgets/chart.js'
     ]
 
-    def _fetch_counts(self, taxonomy, count, other_label):
+    def _fetch_counts(self, taxonomy, count, start, end, other_label):
         """ Given a taxonomy, fetch the count per term.
 
         Args:
-            - taxonomy: Taxonomy slug
-            - count: If >0, maximum number of rows to returns. If the data
-                     has more terms, all other terms are aggregated under
-                     an 'others' section
-            - other_label: Label for the 'Others' section
+            taxonomy (str): Taxonomy slug
+            count (int): If >0, maximum number of rows to returns. If the data
+                has more terms, all other terms are aggregated under
+                an 'others' section
+            start (datetime or None): If not None, the start of the time period
+                to get the count for
+            end (datetime or None): If not None, the start of the time period
+                to get the count for
+            other_label (str): Label for the 'Others' section
         """
-        itemcount = term_itemcount(taxonomy)
+        itemcount = None
+        if start is not None and end is not None:
+            itemcount = term_itemcount(
+                taxonomy, start_time=start, end_time=end
+            )
+        else:
+            itemcount = term_itemcount(taxonomy)
         itemcount.sort(key=lambda k: int(k['count']), reverse=True)
         if count > 0:
             head = itemcount[0:count-1]
@@ -72,13 +87,74 @@ class TermCountChartWidget(object):
 
         return values, yticks
 
+    def _create_date_range_label(self, start, end):
+        """ Create a label to display a date range.
+
+        The dates are formatter such that:
+        - If either start or end include hours/minutes/seconds
+          that are not 00:00:00 then the full date time is
+          displayed;
+        - If both start and end have zero hours/minutes/seconds
+          then only the day is displayed, and the end day
+          is set to the previous day (to show an inclusive
+          range);
+
+        Args:
+            start (datetime): Start date time
+            end (datetime): End date time
+        Returns:
+            str: Label to use for the date range.
+        """
+        if not start.time() and not end.time():
+            start_str = dateformat.format(start,
+                                          settings.SHORT_DATE_FORMAT)
+            end_str = dateformat.format(end - timedelta(days=1),
+                                        settings.SHORT_DATE_FORMAT)
+        else:
+            start_str = dateformat.format(start,
+                                          settings.SHORT_DATETIME_FORMAT),
+            end_str = dateformat.format(end,
+                                        settings.SHORT_DATETIME_FORMAT)
+        if start_str == end_str:
+            return _('%(date)s') % {'date': start_str}
+        else:
+            return _('%(start)s - %(end)s') % {
+                'start': start_str,
+                'end': end_str
+            }
+
     def get_context_data(self, **kwargs):
         title = kwargs.get('title', _('(missing title)'))
         taxonomy = kwargs.get('taxonomy')
         count = kwargs.get('count', 0)
         other_label = kwargs.get('other_label', 'Others')
+        periods = kwargs.get('periods', [])
 
-        counts = self._fetch_counts(taxonomy, count, other_label)
+        if len(periods) > 1:
+            raise WidgetError('Only one time period is currently supported')
+        if len(periods) == 1:
+            try:
+                start_time = parser.parse(periods[0]['start_time'])
+                end_time = parser.parse(periods[0]['end_time'])
+            except ValueError:
+                raise WidgetError('Error parsing start/end time')
+            legend = {
+                'show': True,
+                'noColumns': 1,
+                'position': 'ne',
+                'labelBoxBorderColor': 'white',
+                'backgroundColor': 'white'
+            }
+            label = self._create_date_range_label(start_time, end_time)
+        else:
+            start_time = None
+            end_time = None
+            legend = {'show': False}
+            label = ''
+
+        counts = self._fetch_counts(
+            taxonomy, count, start_time, end_time, other_label
+        )
         (values, yticks) = self._create_axis_values(counts)
         return {
             'title': title,
@@ -117,7 +193,12 @@ class TermCountChartWidget(object):
                     'margin': 10,
                     'labelMargin': 20,
                     'backgroundColor': '#fafafa'
-                }
+                },
+                'legend': legend
             },
-            'data': [values]
+            'data': [{
+                'label': label,
+                'color': '#f29e30',
+                'data': values
+            }]
         }

django/website/media/less/internews.less

@@ -403,6 +403,11 @@ body {
     height: 100%;
 }
 
+.flot-legend {
+    padding-bottom: @padding-small-vertical;
+    border-bottom: solid 1px @gray-lighter;
+}
+
 // Circle Buttons 
 
 .btn-circle {

django/website/settings.py

@@ -64,6 +64,7 @@ USE_L10N = True
 USE_TZ = True
 
 SHORT_DATETIME_FORMAT = 'd M Y H:i'
+SHORT_DATE_FORMAT = 'd M Y'
 # TODO this is used in hid/tables.py
 # and should probably use FORMAT_MODULE_PATH instead.?