quality assurance updates #772
Conversation
takinbo
left a comment
takinbo
left a comment
There was a problem hiding this comment.
This PR should be simplified. Rather than display a modal with an error, we should rather not return any question codes that are multiple choice, image, location or comment fields. If a QA expression for some reason contains any of these, then it should return the no data state and not attempt to compute the QA status.
apollo/formsframework/views_forms.py
Outdated
| # sanity check | ||
| expression = build_expression(quality_control) | ||
| if expression == '': | ||
| errors.add(FlagCause.EMPTY_EXPRESSION) | ||
| else: | ||
| try: | ||
| parse_tree = parser.parse(expression) | ||
| invalid_tags, multiselect_tags = verify_expression( | ||
| form, parse_tree) | ||
|
|
||
| if invalid_tags: | ||
| errors.add(FlagCause.MISSING_VARIABLE) | ||
|
|
||
| if multiselect_tags: | ||
| errors.add(FlagCause.MULTISELECT_VARIABLE) | ||
| except NoMatch: | ||
| errors.add(FlagCause.MALFORMED_EXPRESSION) | ||
|
|
||
| # the invalid tags and multi-select tags are returned | ||
| # as sets | ||
| invalid_tags = list(invalid_tags) | ||
| multiselect_tags = list(multiselect_tags) | ||
|
|
There was a problem hiding this comment.
This will probably also need clean up considering we're not using those errors anymore?
apollo/submissions/qa/messages.py
Outdated
| class FlagCause(enum.IntEnum): | ||
| EMPTY_EXPRESSION = 1 | ||
| MALFORMED_EXPRESSION = 2 | ||
| MISSING_VARIABLE = 3 | ||
| MULTISELECT_VARIABLE = 4 |
There was a problem hiding this comment.
If we're not returning any errors in the QA builder, do we still need this?
takinbo
left a comment
takinbo
left a comment
There was a problem hiding this comment.
A few extraneous unused lines and this PR should be good for merging
| def visit_variable(self, node, children): | ||
| var_name = node.value | ||
| if var_name not in self.form.tags: | ||
| self.invalid_variables.add(var_name) |
There was a problem hiding this comment.
Is there a current usecase for these?
|
|
||
| field = self.form.get_field_by_tag(var_name) | ||
| if field['type'] == 'multiselect': | ||
| self.multiple_choice_variables.add(var_name) |
| self.variables.add(var_name) | ||
| if var_name not in self.form.tags: | ||
| self.lock_null = True | ||
| self.invalid_variables.add(var_name) |
|
|
||
| if field['type'] == 'multiselect': | ||
| self.lock_null = True | ||
| self.multiple_choice_variables.add(var_name) |
dodumosu
force-pushed
the
issues-97-99-qa-updates
branch
from
0e956f4 to
a2729c0
Compare
this commit restricts QA to `integer`, `select` and `string` question types
dodumosu
force-pushed
the
issues-97-99-qa-updates
branch
from
a2729c0 to
4c56a1d
Compare
takinbo
left a comment
takinbo
left a comment
There was a problem hiding this comment.
Could you please test this and confirm it's working? I applied the PR to a working system and all the questions are no longer visible in the QA builder.
|
i'm unable to replicate the issue you have had. perhaps you could send a dump of your database? the screenshots below are also from a replicate of a working system
|
2 participants
this PR contains the following changes to how Apollo works:
Missing