Fix #4188: Fixes for the GenerateV1StatisticsJob #4189
Conversation
core/domain/stats_jobs_one_off.py
Outdated
| @@ -429,7 +443,9 @@ def reduce(exp_id, stringified_values): | |||
| dest_states = [ | |||
| answer_group.outcome.dest | |||
| for answer_group in init_state.interaction.answer_groups] | |||
| dest_states.append(init_state.interaction.default_outcome.dest) | |||
| if init_state.interaction.default_outcome: | |||
There was a problem hiding this comment.
if init_state.interaction.default_outcome is not None:
core/domain/stats_jobs_one_off.py
Outdated
| if change_dict['old_state_name'] not in ( | ||
| state_stats_mapping): | ||
| yield ( | ||
| 'State name %s not in state stats mapping of ' |
There was a problem hiding this comment.
Good idea to catch this here too, but don't you want to distinguish the error messages for the two cases? Otherwise you won't know what triggered it.
Also, I'm not sure if we should just continue along the changelist if we find an error. Shouldn't we just ... stop, and not process that exploration? Otherwise there may be further errors down the line, e.g. if the new state name subsequently doesn't appear in the changelist.
There was a problem hiding this comment.
Makes sense. Done.
Codecov Report
@@ Coverage Diff @@
## develop #4189 +/- ##
========================================
Coverage 44.04% 44.04%
========================================
Files 363 363
Lines 22435 22435
Branches 3565 3565
========================================
Hits 9881 9881
Misses 12554 12554Continue to review full report at Codecov.
|
|
LGTM |
@seanlip PTAL
Thanks