From b891fd188974c1946c028e898a670086893c7f9c Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Mon, 20 Feb 2023 15:13:15 +0100 Subject: [PATCH 1/2] dataviz: move choice field contruction to separate method (#72896) --- combo/apps/dataviz/forms.py | 106 +++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 51 deletions(-) diff --git a/combo/apps/dataviz/forms.py b/combo/apps/dataviz/forms.py index 0af3b3c8..749204bb 100644 --- a/combo/apps/dataviz/forms.py +++ b/combo/apps/dataviz/forms.py @@ -75,57 +75,7 @@ class ChartFiltersMixin: ): continue - has_option_groups = isinstance(filter_['options'][0], list) - if filter_['options'] and has_option_groups: - choices = { - group: [(opt['id'], opt['label']) for opt in options] - for group, options in filter_['options'] - } - choices_to_complete = choices[None] = choices.get(None, []) - choices = list(choices.items()) - else: - choices = [(option['id'], option['label']) for option in filter_['options']] - choices_to_complete = choices - - initial = cell.filter_params.get(filter_id, filter_.get('default')) - - if filter_id == 'time_interval': - self.extend_time_interval_choices(choices) - - required = filter_.get('required', False) - multiple = filter_.get('multiple') - if not required: - choices_to_complete.insert(0, BLANK_CHOICE_DASH[0]) - - extra_variables = cell.page.get_extra_variables_keys() - variable_choices = [('variable:' + key, key) for key in extra_variables] - - if has_option_groups: - possible_choices = {choice[0] for _, group_choices in choices for choice in group_choices} - else: - possible_choices = {choice[0] for choice in choices} - for choice in initial if isinstance(initial, list) else [initial]: - if not choice: - continue - if choice.startswith('variable:'): - variable = choice.replace('variable:', '') - if not variable in extra_variables: - variable_choices.append((choice, _('%s (unavailable)') % variable)) - elif choice not in possible_choices: - choices_to_complete.append((choice, _('%s (unavailable)') % choice)) - - if variable_choices and not multiple and filter_id != 'time_interval': - choices.append((_('Page variables'), variable_choices)) - - field_class = forms.MultipleChoiceField if multiple else forms.ChoiceField - widget_class = MultiSelectWidget if multiple else forms.Select - fields[filter_id] = field_class( - label=filter_['label'], - choices=choices, - required=required, - initial=initial, - widget=widget_class, - ) + fields[filter_id] = self.build_choice_field(cell, filter_) if filter_.get('deprecated'): fields[filter_id].label += ' (%s)' % _('deprecated') fields[filter_id].help_text = filter_.get('deprecation_hint') @@ -133,6 +83,60 @@ class ChartFiltersMixin: return fields + def build_choice_field(self, cell, filter_): + filter_id = filter_['id'] + + has_option_groups = isinstance(filter_['options'][0], list) + if filter_['options'] and has_option_groups: + choices = { + group: [(opt['id'], opt['label']) for opt in options] for group, options in filter_['options'] + } + choices_to_complete = choices[None] = choices.get(None, []) + choices = list(choices.items()) + else: + choices = [(option['id'], option['label']) for option in filter_['options']] + choices_to_complete = choices + + initial = cell.filter_params.get(filter_id, filter_.get('default')) + + if filter_id == 'time_interval': + self.extend_time_interval_choices(choices) + + required = filter_.get('required', False) + multiple = filter_.get('multiple') + if not required: + choices_to_complete.insert(0, BLANK_CHOICE_DASH[0]) + + extra_variables = cell.page.get_extra_variables_keys() + variable_choices = [('variable:' + key, key) for key in extra_variables] + + if has_option_groups: + possible_choices = {choice[0] for _, group_choices in choices for choice in group_choices} + else: + possible_choices = {choice[0] for choice in choices} + for choice in initial if isinstance(initial, list) else [initial]: + if not choice: + continue + if choice.startswith('variable:'): + variable = choice.replace('variable:', '') + if not variable in extra_variables: + variable_choices.append((choice, _('%s (unavailable)') % variable)) + elif choice not in possible_choices: + choices_to_complete.append((choice, _('%s (unavailable)') % choice)) + + if variable_choices and not multiple and filter_id != 'time_interval': + choices.append((_('Page variables'), variable_choices)) + + field_class = forms.MultipleChoiceField if multiple else forms.ChoiceField + widget_class = MultiSelectWidget if multiple else forms.Select + return field_class( + label=filter_['label'], + choices=choices, + required=required, + initial=initial, + widget=widget_class, + ) + def extend_time_interval_choices(self, choices): choice_ids = {choice_id for choice_id, _ in choices} if 'day' in choice_ids: -- 2.39.2 From 16844f570089e3bad8d44970d08ed80c6d4aa49a Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Mon, 20 Feb 2023 16:37:10 +0100 Subject: [PATCH 2/2] dataviz: display required boolean filter as checkbox (#72896) --- combo/apps/dataviz/forms.py | 27 ++++++++++++++++++++++----- tests/test_dataviz.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/combo/apps/dataviz/forms.py b/combo/apps/dataviz/forms.py index 749204bb..39e90ea1 100644 --- a/combo/apps/dataviz/forms.py +++ b/combo/apps/dataviz/forms.py @@ -75,7 +75,15 @@ class ChartFiltersMixin: ): continue - fields[filter_id] = self.build_choice_field(cell, filter_) + initial = cell.filter_params.get(filter_id, filter_.get('default')) + required = filter_.get('required', False) + + if required and {x['id'] for x in filter_['options'] if isinstance(x, dict)} == {'true', 'false'}: + field = self.build_boolean_field(cell, filter_, initial) + else: + field = self.build_choice_field(cell, filter_, initial) + + fields[filter_id] = field if filter_.get('deprecated'): fields[filter_id].label += ' (%s)' % _('deprecated') fields[filter_id].help_text = filter_.get('deprecation_hint') @@ -83,7 +91,7 @@ class ChartFiltersMixin: return fields - def build_choice_field(self, cell, filter_): + def build_choice_field(self, cell, filter_, initial): filter_id = filter_['id'] has_option_groups = isinstance(filter_['options'][0], list) @@ -97,8 +105,6 @@ class ChartFiltersMixin: choices = [(option['id'], option['label']) for option in filter_['options']] choices_to_complete = choices - initial = cell.filter_params.get(filter_id, filter_.get('default')) - if filter_id == 'time_interval': self.extend_time_interval_choices(choices) @@ -137,6 +143,13 @@ class ChartFiltersMixin: widget=widget_class, ) + def build_boolean_field(self, cell, filter_, initial): + return forms.BooleanField( + label=filter_['label'], + required=False, + initial=bool(initial == 'true'), + ) + def extend_time_interval_choices(self, choices): choice_ids = {choice_id for choice_id, _ in choices} if 'day' in choice_ids: @@ -228,7 +241,11 @@ class ChartNgForm(ChartFiltersMixin, forms.ModelForm): else: for filter_ in self.instance.available_filters: if filter_['id'] in self.cleaned_data: - self.instance.filter_params[filter_['id']] = self.cleaned_data[filter_['id']] + value = self.cleaned_data[filter_['id']] + if isinstance(value, bool): + value = 'true' if value is True else 'false' + + self.instance.filter_params[filter_['id']] = value cell = super().save(*args, **kwargs) diff --git a/tests/test_dataviz.py b/tests/test_dataviz.py index 28b234fd..67fbb7a1 100644 --- a/tests/test_dataviz.py +++ b/tests/test_dataviz.py @@ -564,6 +564,23 @@ STATISTICS_LIST = { 'id': 'deprecated', 'deprecated': True, }, + { + 'url': 'https://authentic.example.com/api/statistics/required-boolean/', + 'name': 'Required boolean choices', + 'id': 'required-boolean', + "filters": [ + { + "id": "test", + "label": "Test", + "options": [ + {"id": "true", "label": "True"}, + {"id": "false", "label": "False"}, + ], + "default": "true", + "required": True, + }, + ], + }, ] } @@ -1656,6 +1673,19 @@ def test_chartng_cell_manager_new_api(app, admin_user, new_api_statistics): cell.refresh_from_db() assert cell.filter_params == {'test': 'b'} + required_boolean_stat = Statistic.objects.get(slug='required-boolean') + resp.form[field_prefix + 'statistic'] = required_boolean_stat.pk + manager_submit_cell(resp.form) + assert resp.form[field_prefix + 'test'].checked is True + cell.refresh_from_db() + assert cell.filter_params == {'test': 'true'} + + manager_submit_cell(resp.form) + resp.form[field_prefix + 'test'] = False + manager_submit_cell(resp.form) + cell.refresh_from_db() + assert cell.filter_params == {'test': 'false'} + deprecated_stat = Statistic.objects.get(slug='deprecated-filter') resp.form[field_prefix + 'statistic'] = deprecated_stat.pk manager_submit_cell(resp.form) -- 2.39.2