Skip to content
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

Merged
merged 37 commits into from
Sep 20, 2023

Conversation

mono424
Copy link
Collaborator

@mono424 mono424 commented Aug 23, 2023

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

  • Go to course management Page
  • Open a course
  • Edit a lecture
  • a) Check uploading media
  • b) Check changing meta data
  • c) Check uploading attachments (Drag and drop Description field)
  • Check JS-Console and Network Tab for errors
  • Reload and check if data is persistent

Screenshots

Way shorter UI specific code (edit-course.ts):

difference

@github-actions
Copy link

Your Testserver will be ready at https://1137.test.live.mm.rbg.tum.de in a few minutes.

Logins
Kurs1 Kurs2 Kurs3 Kurs4
public public loggedin enrolled
prof1 prof1 prof2 prof1
prof2
student1
student2
student3
student1
student2
student2
student3
student1
student2

@mono424 mono424 changed the title started wit hrefactor of course management course management data-providers refactor Aug 23, 2023
@mono424 mono424 marked this pull request as ready for review September 11, 2023 09:09
Copy link
Collaborator

@MatthiasReumann MatthiasReumann left a 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.

web/ts/api/admin-lecture-list.ts Outdated Show resolved Hide resolved
web/ts/api/admin-lecture-list.ts Outdated Show resolved Hide resolved
web/ts/api/admin-lecture-list.ts Show resolved Hide resolved
Copy link
Collaborator

@MatthiasReumann MatthiasReumann left a 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) 😅

@mono424
Copy link
Collaborator Author

mono424 commented Sep 14, 2023

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, delete and deleteSeries should be fixed now ;)

Copy link
Collaborator

@MatthiasReumann MatthiasReumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. Thanks!

@mono424
Copy link
Collaborator Author

mono424 commented Sep 15, 2023

@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?

Copy link
Member

@joschahenningsen joschahenningsen left a 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!

@joschahenningsen joschahenningsen merged commit ac6ae65 into dev Sep 20, 2023
8 checks passed
@joschahenningsen joschahenningsen deleted the improvement/refactor-course-mgmt branch September 20, 2023 15:58
SebiWrn pushed a commit that referenced this pull request May 7, 2024
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants