Skip to content

feat: provisionally support V2 libraries in LibraryContentBlock (randomized only) - #33263

Merged
kdmccormick merged 78 commits into
masterfrom
kdmccormick/library-content-v2
Nov 20, 2023
Merged

feat: provisionally support V2 libraries in LibraryContentBlock (randomized only)#33263
kdmccormick merged 78 commits into
masterfrom
kdmccormick/library-content-v2

Conversation

@kdmccormick

@kdmccormick kdmccormick commented Sep 14, 2023

Copy link
Copy Markdown
Member

Co-author credit to @connorhaugh and RaccoonGang.

Rather than implementing V2-library and static-library-reference support
in a new block, we will be enhancing the existing `LibraryContentBlock`
in-place.

Relevant ADR PR: #33231
Co-Authored-By: Connor Haugh <chaugh@2u.com>
Co-Authored-By: Eugene Dyudyunov <evgen.dyudyunov@raccoongang.com>
@kdmccormick
kdmccormick force-pushed the kdmccormick/library-content-v2 branch from 02bd84a to 9eee194 Compare September 14, 2023 22:00

@kdmccormick kdmccormick left a comment

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.

Carrying over all outstanding comments from #33057

Comment thread lms/envs/common.py
Comment thread cms/djangoapps/contentstore/tests/test_libraries.py
Comment thread xmodule/library_content_block.py Outdated
Comment thread xmodule/library_tools.py Outdated
Comment thread xmodule/tasks.py Outdated
Comment thread xmodule/tasks.py Outdated
Comment thread xmodule/tasks.py Outdated
Comment thread xmodule/tasks.py Outdated
@connorhaugh
connorhaugh marked this pull request as ready for review September 19, 2023 13:35
connorhaugh and others added 6 commits September 19, 2023 14:14
...per comment in cms/djangoapps/contentstore/tasks.py.

This fixes an exception that was raised in the background whenever
the update_children_task was started:

     Traceback (most recent call last):
       File "/openedx/venv/lib/python3.8/site-packages/celery/utils/dispatch/signal.py", line 276, in send
         response = receiver(signal=self, sender=sender, **named)
       File "/openedx/venv/lib/python3.8/site-packages/user_tasks/signals.py", line 215, in start_user_task
         sender.status.start()
       File "/openedx/venv/lib/python3.8/site-packages/user_tasks/tasks.py", line 84, in status
         name = self.generate_name(arguments_dict)
       File "/openedx/edx-platform/openedx/core/djangoapps/content_libraries/tasks.py", line 274, in generate_name
         key = arguments_dict['dest_block_key']
     KeyError: 'dest_block_key'
Just like ./common/ and ./openedx/, the unit tests in ./xmodule/
validate behavior in both LMS and CMS. In order to fully test
./xmodule/, we need to run its tests in both contexts.

Also in this commit:

* refactor: Rename the shards to be clear whether they're
            running under LMS or CMS.
* docs: correct comments regarding conditions under which
        codejail's test_cant_do_something_forbidden is skipped.

* test: update a unit test which was using the now-deleted
        library_sourced block to use library_content block instead.
@open-craft-grove

Copy link
Copy Markdown

Sandbox deployment failed.

Please check the settings and requirements and retry deployment.

@kdmccormick

Copy link
Copy Markdown
Member Author

@connorhaugh Unit tests are all set now. The only failing check is a random pylint thing that I can fix Monday.

I'll be manually testing this more Monday morning. Assuming the recent commits look good to you and we don't find bugs, let's merge in the afternoon?

@connorhaugh

Copy link
Copy Markdown
Contributor

@kdmccormick I'm also giving it one final once over.

@kdmccormick kdmccormick changed the title feat: support V2 libraries in LibraryContentBlock (randomized only) feat: provisionally support V2 libraries in LibraryContentBlock (randomized only) Nov 20, 2023
@kdmccormick
kdmccormick enabled auto-merge (squash) November 20, 2023 14:59
@kdmccormick
kdmccormick disabled auto-merge November 20, 2023 15:01
@kdmccormick
kdmccormick enabled auto-merge (squash) November 20, 2023 15:09
auto-merge was automatically disabled November 20, 2023 15:09

Pull request was closed

@kdmccormick kdmccormick reopened this Nov 20, 2023
@kdmccormick
kdmccormick enabled auto-merge (squash) November 20, 2023 15:51
@kdmccormick
kdmccormick merged commit e800ae7 into master Nov 20, 2023
@kdmccormick
kdmccormick deleted the kdmccormick/library-content-v2 branch November 20, 2023 15:58
kdmccormick added a commit that referenced this pull request Nov 20, 2023
@open-craft-grove

Copy link
Copy Markdown

Sandbox deployment started.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@open-craft-grove

Copy link
Copy Markdown

Sandbox update request received. Deployment will start soon.

@open-craft-grove

Copy link
Copy Markdown

Sandbox deployment failed.

Please check the settings and requirements and retry deployment.

@open-craft-grove

Copy link
Copy Markdown

Sandbox update request received. Deployment will start soon.

2 similar comments
@open-craft-grove

Copy link
Copy Markdown

Sandbox update request received. Deployment will start soon.

@open-craft-grove

Copy link
Copy Markdown

Sandbox update request received. Deployment will start soon.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

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.

5 participants