Skip to content

Implement "Edit on Git" link for each section#1

Merged
tecoholic merged 15 commits intomainfrom
tecoholic/BB-6206-edit-links-poc
Aug 25, 2022
Merged

Implement "Edit on Git" link for each section#1
tecoholic merged 15 commits intomainfrom
tecoholic/BB-6206-edit-links-poc

Conversation

@tecoholic
Copy link
Member

@tecoholic tecoholic commented Jun 24, 2022

Description

The first feature functional PR to the edit_links Open edX extension.

  • Adds a model with admin view to configure course specfiic repositories
    and the base_url to the edit interfaces of the source code hosting
    services that hold the course information.
  • Implements the pipeline for
    org.openedx.learning.vertical_block_child.render.started.v1 event, that can
    generate the "Edit this on " links to the XBlocks based
    on the XBlock IDs

JIRA:

Dependencies:

Installation instructions:

This assumes you are testing this on a devstack.

  1. Checkout the edx-platform PR branch. Note: add the open-craft remote if you don't have it already.
    git checkout open-craft/tecoholic/add-filter-hook-to-vertical-block-rendering
    
  2. Clone the edx-filters repository to the src folder and checkout to the corresponding PR branch
    cd <your-devstack-location>/src
    git clone https://github.com/open-craft/openedx-filters.git
    # if you already have openedx repo checkout, add the open-craft remote and fetch the PR branch
    git checkout tecoholic/add-vertical-block-child-render-started-filter
    
  3. Clone the repository to your src directory in your devstack setup and checkout to the PR branch
    git clone https://github.com/open-craft/openedx-edit-links.git
    git checkout tecoholic/BB-6206-edit-links-poc
    
  4. Drop into LMS shell and install the openedx-filters extension and the openedx-edit-links extension
    make lms-shell
    pip install -e /edx/src/openedx-filters
    pip install -e /edx/src/openedx-edit-links
    
  5. Add the following configuration to lms/envs/private.py of edx-platform
    OPEN_EDX_FILTERS_CONFIG = {
            "org.openedx.learning.vertical_block_child.render.started.v1": {
                "fail_silently": False,
                "pipeline": [
                    "edit_links.pipeline.AddEditLink"
                ]
            }
        }
    EDIT_LINKS_PLUGIN_GIT_REPOS = {
    }
    EDIT_LINKS_PLUGIN_GIT_EDIT_LABEL = "Gitlab"
  6. Restart the LMS make lms-restart

Testing instructions:

