openedx / openedx/openedx-platform

Break up contentstore/helpers.py

Open
#37,637 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
8.2k
Forks
4.4k
Avg merge
6d 18h
Merged PRs (30d)
42

Description

Background

This contenstore.helpers module contains a big mix of:

  • studio-specific helpers (which should arguably stay in this module), and
  • edx-platform-wide APIs which really don't belong in the CMS file tree... causing 2 problems:
    1. divergent behavior, because the helpers are re-implemented in other parts of the platform; and
    2. problematic coupling between LMS and CMS, because shared or LMS-specific modules will directly or indirectly reach into the CMS tree to use these helpers.

As an example of a function which has both problems: cms.djangoapps.contenstore.helpers.xblock_type_display_name.

  1. It has a redundant/divergent implementation in openedx.core.djangoapps.xblock.api.xblock_type_display_name. This means that default block display name are inconsistent depending on which part of the platform you're looking at.
  2. The cms version of it is problematically used by openedx.core.djangoapps.content_libraries.api.libraries. This slowed us down when working on https://github.com/openedx/edx-platform/pull/37600, we were finding that this import link was causing a failure, which didn't make sense:
    lms.djangoapps.courseware.tests.test_video_handlers -> openedx.core.djangoapps.video_config.transcripts_utils
    lms.djangoapps.courseware.tests.test_video_mongo -> openedx.core.djangoapps.video_config.transcripts_utils
    
    but it turned out that the underlying issue was this import link:
    openedx.core.djangoapps.content_libraries.api.libraries -> cms.djangoapps.contentstore.helpers
    

which exists only because the authoritative xblock_type_display_name is defined within CMS instead of a platform-wide API module.

Acceptance criteria

  • enumerate contenstore.helpers functions.
  • identify a destination for each one (stay put, move to openedx/core/djangoapps/xblock/api.py, move somewhere else).
  • identify which ones have redundant implementations somewhere else in the platform.
    • for each of conflicts, propose a way to resolve the conflict.
  • get review on the steps above before proceeding; incorporate feedback.
  • make the code changes, updating references and tests as necessary.
  • search for this GitHub issue's URL in edx-platform, and remove any associated TODOs.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by enumerating functions in cms/djangoapps/contentstore/helpers.py and searching their uses, including openedx/core/djangoapps/xblock/api.py and openedx/core/djangoapps/content_libraries/api/libraries.py. Document destinations and redundant implementations, get review on the plan, then update references and tests as needed. Done means the acceptance criteria are met and references to this issue URL and associated TODOs are removed.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.