feat: api for adding, removing and updating components in container #36434

navinkarkera · 2025-03-24T10:51:34Z

Description

Rest API to add, remove and update components under containers in content libraries.

Useful information to include:

Which edX user roles will this change impact?: "Developer".

Supporting information

Part of: Backend: REST API to create a new component within a unit frontend-app-authoring#1706 and Backend: Allow adding existing components when creating a unit via the REST API. frontend-app-authoring#1705
Depends on:
- feat: api for updating children in container [FC-0083] openedx-learning#292
- Basic CRUD REST Endpoints for units in content libraries [FC-0083] #36371
Private-ref: FAL-4109

Testing instructions

Read code
Verify the tests and make sure they are passing.

Deadline

"None" if there's no rush, or provide a specific date or event (and reason) if there is one.

Concerns

Enforce restrictions on which block types are allowed.: Not sure about this one. Do we have a fixed list of allowed blocks that we want to restrict via rest api or is this configurable per unit/library etc.?

openedx-webhooks · 2025-03-24T10:51:39Z

Thanks for the pull request, @navinkarkera!

This repository is currently maintained by @openedx/wg-maintenance-edx-platform.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

rpenido

LGTM 👍
Thank you for your work, @navinkarkera!

I tested this using the instructions from the PR, and manually calling the endpoint
I read through the code
I checked for accessibility issues
Includes documentation

rpenido · 2025-03-26T19:59:29Z

openedx/core/djangoapps/content/search/api.py

+    """
+    Creates, updates, or deletes the document for the given Library Container in the search index.
+
+    TODO: add support for indexing a container's components, like upsert_library_collection_index_doc does.


This would be part of another task?

Not sure if we need this right now as search is not planned yet for units. If we add containers to each component index document similar to collections, we'll need to update this function to reindex each child component.

pomegranited

Hi @navinkarkera -- this is looking really good for the "add existing components to a container" use case, but we're missing a view that lets us "add new component to a container".

Have a couple more concerns inline, please feel free to push back.

Note to self: code review done by comparing to braden/units-api

pomegranited · 2025-03-27T01:13:22Z

openedx/core/djangoapps/content_libraries/api/containers.py

+    )
+
+
+def get_container(container_key: LibraryContainerLocator) -> authoring_models.Container:


@navinkarkera I think our API should still only return ContainerMetadata as originally designed.
But see https://github.com/open-craft/edx-platform/pull/757/files#diff-5a248b18ae17c86d454776cb48e0372a8cbdc9610b3886f175a116f9b280e681R118 -- I did end up adding an internal method to get the Container object to save some code duplication. Feel free to pull in that code?

Open for discussion.

Nice! I had initially used it outside of the file, but changed it later on. I'll pull changes from 757.

pomegranited · 2025-03-27T01:13:57Z

openedx/core/djangoapps/content_libraries/api/containers.py

+    return container
+
+
+def get_container_metadata(container_key: LibraryContainerLocator) -> ContainerMetadata:


See above -- get_container should return ContainerMetadata, so no need for this added method.

pomegranited · 2025-03-27T07:01:05Z

openedx/core/djangoapps/content_libraries/api/libraries.py

    published_by: str = ""
    last_draft_created: datetime | None = None
    # The username of the user who created the last draft.
    last_draft_created_by: str = ""
    has_unpublished_changes: bool = False
    collections: list[CollectionMetadata] = field(default_factory=list)
+    can_stand_alone: bool = True


I don't think we should expose can_stand_alone on the dataclasses -- it's for internal use only.

See below: #36434 (comment), we need the frontend to set this flag to False whenever a new component is created under a container like unit.

Ah ok -- I missed that subtlety, that's fine :)

But could you add some documentation about the parameters to create_library_block to explain what this flag is for to users of the API?

@pomegranited Done. cb9ea4f

openedx/core/djangoapps/content_libraries/rest_api/serializers.py

pomegranited · 2025-03-27T07:10:28Z

openedx/core/djangoapps/content_libraries/rest_api/serializers.py

@@ -298,7 +342,7 @@ def to_internal_value(self, value: str) -> UsageKeyV2:
            raise ValidationError from err


-class ContentLibraryCollectionComponentsUpdateSerializer(serializers.Serializer):
+class ContentLibraryComponentKeysSerializer(serializers.Serializer):


Ah, that's better, thank you. :)

openedx/core/djangoapps/content_libraries/rest_api/containers.py

pomegranited

👍 Thank you for answering my questions @navinkarkera !

Just one nit re documentation for create_library_block, since we're exposing that can_stand_alone flag, but this looks and works great.

~~I tested this~~ -- I checked that the functionality is covered by unit tests
I read through the code by comparing against braden/units-api
~~I checked for accessibility issues by using my keyboard to navigate~~ N/A
Includes documentation
User-facing strings are extracted for translation

edx-pipeline-bot · 2025-03-31T13:24:03Z

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

edx-pipeline-bot · 2025-03-31T13:44:24Z

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

edx-pipeline-bot · 2025-03-31T13:44:24Z

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

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 24, 2025

navinkarkera marked this pull request as draft March 24, 2025 10:53

navinkarkera force-pushed the navin/fal-4109/container-components branch from f82a882 to 6f298a5 Compare March 25, 2025 04:44

navinkarkera marked this pull request as ready for review March 25, 2025 05:12

rpenido approved these changes Mar 26, 2025

View reviewed changes

rpenido reviewed Mar 26, 2025

View reviewed changes

navinkarkera force-pushed the navin/fal-4109/container-components branch from 6f298a5 to 2cfdbad Compare March 27, 2025 05:33

pomegranited requested changes Mar 27, 2025

View reviewed changes

pomegranited mentioned this pull request Mar 27, 2025

feat: api for updating children in container [FC-0083] openedx/openedx-learning#292

Merged

navinkarkera force-pushed the navin/fal-4109/container-components branch from 2cfdbad to 5950aed Compare March 27, 2025 10:21

pomegranited approved these changes Mar 27, 2025

View reviewed changes

pomegranited mentioned this pull request Mar 28, 2025

Epic 12: Units can be created and edited in Libraries openedx/frontend-app-authoring#1594

Open

navinkarkera force-pushed the navin/fal-4109/container-components branch 2 times, most recently from cb9ea4f to 8e004fa Compare March 28, 2025 13:31

navinkarkera added 12 commits March 31, 2025 17:14

feat: add components to container api

ea64255

feat: remove and replace components in container api

e49762e

refactor: container childern api

704dd37

chore: fix lint issues

985a884

temp: install openedx-learning dev branch

185c9ca

feat: update publish_status and children count in index

95dbca4

chore: fix mypy issues

48f54dc

test: fix reindex test

2f67069

refactor: rebase and fix conflicts

f002ef4

test: update test to check signals

453b632

docs: document can_stand_alone flag

50fb807

chore: bump openedx-learning version

d1d5b8e

navinkarkera force-pushed the navin/fal-4109/container-components branch from 8e004fa to d1d5b8e Compare March 31, 2025 11:49

navinkarkera merged commit bcaa79c into openedx:master Mar 31, 2025
49 checks passed

navinkarkera deleted the navin/fal-4109/container-components branch March 31, 2025 12:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: api for adding, removing and updating components in container #36434

feat: api for adding, removing and updating components in container #36434

navinkarkera commented Mar 24, 2025 •

edited

Loading

openedx-webhooks commented Mar 24, 2025 •

edited

Loading

rpenido left a comment

rpenido Mar 26, 2025

navinkarkera Mar 27, 2025

pomegranited left a comment •

edited

Loading

pomegranited Mar 27, 2025

navinkarkera Mar 27, 2025

pomegranited Mar 27, 2025

pomegranited Mar 27, 2025

navinkarkera Mar 27, 2025

pomegranited Mar 27, 2025

navinkarkera Mar 28, 2025

pomegranited Mar 27, 2025

pomegranited left a comment

edx-pipeline-bot commented Mar 31, 2025

edx-pipeline-bot commented Mar 31, 2025

edx-pipeline-bot commented Mar 31, 2025

		)


		def get_container(container_key: LibraryContainerLocator) -> authoring_models.Container:

		return container


		def get_container_metadata(container_key: LibraryContainerLocator) -> ContainerMetadata:

feat: api for adding, removing and updating components in container #36434

feat: api for adding, removing and updating components in container #36434

Conversation

navinkarkera commented Mar 24, 2025 • edited Loading

Description

Supporting information

Testing instructions

Deadline

Concerns

openedx-webhooks commented Mar 24, 2025 • edited Loading

rpenido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pomegranited left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pomegranited left a comment

Choose a reason for hiding this comment

edx-pipeline-bot commented Mar 31, 2025

edx-pipeline-bot commented Mar 31, 2025

edx-pipeline-bot commented Mar 31, 2025

navinkarkera commented Mar 24, 2025 •

edited

Loading

openedx-webhooks commented Mar 24, 2025 •

edited

Loading

pomegranited left a comment •

edited

Loading