Backport completion-by-viewing logic

[jcdyer]

Brings complete-by-viewing code into edx-solutions fork of edx platform. This enables the completion system to register when a user has seen blocks that are completed by being viewed, including, most notably, html blocks.

JIRA tickets: OC-4520, MCKIN-7751

Discussions:

Dependencies: None

Screenshots:

Sandbox URL: TBD - sandbox is being provisioned.

Merge deadline: None

Testing instructions:

1. Set completion.enable_completion_tracking waffle switch.
2. Delete existing block completions.
   from completion.models import BlockCompletion
   BlockCompletion.objects.all().delete()
3. Open up your browser's network traffic monitor (F12, and navigate to the appropriate tab
4. Sign into a course, and navigate to a page with HTML blocks. Look at a page, spending at least 5 seconds on a block, and view the top and bottom of the block.
5. Verify that a POST request is made to and endpoint called .../publish_completion
6. Verify that a BlockCompletion is created for that block.
7. Delete the created BlockCompletion, then verify that viewing the block at /xblock/<UsageKey> also marks the block as complete.

Author notes and concerns:

It's proving more difficult than I anticipated to import the javascript from the location at STATIC_URL, so I've pulled that out to get a review on the rest of the code. Things I've tried:

• Using os.path.join(settings.STATIC_URL, 'bundles/CompletionOnViewService.js') in vertical_block.py, where the import currently lives. settings.STATIC_URL returns None during tests, probably because code under common does not get tested in a django context. This causes the os.path.join to raise an AttributeError (or some other exception).
• Using the SettingsService in vertical_block.py. This only provides access to XBlock-specific settings, not all django settings.
• Using ${static.url('bundles/CompletionOnViewService.js')} in various mako templates, including lms/templates/seq_module.html and lms/templates/vert_module.html. This caused the js file to get downloaded properly, but the required function (markBlockCompletedOnViewIfNeeded) was then not found. I'm not sure if the problem was some kind of namespacing issue, or that the js was loaded too late.

This is exactly the kind of dumb issue that should be easy, but always makes JS tickets take longer than I think they should for me. If you have any suggestions, I'd love to hear it. Otherwise, I'll just keep investigating.

Reviewers

• @xitij2000

Settings
N/A

[jcdyer]

@xitij2000 Just a heads up: This is nearly ready for review. I'll have testing instructions up soon.

[jcdyer]

@xitij2000 Can you please take a look when you get a moment. I have one outstanding issue I'm trying to resolve, but I think it's reviewable in spite of that issue, and I'd rather get feedback sooner than later.

[jcdyer]

This is the location of the hardcoded static path.

[jcdyer]

@xitij2000 I resolved the STATIC_URL issue.

[xitij2000]

@jcdyer Sorry for not being much help with the static url issue. I'm glad you figured it out! I've been through the code and everything seems fine. Will just test and approve now.

[xitij2000]

@jcdyer I've tested this using the steps you provided, and while it seems to work fine on a regular page, on the /xblock/<UsageKey> page I get:
ReferenceError: webpackJsonp is not defined
and
ReferenceError: markBlocksCompletedOnViewIfNeeded is not defined

The block is marked as completed though. Is this behaviour expected? Perhaps CompletionOnViewService.js can be a regular ES5 file so it doesn't need to go through webpack? That might avoid both these issues.

Other than the above this is a 👍 from me.

[jcdyer]

@xitij2000 I'll need to look into that. The /xblock/ code is backported from a WIP PR (https://github.com/edx/edx-platform/pull/18273), so it's not yet as thoroughly tested as the other part. I'd rather avoid making extensive changes to the core code if possible, as this is a backport from mainline edx platform, and we don't want to make it more painful than necessary to migrate to Hawthorne in the future.

[xitij2000]

@jcdyer If it helps, webpackJsonp is defined in /static/bundles/commons.js. It's a pretty large file though.

[jcdyer]

@xitij2000 It's a big file, but it's used in most pages (because it defines the common code), so it should be cached most of the time. I just came to the same conclusion. It's because we use CommonsChunkLoader in our webpack config. I've added it in, and everything seems to be working without complaints now.

[xitij2000]

@jcdyer 👍 I'm no longer getting the error!

[bradenmacdonald]

@ihtram FYI. I realized we didn't ping you on these PRs (sorry!), so wanted to make sure you're aware that we're merging the PRs that backport the new completion API, so we can soon move to it and then deprecate+remove progress-edx-platform-extensions. Epic is MCKIN-7598. Most of the code has already been reviewed by others at edX since it's merged to edx-platform master so should be non-controversial, but let me know if you want any additional review etc.