From 9b9869908bbb7a6b1203a77b4cf5ade1a0d76f74 Mon Sep 17 00:00:00 2001 From: Brian Mesick Date: Wed, 8 Nov 2023 12:55:50 -0500 Subject: [PATCH 1/2] fix: Bump ERB to 7.0.1 for problem result fixes The new version of ERB makes user responses to problems parsable as JSON, allowing our reports to work correctly. This PR updates the ERB version and updates downstream models to parse the values correctly. --- tutoraspects/plugin.py | 2 +- .../versions/0025_problem_responses.py | 152 ++++++++++++++++++ .../queries/fact_problem_responses.sql | 7 +- 3 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 tutoraspects/templates/aspects/apps/aspects/migrations/alembic/versions/0025_problem_responses.py diff --git a/tutoraspects/plugin.py b/tutoraspects/plugin.py index 182e28226..8af60a37a 100644 --- a/tutoraspects/plugin.py +++ b/tutoraspects/plugin.py @@ -41,7 +41,7 @@ "OPENEDX_EXTRA_PIP_REQUIREMENTS", [ "openedx-event-sink-clickhouse==0.4.0", - "edx-event-routing-backends==v7.0.0", + "edx-event-routing-backends==v7.0.1", ], ), ("EVENT_SINK_CLICKHOUSE_MODELS", ["course_overviews", "user_profile"]), diff --git a/tutoraspects/templates/aspects/apps/aspects/migrations/alembic/versions/0025_problem_responses.py b/tutoraspects/templates/aspects/apps/aspects/migrations/alembic/versions/0025_problem_responses.py new file mode 100644 index 000000000..7b0663d6e --- /dev/null +++ b/tutoraspects/templates/aspects/apps/aspects/migrations/alembic/versions/0025_problem_responses.py @@ -0,0 +1,152 @@ +""" +Relies on ERB to do the right thing with responses instead of trying to +escape quotes for lists. +""" + +from alembic import op + +revision = "0025" +down_revision = "0024" +branch_labels = None +depends_on = None +on_cluster = ( + " ON CLUSTER '{{CLICKHOUSE_CLUSTER_NAME}}' " + if "{{CLICKHOUSE_CLUSTER_NAME}}" + else "" +) +engine = ( + "ReplicatedReplacingMergeTree" + if "{{CLICKHOUSE_CLUSTER_NAME}}" + else "ReplacingMergeTree" +) + + +TABLE_NAME = "{{ ASPECTS_XAPI_DATABASE }}.{{ ASPECTS_PROBLEM_EVENTS_TABLE }}" +VIEW_NAME = "{{ ASPECTS_XAPI_DATABASE }}.{{ ASPECTS_PROBLEM_TRANSFORM_MV }}" + +PROBLEM_DDL = f""" +CREATE OR REPLACE TABLE {{ ASPECTS_XAPI_DATABASE }}.{{ ASPECTS_PROBLEM_EVENTS_TABLE }} +{on_cluster} +( + `event_id` UUID NOT NULL, + `emission_time` DateTime NOT NULL, + `actor_id` String NOT NULL, + `object_id` String NOT NULL, + `course_key` String NOT NULL, + `org` String NOT NULL, + `verb_id` LowCardinality(String) NOT NULL, + `responses` String, + `scaled_score` String, + `success` Bool, + `interaction_type` LowCardinality(String), + `attempts` Int16 +) ENGINE = {engine} +PRIMARY KEY (org, course_key, verb_id) +ORDER BY (org, course_key, verb_id, emission_time, actor_id, object_id, responses, success, event_id); +""" + + +OLD_PROBLEM_QUERY = """ +SELECT + event_id, + cast(emission_time as DateTime) as emission_time, + actor_id, + object_id, + splitByString('/', course_id)[-1] AS course_key, + org, + verb_id, + replaceAll( + replaceAll( + JSON_VALUE(event_str, '$.result.response'), + '\\\'', '\\\\\\\'' + ), '"', '\\\'') as responses, + JSON_VALUE(event_str, '$.result.score.scaled') as scaled_score, + if( + verb_id = 'https://w3id.org/xapi/acrossx/verbs/evaluated', + cast(JSON_VALUE(event_str, '$.result.success') as Bool), + false + ) as success, + JSON_VALUE(event_str, '$.object.definition.interactionType') as interaction_type, + if( + verb_id = 'https://w3id.org/xapi/acrossx/verbs/evaluated', + cast(JSON_VALUE(event_str, '$.object.definition.extensions."http://id.tincanapi.com/extension/attempt-id"') as Int16), + 0 + ) as attempts +FROM + {{ ASPECTS_XAPI_DATABASE }}.{{ ASPECTS_XAPI_TABLE }} +WHERE + verb_id in ( + 'https://w3id.org/xapi/acrossx/verbs/evaluated', + 'http://adlnet.gov/expapi/verbs/passed', + 'http://adlnet.gov/expapi/verbs/asked' + ); +""" + + +NEW_PROBLEM_QUERY = """ +SELECT + event_id, + cast(emission_time as DateTime) as emission_time, + actor_id, + object_id, + splitByString('/', course_id)[-1] AS course_key, + org, + verb_id, + JSON_VALUE(event_str, '$.result.response') as responses, + JSON_VALUE(event_str, '$.result.score.scaled') as scaled_score, + if( + verb_id = 'https://w3id.org/xapi/acrossx/verbs/evaluated', + cast(JSON_VALUE(event_str, '$.result.success') as Bool), + false + ) as success, + JSON_VALUE(event_str, '$.object.definition.interactionType') as interaction_type, + if( + verb_id = 'https://w3id.org/xapi/acrossx/verbs/evaluated', + cast(JSON_VALUE(event_str, '$.object.definition.extensions."http://id.tincanapi.com/extension/attempt-id"') as Int16), + 0 + ) as attempts +FROM + {{ ASPECTS_XAPI_DATABASE }}.{{ ASPECTS_XAPI_TABLE }} +WHERE + verb_id in ( + 'https://w3id.org/xapi/acrossx/verbs/evaluated', + 'http://adlnet.gov/expapi/verbs/passed', + 'http://adlnet.gov/expapi/verbs/asked' + ); +""" + + +def drop_objects(): + op.execute( + f""" + DROP TABLE IF EXISTS {TABLE_NAME} {on_cluster} + """ + ) + + op.execute( + f""" + DROP VIEW IF EXISTS {VIEW_NAME} {on_cluster} + """ + ) + + +def upgrade(): + drop_objects() + op.execute(PROBLEM_DDL) + op.execute(f"INSERT INTO {TABLE_NAME} {NEW_PROBLEM_QUERY}") + op.execute( + f""" + CREATE MATERIALIZED VIEW {VIEW_NAME} {on_cluster} TO {TABLE_NAME} AS {NEW_PROBLEM_QUERY} + """ + ) + + +def downgrade(): + drop_objects() + op.execute(PROBLEM_DDL) + op.execute(f"INSERT INTO {TABLE_NAME} {OLD_PROBLEM_QUERY}") + op.execute( + f""" + CREATE MATERIALIZED VIEW {VIEW_NAME} {on_cluster} TO {TABLE_NAME} AS {OLD_PROBLEM_QUERY} + """ + ) diff --git a/tutoraspects/templates/openedx-assets/queries/fact_problem_responses.sql b/tutoraspects/templates/openedx-assets/queries/fact_problem_responses.sql index 94d12be8a..0962f17a9 100644 --- a/tutoraspects/templates/openedx-assets/queries/fact_problem_responses.sql +++ b/tutoraspects/templates/openedx-assets/queries/fact_problem_responses.sql @@ -16,10 +16,11 @@ select success, arrayJoin( if( - startsWith(responses, '['), - cast(responses as Array(String)), + JSONArrayLength(responses) > 0, + JSONExtractArrayRaw(responses), [responses] - )) as responses + ) + ) as responses from problem_responses where From 24dc7547e7ab1770cb20ad61cc37d4ad11d0c9e5 Mon Sep 17 00:00:00 2001 From: Brian Mesick Date: Wed, 8 Nov 2023 12:57:17 -0500 Subject: [PATCH 2/2] fix: Remove split on requirements in versions chart The requirement may not be versioned this way, for instance it breaks tutor init when using a git hash or branch for testing. --- .../openedx-assets/assets/dashboards/Operator_Dashboard.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tutoraspects/templates/aspects/build/aspects-superset/openedx-assets/assets/dashboards/Operator_Dashboard.yaml b/tutoraspects/templates/aspects/build/aspects-superset/openedx-assets/assets/dashboards/Operator_Dashboard.yaml index c6364199f..534f35c78 100644 --- a/tutoraspects/templates/aspects/build/aspects-superset/openedx-assets/assets/dashboards/Operator_Dashboard.yaml +++ b/tutoraspects/templates/aspects/build/aspects-superset/openedx-assets/assets/dashboards/Operator_Dashboard.yaml @@ -878,9 +878,9 @@ position: ## Dependencies: {% for requirement in OPENEDX_EXTRA_PIP_REQUIREMENTS %} {% if 'openedx-event-sink-clickhouse' in requirement %} - - **Clickhouse Event Sink Version**: {{ requirement.split('==')[1] }} \n + - **Clickhouse Event Sink Version**: {{ requirement }} \n {% elif 'event-routing-backends' in requirement %} - - **Event Routing Backends Version**: {{ requirement.split('==')[1] }} \n + - **Event Routing Backends Version**: {{ requirement }} \n {% endif %} {% endfor %} "