-
Notifications
You must be signed in to change notification settings - Fork 26
Add Playlist management #1588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
owi92
wants to merge
27
commits into
elan-ev:main
Choose a base branch
from
owi92:playlists
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add Playlist management #1588
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
LukasKalbertodt
requested changes
Nov 20, 2025
Member
LukasKalbertodt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backend review: I did not yet test it or look at any frontend code.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
Works the same as creating events or series in the sense that we send the request to Opencast and insert the playlist in our DB without waiting for sync. Authorization will be added in a later commit.
Unlike series/video metadata and acl updating, this uses a single endpoint for both (which is how it is done in Opencast). That means that besides the playlist id, each parameter is optional and will only be updated when it is `Some`. Calling this API will update the Playlist in Tobira's Database without waiting for sync.
This utilizes the generic `delete_opencast_thing` endpoint that is also used for series and events. The corresponding playlist endpoint in Opencast however works a little different: There are no workflows or other magic background operations involved. When we get a 200 response, it simply means that the playlist has been deleted in Opencast. This in turn lets us avoid having to create yet another table/view that tracks "deleted" playlists (which we have for series and events). We can simply go ahead and delete the playlist directly in our DB, no need for a `pending` limbo state and no fear of sync concflicts.
Something something consistent naming.
**Logic of mutations remains untouched** Since `model/playlist` is a folder rather than a single file and it already has a `mod.rs` file, I think it's nicer to also have the mutations in a separate file and not cram everything in `mod.rs`. I don't know about best practices for this, and `series` and `events` do do everything in one file. I personally prefer separation of concerns (at least in this case).
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Member
Author
|
You can ignore the above warning. Mounting series still works with that change. |
LukasKalbertodt
requested changes
Dec 1, 2025
This is the table overview analogous to "my videos" and "my series". It is missing some stuff like thumbnails for now, that will be added in a later commit.
These are basically just the same as series thumbnail stacks with a different query, reusing most of the other relevant code.
I didn't do this before because I wasn't sure what to do about the playlist's state/status. But as far as I can tell, playlists do not need that, since their API works a little different. Creating, updating and deleting will either succeed or fail, and we get that result as an immediate response from Opencast. So when it fails, we just don't update it. This means that it should always be in sync.
This is mainly in preparation for the next few commits that will add the remaining management routes. This commit adds the shared route building function, the necessary graphQL query and the navigation component for single playlists.
For now this has only the share menu and the delete button and function. I'd rather add the mutations and interfaces one by one to keep these commits in a reasonable size.
This had to adjust the way metadata is passed to the API. Instead of having both title and description optional, it now uses the `BasicMetadata` struct again, which necessary for the frontend component and where the title is required. That component is used for both creating and editing metadata, and while the title is technically optional when editing, it is mandatory for creation.
This adds the interface to add and remove videos to/from playlists. The backend and API needed a slight adjustment, because OC needs the `opencast_id` of the entries. We could either pass them from frontend (which would need some query adjustments) or load them in backend (which is what I do here now).
Previously they just weren't shown, and worse: Any entries that are unknown to Tobira were just dropped and lost upon using the `update` endpoint. This commit adds (1): a note to playlist editing with the count of unknown videos, and (2): some code in backend that adds these entries back to the mutation. Caveat: They will get new ids (the `entry_id`, which is given by opencast, NOT the `opencast_id`, that remains the same of course). But I don't think that is an issue.
Also removed some french translations related to series and video ACL editing. The translator also supplied version that are count agnostic, which I believe we should prefer whenever possible.
Again, mostly copied from already existing series stuff, so this could probably be generalized. On the other hand, I will be adding the option to choose initial entries as a feature for playlist creation in the next commit, so a generalization might not be worth the added complexity.
This comment has been minimized.
This comment has been minimized.
It is now possible to set initial entries when creating a playlist. I reused some code from `EditVideoList` and factor out some code to prevent duplication. This has a small side effect: When adding or removing videos with that component (before submitting the form), the event count above the interface will now update at the same time and reflect the number of events that is currently displayed. Previously, this was using the count from the DB entry and was only updated when the user submitted the form. Another thing: I wasn't sure whether to integrate the events thing to react hook form. State management is easier when it's not integrated, and it is not necessary to do any form validation on the entries field (since that can also be left empty). On the other hand, integration would be nice at the very least for consistency.
Users shouldn't need write access for events they want to remove from their playlist.
So it's not just typed as String everywhere. Still behaves like a String though. Pretty sure this commit doesn't catch all occurences, since it's not always called `opencast_id` or `oc_id`. Sometimes it's just `id` or something else.
This should prevent unnecessary errors in case of missing/not loaded thumbnails. Instead we just show placeholders.
Unfortunately, GraphQL doesn't know that an OpencastId is just a String in disguise, and we can't tell it either. So to keep things backwards compatible with Opencast (which also uses these `by_opencast_id` queries), this has to stick with the explicit `String` type. Note on playlists: Opencast and the Admin UI don't have any Playlist features yet that could talk with the respective endpoint, but I expect this will come in the future. While we could in theory say "use our special type and deal with it", I think it's nicer if it can mirror the existing series and event endpoints.
|
🚨🚨🚨 This PR changes APIs used by the Opencast Admin UI integration 🚨🚨🚨 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We already harvest playlist data from Opencast and it has been possible to add playlist blocks pages in Tobira for a while.
This adds the necessary API and the following pages for playlist management:
Like series and events, any C(R)UD operation will call the respective external Opencast API. When the response is OK, the operation is also performed in Tobira's DB, so users do not have to wait for the next sync in order to see results.
Right now
allmost dummy users are allowed to create playlists, but this is generally tied to a dedicated role, which can be configured.Closes #1312
Can be reviewed commit by commit