Test course feature for lecturers #1425
Conversation
|
Your Testserver will be ready at https://1425.test.live.mm.rbg.tum.de in a few minutes. Logins
|
| func (r coursesRoutes) createTestCourse(c *gin.Context) { | ||
| tumLiveContext := c.MustGet("TUMLiveContext").(tools.TUMLiveContext) | ||
|
|
||
| _, err := r.StreamsDao.CreateOrGetTestCourse(tumLiveContext.User) |
There was a problem hiding this comment.
Should every user be able to create a test course? If every student can create a test course, they could possibly fill up the entire disk just by streaming nonsense
dao/streams.go
Outdated
| @@ -405,8 +406,8 @@ func (d streamsDao) CreateOrGetTestStreamAndCourse(user *model.User) (model.Stre | |||
| func (d streamsDao) CreateOrGetTestCourse(user *model.User) (model.Course, error) { | |||
| var course model.Course | |||
| err := DB.FirstOrCreate(&course, model.Course{ | |||
| Name: "(" + strconv.Itoa(int(user.ID)) + ") " + user.Name + "'s Test Course", | |||
| TeachingTerm: "Test", | |||
| Name: user.GetPreferredName() + "'s Test Course", | |||
There was a problem hiding this comment.
Maybe we should have a fallback if the users preferred name is empty
There was a problem hiding this comment.
Thanks for pointing that out; I updated it so that it defaults to Test Couse if a user has a blank name.
Edit: On a side note, is it an issue for the workers/edge server if all test courses have the same slug? (/should the slug be unique for all courses?) From what I was able to test locally, this is not an issue in the web client.
web/admin.go
Outdated
| @@ -221,7 +231,7 @@ func (r mainRoutes) CourseStatsPage(c *gin.Context) { | |||
| logger.Error("couldn't query courses for user.", "err", err) | |||
| courses = []model.Course{} | |||
| } | |||
| semesters := r.CoursesDao.GetAvailableSemesters(c) | |||
| semesters := r.CoursesDao.GetAvailableSemesters(c, false) | |||
There was a problem hiding this comment.
Should we allow to see stats of the test courses? Maybe with that users could also test this stats functionality?
There was a problem hiding this comment.
I don't think there should be any problem if we allow lecturers to see stats for test courses; updated it in my latest commit 👍
web/admin.go
Outdated
| y, t := tum.GetCurrentSemester() | ||
|
|
There was a problem hiding this comment.
Duplicate with line 284, this should be another function so the test course check can be adjusted if we change the implementation of test courses
Motivation and Context
Closes #1418.
While this approach is not optimal, as we need to "reserve" special values for test courses, it is relatively simple and builds on top of the existing code in comparison to more complex approaches (e.g., updating the current course model & table to include a
isTestCourseattribute or adding a completely new test course model).Description
1234).includeTestSemesterflag, which defaults to false. This allows the endpoint & method to return the same information as before (all normal courses), and only if the flag is set, also include the test courses.Steps for Testing
Prerequisites:
+ Create Test Courseto create a new test courseScreenshots
How to create a test course:
How an Admin sees all test courses:
