From da6556a23919153456c92a34038e330dd0daf693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 15 May 2016 17:41:49 +0200 Subject: [PATCH] [IMP] mis_builder: move comparison code to the style class Ultimately KpiMatrix should not have to know about kpi at all, it should become a kind of spreadsheet object that can render values in row/cols/subcols with styles. At this stage, the qweb and pdf reports, as well as the as_dict() method of the matrix already work without knowning anything about kpis. --- mis_builder/models/mis_report.py | 111 ++++++------------------- mis_builder/models/mis_report_style.py | 69 +++++++++++++++ mis_builder/tests/test_render.py | 61 +++++++------- 3 files changed, 124 insertions(+), 117 deletions(-) diff --git a/mis_builder/models/mis_report.py b/mis_builder/models/mis_report.py index 668de86c..028d6281 100644 --- a/mis_builder/models/mis_report.py +++ b/mis_builder/models/mis_report.py @@ -21,6 +21,9 @@ from .aggregate import _sum, _avg, _min, _max from .accounting_none import AccountingNone from .simple_array import SimpleArray from .mis_safe_eval import mis_safe_eval, DataError +from .mis_report_style import ( + TYPE_NUM, TYPE_PCT, TYPE_STR, CMP_DIFF, CMP_PCT, CMP_NONE +) _logger = logging.getLogger(__name__) @@ -227,7 +230,8 @@ class KpiMatrix(object): val_rendered = val.name val_comment = val.msg else: - val_rendered = kpi.render(self.lang, row.style_props, val) + val_rendered = self._style_model.render( + self.lang, row.style_props, kpi.type, val) if subcol.subkpi: val_comment = u'{}.{} = {}'.format( row.kpi.name, @@ -283,8 +287,11 @@ class KpiMatrix(object): base_vals, comparison_col.iter_subcols()): # TODO FIXME average factors - delta, delta_r, style_r = row.kpi.compare_and_render( - self.lang, row.style_props, val, base_val, 1, 1) + delta, delta_r, style_r = \ + self._style_model.compare_and_render( + self.lang, row.style_props, + row.kpi.type, row.kpi.compare_method, + val, base_val, 1, 1) comparison_cell_tuple.append(KpiMatrixCell( row, comparison_subcol, delta, delta_r, None, style_r, None)) @@ -442,18 +449,18 @@ class MisReportKpi(models.Model): string='Style expression', help='An expression that returns a style depending on the KPI value. ' 'Such style is applied on top of the row style.') - type = fields.Selection([('num', _('Numeric')), - ('pct', _('Percentage')), - ('str', _('String'))], + type = fields.Selection([(TYPE_NUM, _('Numeric')), + (TYPE_PCT, _('Percentage')), + (TYPE_STR, _('String'))], required=True, string='Value type', - default='num') - compare_method = fields.Selection([('diff', _('Difference')), - ('pct', _('Percentage')), - ('none', _('None'))], + default=TYPE_NUM) + compare_method = fields.Selection([(CMP_DIFF, _('Difference')), + (CMP_PCT, _('Percentage')), + (CMP_NONE, _('None'))], required=True, string='Comparison Method', - default='pct') + default=CMP_PCT) sequence = fields.Integer(string='Sequence', default=100) report_id = fields.Many2one('mis.report', string='Report', @@ -533,88 +540,18 @@ class MisReportKpi(models.Model): @api.onchange('type') def _onchange_type(self): - if self.type == 'num': - self.compare_method = 'pct' - elif self.type == 'pct': - self.compare_method = 'diff' - elif self.type == 'str': - self.compare_method = 'none' + if self.type == TYPE_NUM: + self.compare_method = CMP_PCT + elif self.type == TYPE_PCT: + self.compare_method = CMP_DIFF + elif self.type == TYPE_STR: + self.compare_method = CMP_NONE def get_expression_for_subkpi(self, subkpi): for expression in self.expression_ids: if expression.subkpi_id == subkpi: return expression.name - @api.multi - def render(self, lang, style_props, value): - """ render a KPI value as a unicode string, ready for display """ - self.ensure_one() - style_obj = self.env['mis.report.style'] - if self.type == 'num': - return style_obj.render_num(lang, value, style_props.divider, - style_props.dp, - style_props.prefix, style_props.suffix) - elif self.type == 'pct': - return style_obj.render_pct(lang, value, style_props.dp) - else: - return style_obj.render_str(lang, value) - - @api.multi - def compare_and_render(self, lang, style_props, value, base_value, - average_value=1, average_base_value=1): - """ render the comparison of two KPI values, ready for display - - Returns a triple, with - * the numeric comparison - * its string rendering - * the update style properties - - If the difference is 0, an empty string is returned. - """ - self.ensure_one() - style_obj = self.env['mis.report.style'] - delta = AccountingNone - style_r = style_props.copy() - if value is None: - value = AccountingNone - if base_value is None: - base_value = AccountingNone - if self.type == 'pct': - delta = value - base_value - if delta and round(delta, (style_props.dp or 0) + 2) != 0: - style_r.update(dict( - divider=0.01, prefix='', suffix=_('pp'))) - else: - delta = AccountingNone - elif self.type == 'num': - if value and average_value: - value = value / float(average_value) - if base_value and average_base_value: - base_value = base_value / float(average_base_value) - if self.compare_method == 'diff': - delta = value - base_value - if delta and round(delta, style_props.dp or 0) != 0: - pass - else: - delta = AccountingNone - elif self.compare_method == 'pct': - if base_value and round(base_value, style_props.dp or 0) != 0: - delta = (value - base_value) / abs(base_value) - if delta and round(delta, 1) != 0: - style_r.update(dict( - divider=0.01, dp=1, prefix='', suffix='%')) - else: - delta = AccountingNone - if delta is not AccountingNone: - delta_r = style_obj.render_num( - lang, delta, - style_r.divider, style_r.dp, - style_r.prefix, style_r.suffix, - sign='+') - return delta, delta_r, style_r - else: - return AccountingNone, '', style_r - class MisReportSubkpi(models.Model): _name = 'mis.report.subkpi' diff --git a/mis_builder/models/mis_report_style.py b/mis_builder/models/mis_report_style.py index f163a790..1cc300a6 100644 --- a/mis_builder/models/mis_report_style.py +++ b/mis_builder/models/mis_report_style.py @@ -31,6 +31,14 @@ PROPS = [ 'divider', ] +TYPE_NUM = 'num' +TYPE_PCT = 'pct' +TYPE_STR = 'str' + +CMP_DIFF = 'diff' +CMP_PCT = 'pct' +CMP_NONE = 'none' + class MisReportKpiStyle(models.Model): @@ -121,6 +129,10 @@ class MisReportKpiStyle(models.Model): @api.model def merge(self, styles): + """ Merge several styles, giving priority to the last. + + Returns a PropertyDict of style properties. + """ r = PropertyDict() for style in styles: if not style: @@ -139,6 +151,17 @@ class MisReportKpiStyle(models.Model): r[prop] = value return r + @api.model + def render(self, lang, style_props, type, value): + if type == 'num': + return self.render_num(lang, value, style_props.divider, + style_props.dp, + style_props.prefix, style_props.suffix) + elif type == 'pct': + return self.render_pct(lang, value, style_props.dp) + else: + return self.render_str(lang, value) + @api.model def render_num(self, lang, value, divider=1.0, dp=0, prefix=None, suffix=None, sign='-'): @@ -165,6 +188,52 @@ class MisReportKpiStyle(models.Model): return u'' return unicode(value) + @api.model + def compare_and_render(self, lang, style_props, type, compare_method, + value, base_value, + average_value=1, average_base_value=1): + delta = AccountingNone + style_r = style_props.copy() + if value is None: + value = AccountingNone + if base_value is None: + base_value = AccountingNone + if type == TYPE_PCT: + delta = value - base_value + if delta and round(delta, (style_props.dp or 0) + 2) != 0: + style_r.update(dict( + divider=0.01, prefix='', suffix=_('pp'))) + else: + delta = AccountingNone + elif type == TYPE_NUM: + if value and average_value: + value = value / float(average_value) + if base_value and average_base_value: + base_value = base_value / float(average_base_value) + if compare_method == CMP_DIFF: + delta = value - base_value + if delta and round(delta, style_props.dp or 0) != 0: + pass + else: + delta = AccountingNone + elif compare_method == CMP_PCT: + if base_value and round(base_value, style_props.dp or 0) != 0: + delta = (value - base_value) / abs(base_value) + if delta and round(delta, 1) != 0: + style_r.update(dict( + divider=0.01, dp=1, prefix='', suffix='%')) + else: + delta = AccountingNone + if delta is not AccountingNone: + delta_r = self.render_num( + lang, delta, + style_r.divider, style_r.dp, + style_r.prefix, style_r.suffix, + sign='+') + return delta, delta_r, style_r + else: + return AccountingNone, '', style_r + @api.model def to_xlsx_style(self, props): num_format = '0' diff --git a/mis_builder/tests/test_render.py b/mis_builder/tests/test_render.py index 62bf7d84..4a08a6ab 100644 --- a/mis_builder/tests/test_render.py +++ b/mis_builder/tests/test_render.py @@ -5,6 +5,9 @@ import openerp.tests.common as common from ..models.accounting_none import AccountingNone +from ..models.mis_report_style import ( + TYPE_NUM, TYPE_PCT, TYPE_STR, CMP_DIFF, CMP_PCT +) class TestRendering(common.TransactionCase): @@ -16,22 +19,18 @@ class TestRendering(common.TransactionCase): self.style = self.style_obj.create(dict( name='teststyle', )) - self.kpi = self.kpi_obj.create(dict( - name='testkpi', - description='test kpi', - type='num', - style_id=self.style.id, - )) self.lang = self.env['res.lang'].search([('code', '=', 'en_US')])[0] - def _render(self, value): + def _render(self, value, type=TYPE_NUM): style_props = self.style_obj.merge([self.style]) - return self.kpi.render(self.lang, style_props, value) + return self.style_obj.render(self.lang, style_props, type, value) - def _compare_and_render(self, value, base_value): + def _compare_and_render(self, value, base_value, + type=TYPE_NUM, compare_method=CMP_PCT): style_props = self.style_obj.merge([self.style]) - return self.kpi.compare_and_render(self.lang, style_props, - value, base_value)[:2] + return self.style_obj.compare_and_render(self.lang, style_props, + type, compare_method, + value, base_value)[:2] def test_render(self): self.assertEquals(u'1', self._render(1)) @@ -89,21 +88,18 @@ class TestRendering(common.TransactionCase): self.assertEquals(u'1,000,000', self._render(1)) def test_render_pct(self): - self.kpi.type = 'pct' - self.assertEquals(u'100\xa0%', self._render(1)) - self.assertEquals(u'50\xa0%', self._render(0.5)) + self.assertEquals(u'100\xa0%', self._render(1, TYPE_PCT)) + self.assertEquals(u'50\xa0%', self._render(0.5, TYPE_PCT)) self.style.dp_inherit = False self.style.dp = 2 - self.assertEquals(u'51.23\xa0%', self._render(0.5123)) + self.assertEquals(u'51.23\xa0%', self._render(0.5123, TYPE_PCT)) def test_render_string(self): - self.kpi.type = 'str' - self.assertEquals(u'', self._render('')) - self.assertEquals(u'', self._render(None)) - self.assertEquals(u'abcdé', self._render(u'abcdé')) + self.assertEquals(u'', self._render('', TYPE_STR)) + self.assertEquals(u'', self._render(None, TYPE_STR)) + self.assertEquals(u'abcdé', self._render(u'abcdé', TYPE_STR)) def test_compare_num_pct(self): - self.assertEquals('pct', self.kpi.compare_method) self.assertEquals((1.0, u'+100.0\xa0%'), self._compare_and_render(100, 50)) self.assertEquals((0.5, u'+50.0\xa0%'), @@ -132,26 +128,31 @@ class TestRendering(common.TransactionCase): self._compare_and_render(None, 50)) def test_compare_num_diff(self): - self.kpi.compare_method = 'diff' self.assertEquals((25, u'+25'), - self._compare_and_render(75, 50)) + self._compare_and_render(75, 50, + TYPE_NUM, CMP_DIFF)) self.assertEquals((-25, u'\u201125'), - self._compare_and_render(25, 50)) + self._compare_and_render(25, 50, + TYPE_NUM, CMP_DIFF)) self.style.suffix_inherit = False self.style.suffix = u'€' self.assertEquals((-25, u'\u201125\xa0€'), - self._compare_and_render(25, 50)) + self._compare_and_render(25, 50, + TYPE_NUM, CMP_DIFF)) self.style.suffix = u'' self.assertEquals((50.0, u'+50'), - self._compare_and_render(50, AccountingNone)) + self._compare_and_render(50, AccountingNone, + TYPE_NUM, CMP_DIFF)) self.assertEquals((50.0, u'+50'), - self._compare_and_render(50, None)) + self._compare_and_render(50, None, + TYPE_NUM, CMP_DIFF)) self.assertEquals((-50.0, u'\u201150'), - self._compare_and_render(AccountingNone, 50)) + self._compare_and_render(AccountingNone, 50, + TYPE_NUM, CMP_DIFF)) self.assertEquals((-50.0, u'\u201150'), - self._compare_and_render(None, 50)) + self._compare_and_render(None, 50, + TYPE_NUM, CMP_DIFF)) def test_compare_pct(self): - self.kpi.type = 'pct' self.assertEquals((0.25, u'+25\xa0pp'), - self._compare_and_render(0.75, 0.50)) + self._compare_and_render(0.75, 0.50, TYPE_PCT))