-
Notifications
You must be signed in to change notification settings - Fork 44
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
course management data-providers refactor #1137
Conversation
Your Testserver will be ready at https://1137.test.live.mm.rbg.tum.de in a few minutes. Logins
|
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.
Great. 🚀 Still amazed how you can write such clean TS code. The comments are always great too!
Tested in the docker-compose setup, everything works fine.
I've added some small remarks.
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.
Changes look good.
Somehow the "Delete Lecture" in the dropdown menu doesn't work anymore. (Console: ReferenceError: Can't find variable: deleteLecture
) 😅
thanks for the ping! Should have tested better, |
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.
Great. Thanks!
@joschahenningsen regarding the size of the pr do you want to take a look first before I merge or can I just go ahead and merge it? |
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.
Thanks a ton, this is great!
* started wit hrefactor of course management * add dao & route * update route * started wit hfetching * Add changeset abstraction * cleanup file * remove get all streams again, as there is AdminJson method * Its rendering * further dev :S * added some directive * little fixes * ... * ... no idea... * right path i guess * changeset working except video sections * fix discard files * save from laptop * Add changeset doku * make private works like charm * changing works pretty well * add video upload * add series update * ported transcoding * add readme and lecture hall select * more description * simplified lecture hall set * linter * more fixes * :) * add attachments * :) * Impl. Feedback * fix delete lecture * reenable page reload on create
Motivation and Context
Currently, the course management relies on Go generated template data. This means we need page reloads
and also it is not good practice to have backend and frontend entangled in that way. Further, much very specific code is used to handle the course management tasks, that could be abstracted in cleaner ways.
Description
This PR untangles backend and frontend by using data-providers that are introduced earlier and are already used for data-sections and other parts of the app. This gives a standardized abstraction to handle the lecture's client side.
Also, this will prevent useless page reloads and give a better standardized and app-global API to create new lectures, fetch or update them.
Steps for Testing
Screenshots
Way shorter UI specific code (edit-course.ts):