Add to community library on submission approve by Jakoma02 · Pull Request #5228 · learningequality/studio

Jakoma02 · 2025-07-30T14:01:41Z

Summary

This PR implements that when a community library submission is approved, the channel is made available in kolibri_public.

Detailed changes:

Extended ChannelMetadata model in kolibri_public with categories and countries fields
Added a new server-only ADDED_TO_COMMUNITY_LIBRARY change
Implemented a add_to_community_library handler for handling the change
Moved logic for exporting a channel to kolibri public to an export_channel_to_kolibri_public util function
Modified ChannelMapper logic to support submitting to the community library
Tests testing the new functionalities

I also tested manually by creating and then approving a submission via the browsable API, and checking that the celery task for applying changes succeeded.

References

Solves #5171.

Reviewer guidance

It might be a good idea to double-check whether the representations of categories and countries at different places match the expectation: categories are represented as comma-separated strings for ContentNodes and as dicts with all values True elsewhere, and countries are represented as a list of country codes inside the ADD_TO_COMMUNITY_LIBRARY change, and as either a list or a QuerySet of Country models elsewhere.

AlexVelezLl

Thanks @Jakoma02! Great work! This is super exciting! I have manually tested the entire workflow, and almost everything was working as expected! I left some comments on things we can refine a little more, but overall this looks great!

AlexVelezLl · 2025-07-31T22:57:59Z

contentcuration/contentcuration/viewsets/channel.py

+        for change in changes:
+            try:
+                self.add_to_community_library(
+                    channel_id=change["channel_id"],


Here we should use the change's key property instead

Could you please help me understand why using the key attribute is better? It seems to me that the channel_id attribute is set correctly in the Change serialize method and using channel_id seems more clear and descriptive to me at this point.

Because of my favorite argument: "For consistency" If you see all "_from_changes" methods https://github.com/search?q=repo%3Alearningequality%2Fstudio%20_from_changes&type=code, they all use the key property to identify the instance the change is being applied to, even for the changes that are only applied to channels like publish, pubilsh_next, deploy, and sync. Following a consistent behavior makes the code more maintainable. It is true that for now it may work fine just setting the change channel_id, but If in the future we want to make decisions around the key or the channel_id attributes in the Change model, it will be easier to update if we use them consistently always.

Thanks for explaining, changed in e6d9950.

AlexVelezLl · 2025-07-31T22:59:30Z

contentcuration/contentcuration/viewsets/channel.py

+
+        if not channel.public:
+            raise ValidationError(
+                "Only public channels can be added to the community library"


I like this double check here!

However, I don't think this is true. Private channels can be added to the community library. Which will then be mapped to the kolibri_public models, but with the public field set to False. On the contrary, now that I think about it, we should not allow public channels to be submitted to be part of the Community Library, as this would result in the channel becoming private when mapped to the kolibri_public models.

@Jakoma02 Could you add a check for this in the CommunityLibrarySubmission creation, please? We shouldnt allow create submissions for public channels in the first place (We can also solve this in a follow up issue if it works better for you 👐).

You are right, the condition I put there makes no sense -- I actually meant to not check whether the channel is public, but whether it is published, but that is already enforced by checking the channel version. I added the check when creating a submission in 611ccd1.

This reminds me that we should probably also handle the case when a channel is included in the community library, and later it is requested to be made public. But that should probably be handled in a separate PR.

AlexVelezLl · 2025-07-31T23:01:53Z

contentcuration/contentcuration/viewsets/channel.py

+            channel_id=channel_id,
+            channel_version=channel_version,
+            public=False,  # Community library
+            categories=categories,


Apologies I didn't thought about this before, but the main motivation for storing the categories as a dictionary was because of the changes architecture, and being consistent with the contentcuration app represetantion of the categories. But since we are now passing these categories to the kolibri_public model where none of the previous reasons are longer true, I think now it is more appropiate to store these categories as array, as you originally suggested 😅. What do you think? I think the best place to make this transformation would be here, where we call this method from the kolibri_public app.

I also think that if categories should be represented as a dict as long as we are working with changes, the best place to change the representation would be here (I have to admit that I am not able to appreciate why this dict representation is practical, but that is likely because I am not familiar with the frontend handling of changes yet 😅).

I changed this in 550fe78 to be represented as a list in the add_to_community_library method. I also tried adding notes documenting which representation is expected where, but I am not sure what the right approach is to take — perhaps introducing docstrings to all methods dealing with categories and explaining it there would be better.

AlexVelezLl · 2025-07-31T23:03:02Z

contentcuration/contentcuration/viewsets/community_library_submission.py

    @action(
        methods=["post"],
        detail=True,
        serializer_class=CommunityLibrarySubmissionResolveSerializer,


I did not catch this before, but it seems that if we don't send any countries in this resolve request, the countries that the submission originally had get removed, and therefore we don't have the countries set once they are mapped to the kolibri_public models. Could you take a look, please? I suspect it has something to do with the serializer.

Thanks for noticing this! Partial updates without the countries field were incorrectly handled inside the update method in CommunityLibrarySubmissionSerializer, fixed in 6f5bdfb.

AlexVelezLl · 2025-07-31T23:06:21Z

contentcuration/kolibri_public/utils/mapper.py

+
+            set_channel_metadata_fields(
+                self.mapped_channel.id,
+                channel_version=self.channel_version,


Seems like we don't need to pass the channel_version to this method, since the channel already has its version set when it is copied in the _map_model method. And since this is the only place where we use this property, seems like we don't even need to get this info in the class constructor

It seems that you are right, I removed the argument in 8a0235e.

AlexVelezLl · 2025-07-31T23:07:39Z

contentcuration/kolibri_public/tests/test_export_channels_to_kolibri_public.py

+
+
+class CustomizedMixer(Mixer):
+    """Slightly modified Mixer that works correctly with the active


AlexVelezLl · 2025-08-01T20:30:02Z

contentcuration/contentcuration/viewsets/channel.py

+        if channel_version <= 0 or channel_version > channel.version:
+            raise ValidationError("Invalid channel version")
+
+        export_channel_to_kolibri_public(


Oh, something I forgot to describe in the issue, apologies. Here, we should set the status of the approved CommunityLibrarySubmission to Live, and update the previous Live submission, if any, to be just Approved again.

This is because this Live status will represent that the described channel version is currently live in the Community Library. And will also help us recognize when the task has finished and the channel is finally live.

That makes sense, I completely forgot about setting the submission as live. Implemented in b8fee43.

AlexVelezLl · 2025-08-05T20:04:37Z

contentcuration/contentcuration/viewsets/channel.py

+
+        # Mark the submission corresponding to the channel version
+        # as the only live submission for the channel
+        CommunityLibrarySubmission.objects.filter(


(Non-blocking) I was just reflecting if the channels viewset is the best place to hold this logic. Sounds like it is business logic intrinsec to the Community Library Submission, and that perhaps we can have this logic inside the CommunityLibrarySubmission model as a class method or as an instance method, and here just do something like:

CommunityLibrarySubmission.mark_live(channel_id, channel_version)

or

new_live_submission = CommunityLibrarySubmission.objects.get( channel_id=channel_id, channel_version=channel_version, ) new_live_submission.mark_live()

And even validate that the submission that is being marked as live is approved.

Would like to know your thoughts @Jakoma02 :)

I agree that the submission model is probably a better place to do this. I am also torn between the class method and instance method, but I am slightly leaning towards the instance method, as marking a submission as live is an action acting on the submission, even though it changes other submissions as well.

Yeah, either direction is fine I think!

Implemented the change in c2dec46.

AlexVelezLl

Thanks a lot @Jakoma02!

Jakoma02 added 2 commits July 30, 2025 14:00

Add to community library on submission resolve

0043f6d

Fix migration dependency

93f2810

AlexVelezLl self-assigned this Jul 30, 2025

AlexVelezLl linked an issue Jul 30, 2025 that may be closed by this pull request

ESoCC: Add a channel to the community library once its submission has been approved #5171

Closed

6 tasks

Jakoma02 mentioned this pull request Jul 31, 2025

Ensure channel version database exists when adding to community library #5233

Merged

AlexVelezLl requested changes Jul 31, 2025

View reviewed changes

AlexVelezLl reviewed Aug 1, 2025

View reviewed changes

Jakoma02 added 8 commits August 4, 2025 16:02

Use change key instead of channel_id

e6d9950

Don't allow creating community library submissions for public channels

611ccd1

Fix tests to test with non-public channels

7fce8bc

Set submission as live when added to community library

b8fee43

Fix handling partial submission update without countries

6f5bdfb

Remove explicit channel_version argument for ChannelMapper

8a0235e

Fix tests expecting the channel_version argument

9152aeb

Represent categories as a list after passing from changes

550fe78

Jakoma02 requested a review from AlexVelezLl August 4, 2025 21:00

AlexVelezLl reviewed Aug 5, 2025

View reviewed changes

Move logic to mark live submission to submission model

c2dec46

AlexVelezLl approved these changes Aug 6, 2025

View reviewed changes

AlexVelezLl merged commit 399bba1 into learningequality:community-channels Aug 6, 2025
11 checks passed

AlexVelezLl mentioned this pull request Aug 6, 2025

ESoCC: Add a channel to the community library once its submission has been approved #5171

Closed

6 tasks



		class CustomizedMixer(Mixer):
		"""Slightly modified Mixer that works correctly with the active

Conversation

Jakoma02 commented Jul 30, 2025

Summary

References

Reviewer guidance

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jakoma02 Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Jakoma02 Aug 4, 2025 •

edited

Loading