Skip to content

[SE-4473] Implement progress bar - #431

Merged
pkulkark merged 3 commits into
opencraft-release/koa.3-esmefrom
pooja/implement-progress-bar
Dec 13, 2021
Merged

[SE-4473] Implement progress bar#431
pkulkark merged 3 commits into
opencraft-release/koa.3-esmefrom
pooja/implement-progress-bar

Conversation

@pkulkark

@pkulkark pkulkark commented Oct 18, 2021

Copy link
Copy Markdown
Member

Description:
This PR implements the changes needed to show course progress bar on the courseware page. There are two progress bars that will be displayed: One at the course level and the other one at section/chapter level.

JIRA: SE-4473

Testing instructions:

  1. Setup your devstack with this branch.
  2. Set the feature flag SHOW_PROGRESS_BAR to true under FEATURES in lms.yml and studio.yml.
  3. Add the COMPLETION_AGGREGATOR_URL setting in lms.yml as shown below:
COMPLETION_AGGREGATOR_URL: /completion-aggregator/progress_bar
  1. Ensure completion tracking is enabled.
  2. Pip install completion-aggregator django app containing [SE-4473] Add progress bar views openedx-completion-aggregator#89.
  3. Start new course in LMS and verify that both progress bars are visible and updates correctly.

Reviewers:

@pkulkark pkulkark changed the title [SE-4473] Implement progress bar WIP: [SE-4473] Implement progress bar Oct 18, 2021
Comment thread common/lib/xmodule/xmodule/js/src/sequence/display.js
Comment thread common/lib/xmodule/xmodule/seq_module.py Outdated

@gabor-boros gabor-boros left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkulkark I tested the changes in Firefox, Safari, and Brave (which is Chromium-based) as well. It seems that X-Frame-Options tricked us. The option is set to DENY, I guess in the aggregator, which means we cannot load those iFrames for every browser. The issue could be mitigated by playing around with CSP and/or X-Frame-Options. Please see the results below.

Firefox 93.0

Screen.Recording.2021-10-22.at.18.21.45.mov

Brave: 1.31.87, Chromium: 95.0.4638.54

Screenshot 2021-10-22 at 18 28 50

Safari 15.0

Screenshot 2021-10-22 at 18 29 58

params = self._get_render_metadata(context, display_items, prereq_met, prereq_meta_info, banner_text, view, fragment)
if settings.FEATURES.get('SHOW_PROGRESS_BAR', False):
parent_block_id = self.get_parent().scope_ids.usage_id.block_id
params['chapter_completion_aggregator_url'] = '/'.join([settings.COMPLETION_AGGREGATOR_URL, str(self.course_id), parent_block_id]) + '/'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkulkark this will raise an attribute error if COMPLETION_AGGREGATOR_URL is not set. Also, the setting has no default value and not documented.

@pkulkark pkulkark Oct 27, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes! Good catch. Fixed it. Also added it in the testing instructions.

@pkulkark
pkulkark force-pushed the pooja/implement-progress-bar branch from a6f4252 to 41ce972 Compare October 27, 2021 09:42

@gabor-boros gabor-boros left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 🎉

Screenshot 2021-11-02 at 12 28 38

  • I tested what's in the test instructions
  • I read through the code

@pkulkark pkulkark changed the title WIP: [SE-4473] Implement progress bar [SE-4473] Implement progress bar Nov 8, 2021
@pkulkark
pkulkark merged commit 6af2de1 into opencraft-release/koa.3-esme Dec 13, 2021
gabor-boros pushed a commit that referenced this pull request Jan 14, 2022
* feat: Implement couse progress bar

(cherry picked from commit 6af2de1)
Signed-off-by: Gabor Boros <gabor.brs@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants