Save channel included categories and expose channel version history#5176
Conversation
AlexVelezLl
left a comment
There was a problem hiding this comment.
LGTM! Code changes makes sense, unit tests give confidence on the changes, and manual QA passes. Just found a potentially unintentional change, after we revert this change, we can proceed with the merge :).
| def channel_language_exists( | ||
| self, request, pk=None | ||
| ) -> Union[JsonResponse, HttpResponse]: | ||
| def channel_language_exists(self, request, pk=None) -> JsonResponse: |
There was a problem hiding this comment.
Was there any motivation to change this line? Seems like an unintentional change.
There was a problem hiding this comment.
Oh no, this was not intentional... this change was suggested to me by Copilot code review, I did not realize it was suggesting changes for a line I haven't even touched. Also, I remember checking that HttpResponse is indeed not returned from the function, I must have been blind 🙈 On the other hand, I in fact had the types wrong in the function I did write, and Copilot did not catch that 🤷 It definitely seems I need to be more careful with this in the future, thanks for catching this! Fixed in 73b72be.
There was a problem hiding this comment.
Or maybe Copilot was right and discovered the actual mistake, and I just messed up when fixing it... I'm not 100% sure right now
dc2eb2c
into
learningequality:community-channels
Summary
This PR includes the list of included categories and licenses in the Channel
published_datafield and adds aget_published_datachannel action to query the published data.Detailed changes:
fill_published_fieldsmethod was modified to include theincluded_licensesandincluded_categoriesfields inpublished_dataon channel publishget_published_dataaction was added toChannelViewSetNo manual testing other than designing the tests and checking that they pass was done.
References
Closes #5169.
Reviewer guidance
The
get_published_datais available for users with edit permissions for the given channel, it should be checked that this matches the expectation.