Kolibri: Add something like `is_leaf` Boolean field to serialized ContentNodes to replace common "leaf or branch" checks in JS code

Created on 19 Feb 2021  路  8Comments  路  Source: learningequality/kolibri

In the ContentNodeViewSet (and other related viewsets), it would be nice to have a field like is_leaf that returns true whenever the node is a resource/leaf and false if it is a topic or a channel.

There are dozens of places in the JS code where we have to check if this is the case based on the kind field that could be simplified if we had this field.

image

tech update / debt

Most helpful comment

Noting that on the backend TOPIC should be the kind on every node that isn't a leaf node (even the channel root node), wherever the special CHANNEL kind is being set it is only on the frontend.

So, the annotated field on the ContentNodeViewset would just be a simple boolean check of kind == 'TOPIC'.

All 8 comments

@jonboiser I would like to work on this issue

@jonboiser The constants defined for the content kinds in le_utils.constants.content_kinds don't include CHANNEL, but in the JS code ContentNodeKinds.CHANNEL has been referred several times. So, should the is_leaf boolean field return false only when the resource is TOPIC, as CHANNEL kind does not exist in le_utils.constants.content_kinds, or am I missing something here?
https://github.com/learningequality/le-utils/blob/6febaf26fd11433fe6668d310c3db8d4c50f3522/le_utils/constants/content_kinds.py#L10-L19

On the Python side, you can tell that a ContentNode is a channel if its parent field is None. So you can have is_leaf = True when kind is TOPIC or ~parent is None~ (simpler solution in comment below)

Noting that on the backend TOPIC should be the kind on every node that isn't a leaf node (even the channel root node), wherever the special CHANNEL kind is being set it is only on the frontend.

So, the annotated field on the ContentNodeViewset would just be a simple boolean check of kind == 'TOPIC'.

Noting that on the backend TOPIC should be the kind on every node that isn't a leaf node (even the channel root node), wherever the special CHANNEL kind is being set it is only on the frontend.

So, the annotated field on the ContentNodeViewset would just be a simple boolean check of kind == 'TOPIC'.

@jonboiser @rtibbles Thanks for clearing that.

So, after adding the is_leaf field on the ContentNodeViewset, checks on the frontend like kind === ContentNodeKinds.TOPIC || kind === ContentNodeKinds.CHANNEL can be replace by a simple !is_leaf. Am I correct?

So, after adding the is_leaf field on the ContentNodeViewset, checks on the frontend like kind === ContentNodeKinds.TOPIC || kind === ContentNodeKinds.CHANNEL can be replace by a simple !is_leaf. Am I correct?

Yes, that's right. I think we can even safely replace the kind === ContentNodeKinds.TOPIC check (without the check for CHANNEL) in most places as well.

@jonboiser @rtibbles I found that when channels are fetched from the backend, kind: ContentNodeKinds.CHANNEL is being explicitly set in the frontend, so, should I also set the is_leaf field on the frontend explicitly or should I update the ChannelMetadataSerializer and PublicChannelSerializer in the backend to add the is_leaf field?

I believe that you do not need to set is_leaf as long as that field is being returned from the API. But in the past, we have had frontend code that transforms the ContentNode data coming from the API by renaming fields or filtering out fields. If you see that is_leaf is not appearing on the front-end, it might be because it has been transformed by a function like that unexpectedly.

so, should I also set the is_leaf field on the frontend explicitly or should I update the ChannelMetadataSerializer and PublicChannelSerializer in the backend to add the is_leaf field?

But in the case of this one transformer

https://github.com/learningequality/kolibri/blob/940d6b3712491dc56236f153a8ad029146b383b1/kolibri/plugins/learn/assets/src/modules/coreLearn/utils.js#L8-L18

...node is spread into the return value, so I believe if you added is_leaf to the serializer, it will be available to the frontend as well.

Was this page helpful?
0 / 5 - 0 ratings