diff --git a/django/website/dashboard/templatetags/render_widget.py b/django/website/dashboard/templatetags/render_widget.py index e9de818e4e26ce5f3f65760b9103bdc03bf3ac60..175ea4a9dbe41215b0aa2f749d70d0a4917c79e2 100644 --- a/django/website/dashboard/templatetags/render_widget.py +++ b/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' diff --git a/django/website/dashboard/tests/widget_tests.py b/django/website/dashboard/tests/widget_tests.py index 2b69bc3326e97cee9a4961087bde2f175c569fcf..0719de5c97acb8e27e8f37436db2bd25208a86a0 100644 --- a/django/website/dashboard/tests/widget_tests.py +++ b/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' + ) diff --git a/django/website/dashboard/widget_pool.py b/django/website/dashboard/widget_pool.py index 98cc08bc6c92c660e979490e8a1074b516db8f9e..83bc41577f503bddd040d69e6ac87429a80bfa75 100644 --- a/django/website/dashboard/widget_pool.py +++ b/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 diff --git a/django/website/hid/static/hid/widgets/chart.js b/django/website/hid/static/hid/widgets/chart.js index 54b79baf93dab1b670af46f59ff50b0849856a0c..34bd445937a3cbbbe46058f8517b35c25ba45a37 100644 --- a/django/website/hid/static/hid/widgets/chart.js +++ b/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 */ diff --git a/django/website/hid/templates/hid/tests/chart.html b/django/website/hid/templates/hid/tests/chart.html index b4e4ca77c639110551fb4fc59570b1abdc5a1879..8176e48febcd8eb8b94c7e0f9c8055ae7c99c642 100644 --- a/django/website/hid/templates/hid/tests/chart.html +++ b/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 %} diff --git a/django/website/hid/templates/hid/widgets/chart.html b/django/website/hid/templates/hid/widgets/chart.html index e8a742cf81a81bf92e2297bee375bae66882a5bd..f514ef1d675c4ded785559a98bc501442c83d692 100644 --- a/django/website/hid/templates/hid/widgets/chart.html +++ b/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 }}' > diff --git a/django/website/hid/tests/term_count_widget_tests.py b/django/website/hid/tests/term_count_widget_tests.py index 6d8e2ddec5a1fea598f9ba653a4866d6da735393..632435fa67505752cc6f5359516023d408fcbb54 100644 --- a/django/website/hid/tests/term_count_widget_tests.py +++ b/django/website/hid/tests/term_count_widget_tests.py @@ -1,4 +1,7 @@ +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']) diff --git a/django/website/hid/widgets/term_count_chart.py b/django/website/hid/widgets/term_count_chart.py index 136650decd0b1317935d007359e79fa3d054e0e3..864e9972b844464d335cb15a82703cde293f0ebe 100644 --- a/django/website/hid/widgets/term_count_chart.py +++ b/django/website/hid/widgets/term_count_chart.py @@ -1,4 +1,9 @@ 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 + }] } diff --git a/django/website/media/less/internews.less b/django/website/media/less/internews.less index 4e34fa209b2a0c85f875595169ca2fa89da47e97..9bc352f153b0377deb70dd4c037db4011ae6e5b0 100644 --- a/django/website/media/less/internews.less +++ b/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 { diff --git a/django/website/settings.py b/django/website/settings.py index 96f2f289b9e81a2e8dc5e925aec4b31d983ca6fc..286d53cb61cb22b856c28cc2de7bf6b85f16d26b 100644 --- a/django/website/settings.py +++ b/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.?