From 17843f270d682bc9ec09f04a28566fe7d6c326b3 Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 18 May 2020 10:37:55 +0100 Subject: [PATCH 01/21] feat: add TRUE and FALSE constants to QA grammar --- apollo/submissions/qa/query_builder.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index f6f253bce..6d1cc1bd2 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -5,7 +5,7 @@ from arpeggio import PTNodeVisitor, visit_parse_tree from arpeggio.cleanpeg import ParserPEG -from sqlalchemy import Integer, String, and_, case, false, func, null, or_ +from sqlalchemy import Integer, String, and_, case, false, func, null, true from sqlalchemy.dialects.postgresql import array from sqlalchemy.sql.operators import concat_op @@ -28,8 +28,10 @@ name = r'[a-zA-Z_][a-zA-Z0-9_]*' lookup = "$" ("location" / "participant" / "submission") ("." / "@") name null = "NULL" +true = "TRUE" +false = "FALSE" factor = ("+" / "-")? (number / variable / lookup / "(" expression ")") -value = null / factor +value = null / true / false / factor exponent = value (("^") value)* product = exponent (("*" / "/") exponent)* sum = product (("+" / "-") product)* @@ -154,6 +156,12 @@ def __init__(self, defaults=True, **kwargs): self.submission = kwargs.pop('submission') super().__init__(defaults, **kwargs) + def visit_false(self, node, children): + return False + + def visit_true(self, node, children): + return True + def visit_variable(self, node, children): var_name = node.value if var_name not in self.form.tags: @@ -197,6 +205,12 @@ def __init__(self, defaults=True, **kwargs): self.variables = set() super().__init__(defaults, **kwargs) + def visit_false(self, node, children): + return false() + + def visit_true(self, node, children): + return true() + def visit_lookup(self, node, children): top_level_attr, symbol, name = children From 2c31bc00185c9689cb9272459d90a7ab4fac75d5 Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Tue, 19 May 2020 10:09:30 +0100 Subject: [PATCH 02/21] feat: add support for single-line QA expressions --- apollo/submissions/qa/query_builder.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index 6d1cc1bd2..1fdc350a6 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -393,7 +393,9 @@ def get_inline_qa_status(submission, condition): def build_expression(logical_check): - if 'criteria' in logical_check: + if 'expression' in logical_check: + control_expression = logical_check.get('expression') + elif 'criteria' in logical_check: control_expression = '' for index, cond in enumerate(logical_check['criteria']): From 380f654576835a7d98bc8c924f389d6a831a987b Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Wed, 20 May 2020 07:26:15 +0100 Subject: [PATCH 03/21] feat: error out for QA on multivalue questions --- apollo/submissions/qa/query_builder.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index 1fdc350a6..2bec97edd 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -234,6 +234,10 @@ def visit_variable(self, node, children): self.lock_null = True return null() + field = self.form.get_field_by_tag(var_name) + if field['type'] == 'multiselect': + raise ValueError('QA not supported for multi-value fields') + # casting is necessary because PostgreSQL will throw # a fit if you attempt some operations that mix JSONB # with other types From 355cb8afb821363dc6c5c960977f8542c328471c Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Thu, 21 May 2020 19:50:29 +0100 Subject: [PATCH 04/21] fix: add missing import --- apollo/submissions/qa/query_builder.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index 2bec97edd..fa382f724 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -5,7 +5,8 @@ from arpeggio import PTNodeVisitor, visit_parse_tree from arpeggio.cleanpeg import ParserPEG -from sqlalchemy import Integer, String, and_, case, false, func, null, true +from sqlalchemy import ( + Integer, String, and_, case, false, func, null, or_, true) from sqlalchemy.dialects.postgresql import array from sqlalchemy.sql.operators import concat_op From a01429ddc28dc5e364cd296c04668733a7f8439d Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Fri, 22 May 2020 14:45:36 +0100 Subject: [PATCH 05/21] fix: fix broken "missing" QA logic this commit fixes an issue with the QA dashboard, where checklists that should be marked as "missing" are marked as "flagged" --- apollo/submissions/qa/query_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index fa382f724..39d59cd5b 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -335,7 +335,7 @@ def get_logical_check_stats(query, form, condition): query = query.join( Participant, Participant.id == Submission.participant_id) - if 'null' in complete_expression.lower(): + if 'null' not in complete_expression.lower(): null_query = or_(*[ Submission.data[tag] == None # noqa for tag in question_codes From b15f4cd10458fee0ec99fecd884008f5f0f7269a Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Fri, 22 May 2020 15:04:04 +0100 Subject: [PATCH 06/21] fix: broken logic in QA dashboard --- apollo/submissions/qa/query_builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index 39d59cd5b..75fc21c41 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -348,7 +348,7 @@ def get_logical_check_stats(query, form, condition): (and_(null_query == False, qa_query == True, ~Submission.verified_fields.has_all(array(question_codes))), 'Flagged'), # noqa (and_(null_query == False, qa_query == True, Submission.verified_fields.has_all(array(question_codes))), 'Verified'), # noqa (and_(null_query == False, qa_query == False), 'OK'), # noqa - (or_(null_query == False, qa_query == None), 'Missing') # noqa + (or_(null_query == True, qa_query == None), 'Missing') # noqa ]) else: qa_case_query = case([ From 5d3b2e85f98468ad0c30c459998c2491e9bd159d Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Fri, 22 May 2020 16:16:48 +0100 Subject: [PATCH 07/21] feat: add typing to QA processing --- apollo/submissions/qa/query_builder.py | 188 +++++++++++++++++++------ 1 file changed, 142 insertions(+), 46 deletions(-) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index 75fc21c41..060680a53 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- '''Query builder module for checklist QA''' +import enum import operator as op from arpeggio import PTNodeVisitor, visit_parse_tree @@ -66,6 +67,13 @@ } +class OperandType(enum.IntEnum): + BOOLEAN = enum.auto() + NULL = enum.auto() + NUMERIC = enum.auto() + TEXT = enum.auto() + + class BaseVisitor(PTNodeVisitor): def __init__(self, defaults=True, **kwargs): @@ -73,15 +81,23 @@ def __init__(self, defaults=True, **kwargs): def visit_number(self, node, children): if node.value.isdigit(): - return int(node.value) + value = int(node.value) else: - return float(node.value) + value = float(node.value) + + return value, OperandType.NUMERIC + + def visit_null(self, node, children): + return node.value, OperandType.NULL def visit_factor(self, node, children): if len(children) == 1: return children[0] + multiplier = -1 if children[0] == '-' else 1 - return multiplier * children[-1] + value, op_type = children[-1] + + return multiplier * value, op_type def visit_value(self, node, children): return children[-1] @@ -90,26 +106,45 @@ def visit_exponent(self, node, children): if len(children) == 1: return children[0] - exponent = children[0] - for i in children[1:]: + exponent, l_op_type = children[0] + if l_op_type != OperandType.NUMERIC: + raise ValueError('Only numeric operands supported for *') + for item, r_op_type in children[1:]: # exponent **= i - exponent = func.pow(exponent, i) + if r_op_type != OperandType.NUMERIC: + raise ValueError('Only numeric operands supported for *') + exponent = func.pow(exponent, item) - return exponent + return exponent, l_op_type def visit_product(self, node, children): - product = children[0] + if len(children) == 1: + return children[0] + + product, l_op_type = children[0] for i in range(2, len(children), 2): + item, r_op_type = children[i] sign = children[i - 1] - product = OPERATIONS[sign](product, children[i]) - return product + if r_op_type != OperandType.NUMERIC: + raise ValueError(f'Only numeric operands supported for {sign}') + product = OPERATIONS[sign](product, item) + + return product, l_op_type def visit_sum(self, node, children): - total = children[0] + if len(children) == 1: + return children[0] + + total, l_op_type = children[0] for i in range(2, len(children), 2): + item, r_op_type = children[i] sign = children[i - 1] - total = OPERATIONS[sign](total, children[i]) + + if r_op_type != OperandType.NUMERIC: + raise ValueError(f'Only numeric operands supported for {sign}') + + total = OPERATIONS[sign](total, item) return total @@ -119,36 +154,76 @@ def visit_concat(self, node, children): self.uses_concat = True - operand = func.cast(children[0], String) + term, _ = children[0] + operand = func.cast(term, String) for i in children[1:]: operand = concat_op(operand, func.cast(i, String)) - return operand + return operand, OperandType.TEXT def visit_comparison(self, node, children): - if getattr(self, 'uses_concat', False): - comparison = func.cast(children[0], String) \ - if children[0] != 'NULL' else None + if len(children) == 1: + return children[0] + + uses_concat = getattr(self, 'uses_concat', False) + first_term, l_op_type = children[0] + if uses_concat: + comparison = func.cast(first_term, String) \ + if first_term != 'NULL' else None + l_op_type = OperandType.NULL if first_term == 'NULL' else l_op_type else: - comparison = children[0] if children[0] != 'NULL' else None + comparison = first_term if first_term != 'NULL' else None + l_op_type = OperandType.NULL if first_term == 'NULL' else l_op_type + for i in range(2, len(children), 2): sign = children[i - 1] - if getattr(self, 'uses_concat', False): - item = func.cast(children[i], String) \ - if children[i] != 'NULL' else None + term, r_op_type = children[i] + + if uses_concat: + item = func.cast(term, String) \ + if term != 'NULL' else None + r_op_type = OperandType.NULL if term == 'NULL' else r_op_type else: - item = children[i] if children[i] != 'NULL' else None + item = term if term != 'NULL' else None + r_op_type = OperandType.NULL if term == 'NULL' else r_op_type + + if l_op_type == OperandType.NULL or r_op_type == OperandType.NULL: + if sign not in ('!=', '='): + raise ValueError('Invalid comparison for null operand') + + if l_op_type != r_op_type: + if ( + l_op_type != OperandType.NULL and + r_op_type != OperandType.NULL + ): + raise ValueError('Cannot compare different types') + comparison = OPERATIONS[sign](comparison, item) - return comparison + return comparison, OperandType.BOOLEAN def visit_expression(self, node, children): - expression = children[0] if children[0] != 'NULL' else None + if len(children) == 1: + return children[0] + + first_term, l_op_type = children[0] + expression = first_term if first_term != 'NULL' else None + l_op_type = OperandType.NULL if first_term == 'NULL' else l_op_type + for i in range(2, len(children), 2): sign = children[i - 1] - expression = OPERATIONS[sign](expression, children[i]) + term, r_op_type = children[i] + + if ( + l_op_type != OperandType.BOOLEAN or + r_op_type != OperandType.BOOLEAN + ): + raise ValueError( + 'Invalid operation for non-boolean expression') + + expression = OPERATIONS[sign](expression, term) - return expression + return expression, OperandType.BOOLEAN class InlineQATreeVisitor(BaseVisitor): @@ -158,10 +233,10 @@ def __init__(self, defaults=True, **kwargs): super().__init__(defaults, **kwargs) def visit_false(self, node, children): - return False + return False, OperandType.BOOLEAN def visit_true(self, node, children): - return True + return True, OperandType.BOOLEAN def visit_variable(self, node, children): var_name = node.value @@ -172,29 +247,41 @@ def visit_variable(self, node, children): if field['type'] == 'multiselect': return 'NULL' - return self.submission.data.get(var_name, 'NULL') + field_value = self.submission.data.get(var_name, 'NULL') + if field_value == 'NULL': + op_type = OperandType.NULL + else: + if FIELD_TYPE_CASTS.get(field['type']) == Integer: + op_type = OperandType.NUMERIC + elif FIELD_TYPE_CASTS.get(field['type']) == String: + op_type = OperandType.TEXT + else: + raise ValueError(f'Unknown data type for field {var_name}') + + return field_value, op_type def visit_lookup(self, node, children): top_level_attr, symbol, name = children + op_type = OperandType.TEXT if top_level_attr in ['location', 'participant']: attribute = getattr(self.submission, top_level_attr) if symbol == '.': - return getattr(attribute, name) + return getattr(attribute, name), op_type else: - return attribute.extra_data.get(name) + return attribute.extra_data.get(name), op_type else: - return getattr(self.submission, name) + return getattr(self.submission, name), op_type def visit_comparison(self, node, children): if len(children) > 1: # left and right are indices 0 and 2 respectively - left = children[0] - right = children[2] + left = children[0][0] + right = children[2][0] if isinstance(left, str) and isinstance(right, str): # both sides are NULL - return 'NULL' + return 'NULL', OperandType.NULL return super().visit_comparison(node, children) @@ -207,26 +294,28 @@ def __init__(self, defaults=True, **kwargs): super().__init__(defaults, **kwargs) def visit_false(self, node, children): - return false() + return false(), OperandType.BOOLEAN def visit_true(self, node, children): - return true() + return true(), OperandType.BOOLEAN def visit_lookup(self, node, children): top_level_attr, symbol, name = children + op_type = OperandType.TEXT if top_level_attr == 'location': if symbol == '.': - return getattr(Location, name).cast(Integer) + return getattr(Location, name).cast(Integer), op_type else: - return Location.extra_data[name].astext.cast(Integer) + return Location.extra_data[name].astext.cast(Integer), op_type elif top_level_attr == 'participant': if symbol == '.': - return getattr(Participant, name).cast(Integer) + return getattr(Participant, name).cast(Integer), op_type else: - return Participant.extra_data[name].astext.cast(Integer) + return ( + Participant.extra_data[name].astext.cast(Integer), op_type) else: - return getattr(Submission, name).cast(Integer) + return getattr(Submission, name).cast(Integer), op_type def visit_variable(self, node, children): var_name = node.value @@ -268,8 +357,15 @@ def visit_variable(self, node, children): else: self.prev_cast_type = cast_type if cast_type is not None: - return Submission.data[var_name].astext.cast(cast_type) - return Submission.data[var_name] + if cast_type == Integer: + op_type = OperandType.NUMERIC + elif cast_type == String: + op_type = OperandType.TEXT + return ( + Submission.data[var_name].astext.cast(cast_type), op_type) + + # this is an error + raise ValueError('Unknown value type') def generate_qa_query(expression, form): @@ -282,7 +378,7 @@ def generate_qa_query(expression, form): visitor = QATreeVisitor(form=form) - return visit_parse_tree(tree, visitor), visitor.variables + return visit_parse_tree(tree, visitor)[0], visitor.variables def generate_qa_queries(form): @@ -394,7 +490,7 @@ def get_inline_qa_status(submission, condition): # most likely return None, set() - return result, used_tags + return result[0], used_tags def build_expression(logical_check): From 584f4ee35a6ee0a85756b39e9efac0ad6508a2ee Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Fri, 22 May 2020 18:10:31 +0100 Subject: [PATCH 08/21] fix: revert lookups to numeric type --- apollo/submissions/qa/query_builder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index 060680a53..559fbb932 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -262,7 +262,7 @@ def visit_variable(self, node, children): def visit_lookup(self, node, children): top_level_attr, symbol, name = children - op_type = OperandType.TEXT + op_type = OperandType.NUMERIC if top_level_attr in ['location', 'participant']: attribute = getattr(self.submission, top_level_attr) @@ -301,7 +301,7 @@ def visit_true(self, node, children): def visit_lookup(self, node, children): top_level_attr, symbol, name = children - op_type = OperandType.TEXT + op_type = OperandType.NUMERIC if top_level_attr == 'location': if symbol == '.': From fb34c7670f6b233e0346b8f04100737810a17bb3 Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Sat, 23 May 2020 07:23:45 +0100 Subject: [PATCH 09/21] feat: add rendering of QA for single-expression type --- apollo/formsframework/views_forms.py | 4 +++- apollo/templates/admin/quality_assurance.html | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/apollo/formsframework/views_forms.py b/apollo/formsframework/views_forms.py index 954476650..3b054e4da 100644 --- a/apollo/formsframework/views_forms.py +++ b/apollo/formsframework/views_forms.py @@ -247,7 +247,9 @@ def quality_controls(view, form_id): quality_control['description'] = quality_check['description'] quality_control['criteria'] = [] - if 'criteria' in quality_check: + if 'expression' in quality_check: + quality_control['expression'] = quality_check['expression'] + elif 'criteria' in quality_check: for index, criterion in enumerate(quality_check['criteria']): quality_control['criteria'].append({ 'lvalue': criterion['lvalue'], diff --git a/apollo/templates/admin/quality_assurance.html b/apollo/templates/admin/quality_assurance.html index ab0f0df74..107df757a 100644 --- a/apollo/templates/admin/quality_assurance.html +++ b/apollo/templates/admin/quality_assurance.html @@ -38,9 +38,13 @@ {{ control.name }} {{ control.description }} + {%- if 'expression' in control -%} +
{{ control.expression }}
+ {%- else -%} {%- for criterion in control.criteria %}
{% if loop.index > 1 %}{% if criterion.conjunction == '&&' %}{{ _('AND') }}{% else %}{{ _('OR') }}{% endif %}{% endif %}{{ criterion.lvalue }}{{ criterion.comparator }}{{ criterion.rvalue }}
{%- endfor %} + {%- endif -%} {%- else %} From f7a4462cf7c0068532cf34df1423b451c1c39fa2 Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Sun, 24 May 2020 17:14:05 +0100 Subject: [PATCH 10/21] feat: add data migration for QA checks this commit adds a data migration for QA checks to single expressions --- .../1e17be41a449_migrate_qa_expressions.py | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 migrations/versions/1e17be41a449_migrate_qa_expressions.py diff --git a/migrations/versions/1e17be41a449_migrate_qa_expressions.py b/migrations/versions/1e17be41a449_migrate_qa_expressions.py new file mode 100644 index 000000000..1cf36a149 --- /dev/null +++ b/migrations/versions/1e17be41a449_migrate_qa_expressions.py @@ -0,0 +1,40 @@ +"""migrate QA expressions + +Revision ID: 1e17be41a449 +Revises: 32263b7ab47e +Create Date: 2020-05-23 18:00:25.856539 + +""" +from alembic import op +import sqlalchemy as sa +from apollo.formsframework.models import Form +from apollo.submissions.qa import query_builder as qb + +# revision identifiers, used by Alembic. +revision = '1e17be41a449' +down_revision = '32263b7ab47e' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + fetch_query = sa.sql.text( + "SELECT id, quality_checks FROM form WHERE quality_checks IS NULL OR quality_checks = 'null'::jsonb;") # noqa + form_table = Form.__table__ + for form_id, quality_checks in op.execute(fetch_query).fetchall(): + for quality_check in quality_checks: + if 'expression' not in quality_check: + quality_check['expression'] = qb.build_expression( + quality_check) + statement = form_table.update().where( + form_table.c.id == form_id + ).values(quality_checks=quality_checks) + op.execute(statement) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + # ### end Alembic commands ### + pass From 67e9f185245d29e1863b3196124cd5b904050aab Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 00:04:52 +0100 Subject: [PATCH 11/21] fix: fix broken query in migration --- migrations/versions/1e17be41a449_migrate_qa_expressions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations/versions/1e17be41a449_migrate_qa_expressions.py b/migrations/versions/1e17be41a449_migrate_qa_expressions.py index 1cf36a149..903229bbb 100644 --- a/migrations/versions/1e17be41a449_migrate_qa_expressions.py +++ b/migrations/versions/1e17be41a449_migrate_qa_expressions.py @@ -20,7 +20,7 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### fetch_query = sa.sql.text( - "SELECT id, quality_checks FROM form WHERE quality_checks IS NULL OR quality_checks = 'null'::jsonb;") # noqa + "SELECT id, quality_checks FROM form WHERE quality_checks IS NOT NULL OR quality_checks != 'null'::jsonb;") # noqa form_table = Form.__table__ for form_id, quality_checks in op.execute(fetch_query).fetchall(): for quality_check in quality_checks: From 07b2513549ad8fc9e5dba814927172df064a01aa Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 06:55:40 +0100 Subject: [PATCH 12/21] chore: make PEP8 compliant --- apollo/formsframework/views_forms.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apollo/formsframework/views_forms.py b/apollo/formsframework/views_forms.py index 3b054e4da..676b93e9f 100644 --- a/apollo/formsframework/views_forms.py +++ b/apollo/formsframework/views_forms.py @@ -423,8 +423,7 @@ def export_form(id): workbook.save(memory_file) memory_file.seek(0) current_timestamp = datetime.utcnow() - filename = slugify( - f'{form.name}-{current_timestamp:%Y %m %d %H%M%S}') + '.xls' + filename = slugify(f'{form.name}-{current_timestamp:%Y %m %d %H%M%S}.xls') return send_file( memory_file, attachment_filename=filename, From 287b50c7c7619ccd03d62b1e8ae169100e45798a Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 07:07:49 +0100 Subject: [PATCH 13/21] chore: clean up QA template --- apollo/templates/admin/quality_assurance.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apollo/templates/admin/quality_assurance.html b/apollo/templates/admin/quality_assurance.html index 107df757a..3d722a89f 100644 --- a/apollo/templates/admin/quality_assurance.html +++ b/apollo/templates/admin/quality_assurance.html @@ -40,10 +40,6 @@ {%- if 'expression' in control -%}
{{ control.expression }}
- {%- else -%} - {%- for criterion in control.criteria %} -
{% if loop.index > 1 %}{% if criterion.conjunction == '&&' %}{{ _('AND') }}{% else %}{{ _('OR') }}{% endif %}{% endif %}{{ criterion.lvalue }}{{ criterion.comparator }}{{ criterion.rvalue }}
- {%- endfor %} {%- endif -%} From d444574b3f19261a085392c7953ee5727ffda3ce Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 07:08:52 +0100 Subject: [PATCH 14/21] chore: clean up QA view --- apollo/formsframework/views_forms.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/apollo/formsframework/views_forms.py b/apollo/formsframework/views_forms.py index 676b93e9f..e51bc844a 100644 --- a/apollo/formsframework/views_forms.py +++ b/apollo/formsframework/views_forms.py @@ -245,27 +245,9 @@ def quality_controls(view, form_id): quality_control['name'] = quality_check['name'] quality_control['description'] = quality_check['description'] - quality_control['criteria'] = [] if 'expression' in quality_check: quality_control['expression'] = quality_check['expression'] - elif 'criteria' in quality_check: - for index, criterion in enumerate(quality_check['criteria']): - quality_control['criteria'].append({ - 'lvalue': criterion['lvalue'], - 'comparator': criterion['comparator'], - 'rvalue': criterion['rvalue'], - 'conjunction': criterion['conjunction'], - 'id': str(index) - }) - else: - quality_control['criteria'].append({ - 'lvalue': quality_check['lvalue'], - 'comparator': quality_check['comparator'], - 'rvalue': quality_check['rvalue'], - 'conjunction': '&&', - 'id': '0' - }) quality_controls.append(quality_control) From bb8732edd847f525dd401985e9254aaeb60651f6 Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 11:41:06 +0100 Subject: [PATCH 15/21] feat: update form exports for single QA --- apollo/formsframework/utils.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apollo/formsframework/utils.py b/apollo/formsframework/utils.py index a944ee913..55f34888d 100644 --- a/apollo/formsframework/utils.py +++ b/apollo/formsframework/utils.py @@ -415,10 +415,9 @@ def export_form(form): if quality_checks and qa_sheet: row = 1 for check in quality_checks: - if 'criteria' in check: - for term in check['criteria']: - qa_sheet.write(row, 0, check['name']) - qa_sheet.write(row, 1, check['description']) + if 'expression' in check: + qa_sheet.write(row, 0, check['name']) + qa_sheet.write(row, 1, check['description']) qa_sheet.write(row, 2, term['lvalue']) qa_sheet.write(row, 3, term['comparator']) qa_sheet.write(row, 4, term['rvalue']) From 87af4615e6a3b0e1da6ef67b60797c4e175ced8e Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 11:41:50 +0100 Subject: [PATCH 16/21] feat: update form export to single expression QA --- apollo/formsframework/utils.py | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/apollo/formsframework/utils.py b/apollo/formsframework/utils.py index 55f34888d..971a02bdb 100644 --- a/apollo/formsframework/utils.py +++ b/apollo/formsframework/utils.py @@ -280,8 +280,7 @@ def export_form(form): 'accredited_voters_tag', 'invalid_votes_tag', 'registered_voters_tag', 'blank_votes_tag', 'quality_checks_enabled', 'vote_shares'] - qa_header = ['name', 'description', 'left', 'relation', 'right', - 'conjunction'] + qa_header = ['name', 'description', 'expression'] # output headers for col, value in enumerate(survey_header): @@ -418,18 +417,7 @@ def export_form(form): if 'expression' in check: qa_sheet.write(row, 0, check['name']) qa_sheet.write(row, 1, check['description']) - qa_sheet.write(row, 2, term['lvalue']) - qa_sheet.write(row, 3, term['comparator']) - qa_sheet.write(row, 4, term['rvalue']) - qa_sheet.write(row, 5, term['conjunction']) - row += 1 - else: - qa_sheet.write(row, 0, check['name']) - qa_sheet.write(row, 1, check['description']) - qa_sheet.write(row, 2, check['lvalue']) - qa_sheet.write(row, 3, check['comparator']) - qa_sheet.write(row, 4, check['rvalue']) - qa_sheet.write(row, 5, '&&') + qa_sheet.write(row, 2, check['expression']) row += 1 return book From 596ab4d6184607753ca9cfe1fdd573a365db1648 Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 13:24:22 +0100 Subject: [PATCH 17/21] fix: update form import/export --- apollo/formsframework/utils.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apollo/formsframework/utils.py b/apollo/formsframework/utils.py index 971a02bdb..a3bf854b3 100644 --- a/apollo/formsframework/utils.py +++ b/apollo/formsframework/utils.py @@ -8,6 +8,7 @@ from xlwt import Workbook from apollo.formsframework.models import Form +from apollo.submissions.qa.query_builder import build_expression from apollo.utils import generate_identifier gt_constraint_regex = re.compile(r'(?:.*\.\s*\>={0,1}\s*)(\d+)') @@ -182,6 +183,10 @@ def _process_qa_worksheet(qa_data): if 'name' in qa_dict: if current_name != qa_dict['name']: if current_check is not None: + if 'expression' not in current_check: + current_check.update( + expression=build_expression(current_check)) + current_check.pop('criteria', None) quality_checks.append(current_check) current_name = qa_dict['name'] current_check = { @@ -210,9 +215,15 @@ def _process_qa_worksheet(qa_data): 'comparator': qa_dict['relation'], 'rvalue': qa_dict['right'] } + qa_check.update(expression=build_expression(qa_check)) + qa_check.pop('comparator') + qa_check.pop('lvalue') + qa_check.pop('rvalue') quality_checks.append(qa_check) if current_check is not None: + current_check.update(expression=build_expression(current_check)) + current_check.pop('criteria', None) quality_checks.append(current_check) return quality_checks From b7670dec2a02e9042797a496eb18c69e7dc78ce1 Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 13:24:38 +0100 Subject: [PATCH 18/21] chore: remove migrated QA criteria --- migrations/versions/1e17be41a449_migrate_qa_expressions.py | 1 + 1 file changed, 1 insertion(+) diff --git a/migrations/versions/1e17be41a449_migrate_qa_expressions.py b/migrations/versions/1e17be41a449_migrate_qa_expressions.py index 903229bbb..158db7ee0 100644 --- a/migrations/versions/1e17be41a449_migrate_qa_expressions.py +++ b/migrations/versions/1e17be41a449_migrate_qa_expressions.py @@ -27,6 +27,7 @@ def upgrade(): if 'expression' not in quality_check: quality_check['expression'] = qb.build_expression( quality_check) + quality_check.pop('criteria') statement = form_table.update().where( form_table.c.id == form_id ).values(quality_checks=quality_checks) From da3b3854d75ab53c67352adda6b9c1acadac6b90 Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Mon, 25 May 2020 13:40:06 +0100 Subject: [PATCH 19/21] chore: remove Boolean constants --- apollo/submissions/qa/query_builder.py | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/apollo/submissions/qa/query_builder.py b/apollo/submissions/qa/query_builder.py index 559fbb932..61f016fb5 100644 --- a/apollo/submissions/qa/query_builder.py +++ b/apollo/submissions/qa/query_builder.py @@ -6,8 +6,7 @@ from arpeggio import PTNodeVisitor, visit_parse_tree from arpeggio.cleanpeg import ParserPEG -from sqlalchemy import ( - Integer, String, and_, case, false, func, null, or_, true) +from sqlalchemy import Integer, String, and_, case, false, func, null, or_ from sqlalchemy.dialects.postgresql import array from sqlalchemy.sql.operators import concat_op @@ -30,10 +29,8 @@ name = r'[a-zA-Z_][a-zA-Z0-9_]*' lookup = "$" ("location" / "participant" / "submission") ("." / "@") name null = "NULL" -true = "TRUE" -false = "FALSE" factor = ("+" / "-")? (number / variable / lookup / "(" expression ")") -value = null / true / false / factor +value = null / factor exponent = value (("^") value)* product = exponent (("*" / "/") exponent)* sum = product (("+" / "-") product)* @@ -232,12 +229,6 @@ def __init__(self, defaults=True, **kwargs): self.submission = kwargs.pop('submission') super().__init__(defaults, **kwargs) - def visit_false(self, node, children): - return False, OperandType.BOOLEAN - - def visit_true(self, node, children): - return True, OperandType.BOOLEAN - def visit_variable(self, node, children): var_name = node.value if var_name not in self.form.tags: @@ -293,12 +284,6 @@ def __init__(self, defaults=True, **kwargs): self.variables = set() super().__init__(defaults, **kwargs) - def visit_false(self, node, children): - return false(), OperandType.BOOLEAN - - def visit_true(self, node, children): - return true(), OperandType.BOOLEAN - def visit_lookup(self, node, children): top_level_attr, symbol, name = children op_type = OperandType.NUMERIC From 3db2fcc851ee514f567d27399d2f74073fd5c19e Mon Sep 17 00:00:00 2001 From: Odumosu 'Dipo Date: Thu, 4 Jun 2020 14:16:34 +0100 Subject: [PATCH 20/21] fix: update migration script - this commit fixes an issue with the migration script where the user receives an `AttributeError` while running it - it also doesn't assume that QA is present on the form --- migrations/versions/1e17be41a449_migrate_qa_expressions.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/migrations/versions/1e17be41a449_migrate_qa_expressions.py b/migrations/versions/1e17be41a449_migrate_qa_expressions.py index 158db7ee0..bc066e975 100644 --- a/migrations/versions/1e17be41a449_migrate_qa_expressions.py +++ b/migrations/versions/1e17be41a449_migrate_qa_expressions.py @@ -22,7 +22,10 @@ def upgrade(): fetch_query = sa.sql.text( "SELECT id, quality_checks FROM form WHERE quality_checks IS NOT NULL OR quality_checks != 'null'::jsonb;") # noqa form_table = Form.__table__ - for form_id, quality_checks in op.execute(fetch_query).fetchall(): + connection = op.get_bind() + for form_id, quality_checks in connection.execute(fetch_query).fetchall(): + if not quality_checks: + continue for quality_check in quality_checks: if 'expression' not in quality_check: quality_check['expression'] = qb.build_expression( From 34fd3e30a8e63ede34155884c9af11db505cfaa3 Mon Sep 17 00:00:00 2001 From: Odumosu Dipo Date: Tue, 9 Feb 2021 11:04:00 +0100 Subject: [PATCH 21/21] chore: reorder migrations --- migrations/versions/1e17be41a449_migrate_qa_expressions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/migrations/versions/1e17be41a449_migrate_qa_expressions.py b/migrations/versions/1e17be41a449_migrate_qa_expressions.py index bc066e975..5db152da9 100644 --- a/migrations/versions/1e17be41a449_migrate_qa_expressions.py +++ b/migrations/versions/1e17be41a449_migrate_qa_expressions.py @@ -1,7 +1,7 @@ """migrate QA expressions Revision ID: 1e17be41a449 -Revises: 32263b7ab47e +Revises: d1b58fcfbd26 Create Date: 2020-05-23 18:00:25.856539 """ @@ -12,7 +12,7 @@ # revision identifiers, used by Alembic. revision = '1e17be41a449' -down_revision = '32263b7ab47e' +down_revision = 'd1b58fcfbd26' branch_labels = None depends_on = None