840 short decription for each video

[karjo24]

Motivation and Context

#840. Having the ability to make short notes or add/change the title of the lecture is useful for organising lectures when titles are not provided by the course admins.

Description

There's an additional option in the dropdown menu for each stream on the course overview page that allows users to input their own (limited to 80 characters) lecture title. There's a new API endpoint, that takes the new title and stores it in a new table in the database.

Steps for Testing

Prerequisites:

• 1 User
• 1 Course with 1 stream with title, 1 stream without

1. Log in and navigate to course
2. Click on the "More options" menu (three dots next to stream)
3. Click on "Edit personal lecture title"
4. Enter new title in popup menu
5. Verify the following behaviour
   5.1 Pressing "Enter" or clicking outside the input field changes the title
   5.2 Pressing "Escape" disregards any changes
   5.3 Deleting the value of the input field resets the title to the actual title provided by the lecturer
6. Reload to make sure the changes are persistent

Screenshots

Screencast.from.2025-03-08.15-19-45.mp4

[carlobortolan]

Tested it locally and it looks good to me 👍

Just two more general questions:

• Should the personal lecture title be displayed only in the course-view, or also in the "Recent VODs" and videoplayer/stream-view?
• Should lecturers/admins of a course be allowed to add personal lecture titles, or would it be better to have the button set the actual lecture title to avoid confusion?

[carlobortolan]

Maybe consider handling both cases (with and without user) in the GetCourseBySlugYearAndTerm method to avoid duplicate code in GetCourseBySlugYearAndTerm and GetCourseBySlugYearTermUser (see comment below)

[karjo24]

Fixed nearly everything, thanks for the review :)

[carlobortolan]

Would it be possible to simplify both GetCourseBySlug... methods into one or would it break something?

E.g., something like:

func (d coursesDao) GetCourseBySlugYearAndTerm(slug string, term string, year int, userID uint) (model.Course, error) {
    var cacheKey string
    if userID == 0 {
        cacheKey = fmt.Sprintf("courseBySlugYearAndTerm%v%v%v", slug, term, year)
    } else {
        cacheKey = fmt.Sprintf("courseBySlugYearTermUser%v%v%v%v", slug, term, year, userID)
    }

    cachedCourses, found := Cache.Get(cacheKey)
    if found {
        return cachedCourses.(model.Course), nil
    }

    var course model.Course
    query := DB.Preload("Streams.VideoSections").Preload("Streams.Units", func(db *gorm.DB) *gorm.DB {
        return db.Order("unit_start desc")
    }).Preload("Streams", func(db *gorm.DB) *gorm.DB {
        return db.Order("start desc")
    }).Preload("Admins").Where("teaching_term = ? AND slug = ? AND year = ?", term, slug, year)

    if userID != 0 {
        query = query.Preload("Streams.CustomLectureTitles", "user_id = ?", userID)
    }

    err := query.First(&course).Error
    if err == nil {
        Cache.SetWithTTL(cacheKey, course, 1, time.Minute)
    }
    return course, err
}

[karjo24]

Yes, that probably makes sense. As this only adds information to the return struct, I doubt that this breaks something (I did have a look at all usages and did a quick test locally). Technically we'd not need the check for userID != 0, because the where clause would not match any entry for == 0, but I'd like to keep it for clarity.