openedx-unsupported / edx-analytics-pipeline

GNU Affero General Public License v3.0
91 stars 116 forks source link

Add extraction of course_id from a block-based URL. #836

Closed doctoryes closed 4 years ago

doctoryes commented 4 years ago

https://openedx.atlassian.net/browse/DENG-522

At some point in August, the format of most course-based URLs changed from:

https://courses.edx.org/courses/course-v1:org+course+run/courseware/unit1/der_3-sequential/?activate_block_id=block-v1%3Aorg%2Bcourse%2Brun%2Btype%40sequential%2Bblock%40der_3-sequential

to:

https://courses.edx.org/xblock/block-v1:org+course+run+type@vertical+block@3848270e75f34e409eaad53a2a7f1da5?show_title=0&show_bookmark_button=0

This PR adds the parsing of course_ids from the "xblock"-based course URLs which are in events. These course_ids are currently ending up as "" (empty string) and are being ignored by the pipeline code.


Make sure that the following steps are done before merging:

codecov[bot] commented 4 years ago

Codecov Report

Merging #836 into master will increase coverage by 0.00%. The diff coverage is 90.47%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #836   +/-   ##
=======================================
  Coverage   74.01%   74.02%           
=======================================
  Files         208      208           
  Lines       23861    23890   +29     
=======================================
+ Hits        17661    17684   +23     
- Misses       6200     6206    +6     
Impacted Files Coverage Δ
edx/analytics/tasks/insights/user_activity.py 73.61% <20.00%> (-1.39%) :arrow_down:
edx/analytics/tasks/util/opaque_key_util.py 100.00% <100.00%> (ø)
edx/analytics/tasks/util/tests/test_eventlog.py 100.00% <100.00%> (ø)
...analytics/tasks/util/tests/test_opaque_key_util.py 94.00% <100.00%> (-6.00%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a58bd93...eaf2012. Read the comment docs.

doctoryes commented 4 years ago

FYI: Soon I plan to execute the plan described here:

https://openedx.atlassian.net/browse/DENG-522?focusedCommentId=498316

where I'll test this branch on actual production data, producing actual production results.

The day I've chosen to re-run is 2020-08-24, which was right before the MFE URL began to be phased in. Depending on the results of that run, I'll make further changes -or- merge and run on more days.

doctoryes commented 4 years ago

@pwnage101 @brianhw I've removed my test code from this branch and I'm ready for a re-review.