-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add series management pages #1313
Draft
owi92
wants to merge
11
commits into
elan-ev:next
Choose a base branch
from
owi92:series-details
base: next
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.
Draft
Conversation
This file contains 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 has been minimized.
This comment has been minimized.
This pull request has conflicts ☹ |
github-actions
bot
added
the
status:conflicts
This PR has conflicts that need to be resolved
label
Jan 17, 2025
This generalizes and factors out some backend code so it can also be used for series and playlists. It also replaces the cursor based pagination with an offset based one. That simplifies a lot of things and allows us to get rid of a bunch of code. The frontend now uses a `page` url parameter to signify the current page. When entered manually, there are some checks and logic to make sure it's always in bounds, i.e. using a number smaller than 0 will always redirect to `page=1` and a number larger that the max number of pages will redirect to the last page. Please note that the generalization of the sorting columns will be done in a later commit.
This factors out most of the table code for re-use with other assets, namely series and playlists. Please note that the generalization of column sorting and display will be done in later commits.
This includes: (a) Backend code that allows generating custom enums for different sorting columns, trying to minimize the necessity of code duplication (b) Frontend code mostly related to parsing the custom sorting columns from URL parameters and passing these to the API Generalization of the table, to represent these custom columns will be done in the next commit.
This will now take any custom columns into consideration and makes their declaration easier while trying to limit duplicated code as much as possible without adding too much complexity. Tables now have a __somewhat__ clear and fixed stucture. Any customization, including additional columns, can be done in the respective `index` files of the asset. This is assuming that all asset tables have thumbnails, titles and descriptions that share at least some portion of their styling. But they can also be further customized. All of this is also in preparation for adding a playlist table later on. Adding that should be pretty straightforward now.
Todo: delete the other commit.
This combines two things: (a) It fixes the event query for the `My videos` table. Adjumstments became necessary after some upstream changes to the series column in the `from_db` impl of authorized events. (b) It moves some table specific query customizations from the generic `load_writable_for_user` function to the impls of the `LoadableAsset` trait.
This generalizes the `Nav` and `Details` code that was used for videos, and repurposes it for series as well. With these changes, it should also be fairly easy to add this for playlists later on.
This required a bunch of changes: - The `update_acl` endpoint which talks to Opencast was generalized to work for both events and series, as their acl `put` endpoints pretty much work the same. - The access page of events was refactored and most code is now usable for both events and series. This tries to walk the thin line between modularity and overspecialization by attempting to balance out reusablility and complexity, limiting both duplicated code and prop drilling.
github-actions
bot
removed
the
status:conflicts
This PR has conflicts that need to be resolved
label
Jan 21, 2025
@owi92 I like seeing the progress here! 😍 |
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.
Step 2 in adding proper series management.
Adds series details and acl pages, making sure all new and/or refactored components can also be used for playlists later on.
Based on #1311
Step 3 will then be more API related stuff, adding creation, editing and deleting of series.