One the extension is installed and configured as above, we can test the extension using the MOOC FLOSS course content.

  1. Download the course folder from https://gitlab.com/mooc-floss/mooc-floss/-/tree/master/course as a .tar.gz file.
    poc-download-course
  2. Create a new course in studio and go to Tools > Import and upload the .tar.gz file just downloaded.
  3. If you face a "problem-builder" related error during the import, install the Problem Builder XBlock in both studio and lms containers and then try the import process again.
    make lms-shell
    pip install xblock-problem-builder
    make studio-shell
    pip install xblock-problem-builder
    
  4. Ensure the course is successfully imported and the units are loading in the LMS. Make a note of the course ID eg., course-v1:FLOSS+OSS101+2022_T4
  5. Edit the EDIT_LINKS_PLUGIN_GIT_REPOS value in private.py to look like the following (use your course's ID)
    EDIT_LINKS_PLUGIN_GIT_REPOS = {
        "course-v1:FLOSS+OSS101+2022_T4": "https://gitlab.com/-/ide/project/mooc-floss/mooc-floss/edit/master/-/course/html/"
    }
    
  6. Now re-open the course in LMS, each section of the unit should have a Edit in Gitlab link on the top.
    Screenshot with Edit Links
  7. Clicking the link should take the user directly to the Gitlab IDE with the contents of the unit available as editable HTML.

Reviewers:

cc: @antoviaque @pomegranited

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed
  • openedx-filters version changed from custom branch to release version

Post merge:

  • Create a tag
  • Check new version is pushed to PyPI after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

Author concerns:

We use a protected method of the XBlock to keep the HTML of the Edit Link from raising error due to XBlock's render function auto-saving any changes.

The first feature commit to the edit_links Open edX extension.

* Adds a model with admin view to configure course specfiic repositories
  and the base_url to the edit interfaces of the source code hosting
  services that hold the course information.
* Implements the pipeline for
  `org.openedx.learning.xblock.render.started.v1` event, that can
  generate the "Edit this on <some-service>" links to the XBlocks based
  on the XBlock IDs
@navinkarkera
Copy link
Member

@tecoholic We should probably get two edit links or open both the files in online ide for cases of a single section divided into two files like below. What do you think?

image

This chapter corresponds to two files in the repo:

image

image

@tecoholic
Copy link
Member Author

@tecoholic We should probably get two edit links or open both the files in online ide for cases of a single section divided into two files like below. What do you think?

@navinkarkera I am not really sure how to handle this situation.

From the UX point of view I see the following issues:

  1. Opening multiple tabs - as in 1 tab per file would be a poor UX as the web IDEs provide a single window to edit multiple files.
  2. I tried researching the Gitlab URL pattern for opening multiple files in the Gitlab IDE, there doesn't seem to be one. However, the IDE does open multiple files when opening a PR using the PR Link. But, doesn't have a way to pass multiple files through a single URL. Also I haven't tested this in Github Editor yet, not sure what the support for multiple files are there.

Since this is a PoC, I am thinking opening the first file should be sufficient. Multiple file support can be added later.

@tecoholic tecoholic changed the title [BB-6206] Implement a way to add "Edit" links based on block ids Implement a way to add "Edit" links based on block ids Jul 24, 2022
@tecoholic tecoholic changed the title Implement a way to add "Edit" links based on block ids Implement "Edit on Git" link for each section Jul 24, 2022
@tecoholic
Copy link
Member Author

@navinkarkera I have updated the PR with new changes, including re-implementing the filter following the guidelines from this issue description relating to configuration, readme documentation and translations. Kindly review when you have the time.

@tecoholic tecoholic self-assigned this Jul 24, 2022
@tecoholic tecoholic added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 24, 2022
@navinkarkera
Copy link
Member

@tecoholic LGTM 👍, we just need to make sure that the tests pass. (Adding _clear_dirty_fields to mock HtmlBlockWithMixins should work).

Just one question, Where can I find the source code for HtmlBlockWithMixins?

  • I tested this: (describe what you tested)
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation
  • I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

@tecoholic
Copy link
Member Author

@navinkarkera Thanks fore reviewing this.

@tecoholic LGTM +1, we just need to make sure that the tests pass. (Adding _clear_dirty_fields to mock HtmlBlockWithMixins should work).

Good catch. I will implement it.

Just one question, Where can I find the source code for HtmlBlockWithMixins?

It is custom class created by the XBlock Runtime using the Mixologist. It takes the HTMLXBlock and resolves the extra mixins and generates a new class with the name "HTMLBlock" + "WithMixins".

@tecoholic
Copy link
Member Author

The PR will be merged once the dependencies on openedx-filters are merged and its version is set in the requirements.

EDIT_LINKS_PLUGIN_GIT_REPOS = {
"course-v1:my+awesome+course": "https://gitlab.com/awesome-course/-/tree/master/course/",
"course-v1:foss+course+2022": "https://gitlab.com/foss-course/-/tree/master/2022/course/",
}
Copy link
Member

Choose a reason for hiding this comment

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

If you want, you could also make it so users can configure this per-course in Studio.

If you enable ENABLE_OTHER_COURSE_SETTINGS , the Studio Advanced Settings page will show "Other Course Settings", and you could put the URL in there like { "git_repo": "https://gitlab.com/awesome-course/-/tree/master/course/" }

No need to change anything for now, but if people want an easier way to configure this in the future, that's what I'd recommend.

Instead of replacing the HTMLBlock's data, this commit replaces the
get_html function with a wrapped function which adds the edit link to
the html. This lets us avoid using protected function to clean dirty
fields.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants