From d089fd4daf83412f2fb48b89f584159ed3e0b35c Mon Sep 17 00:00:00 2001 From: johanneskarrer Date: Thu, 24 Oct 2024 17:34:13 +0200 Subject: [PATCH] cleared up possibly confusing behaviour for some exported methods --- api/search.go | 55 ++++++++++++++++++++++++++++++++--------------- model/semester.go | 27 ++++++++++++++--------- model/user.go | 34 +++++++++++++++++++++++++---- 3 files changed, 85 insertions(+), 31 deletions(-) diff --git a/api/search.go b/api/search.go index 4bc29a903..49010cdb1 100644 --- a/api/search.go +++ b/api/search.go @@ -138,24 +138,17 @@ func semesterSearchHelper(c *gin.Context, m tools.MeiliSearchInterface, query st if (err1 != nil || err2 != nil || len(semesters1) != 1 || len(semesters2) != 1) && err3 != nil { return nil, errors.New("wrong parameters") } - rangeSearch := false if len(semesters1) > 0 && len(semesters2) > 0 { firstSemester = semesters1[0] lastSemester = semesters2[0] - rangeSearch = true } - if !courseSearchOnly && (rangeSearch && firstSemester.Year == lastSemester.Year && firstSemester.TeachingTerm == lastSemester.TeachingTerm || len(semesters) == 1) { + isSingleSemesterSearch, singleSemester := determineSingleSemester(firstSemester, lastSemester, semesters) + if !courseSearchOnly && isSingleSemesterSearch { // single semester search - var semester model.Semester - if rangeSearch { - semester = firstSemester - } else { - semester = semesters[0] - } - res = m.Search(query, limit, 6, meiliCourseFilter(c, user, firstSemester, lastSemester, semesters), meiliStreamFilter(c, user, semester, nil), "") + res = m.Search(query, limit, 6, meiliCourseFilter(c, user, singleSemester, singleSemester, nil), meiliStreamFilter(c, user, singleSemester, nil), "") } else { - // multiple semester search + // multiple semester or course only search res = m.Search(query, limit, 4, meiliCourseFilter(c, user, firstSemester, lastSemester, semesters), "", "") } return res, nil @@ -379,12 +372,12 @@ func meiliStreamFilter(c *gin.Context, user *model.User, semester model.Semester if user == nil { permissionFilter = "(visibility = \"public\" AND private = 0)" } else { - enrolledCourses := user.CoursesForSemestersWithoutAdministeredCourses(semester, semester, nil) + enrolledCourses := user.CoursesBetweenSemestersWithoutAdministeredCourses(semester, semester) enrolledCoursesFilter := courseSliceToString(enrolledCourses) if len(user.AdministeredCourses) == 0 { permissionFilter = fmt.Sprintf("((visibility = \"loggedin\" OR visibility = \"public\" OR (visibility = \"enrolled\" AND courseID IN %s)) AND private = 0)", enrolledCoursesFilter) } else { - administeredCourses := user.AdministeredCoursesForSemesters(semester, semester, nil) + administeredCourses := user.AdministeredCoursesBetweenSemesters(semester, semester) administeredCoursesFilter := courseSliceToString(administeredCourses) permissionFilter = fmt.Sprintf("((visibility = \"loggedin\" OR visibility = \"public\" OR (visibility = \"enrolled\" AND courseID IN %s)) AND private = 0 OR courseID IN %s)", enrolledCoursesFilter, administeredCoursesFilter) } @@ -398,7 +391,9 @@ func meiliStreamFilter(c *gin.Context, user *model.User, semester model.Semester // meiliCourseFilter returns a filter conforming to MeiliSearch filter format that can be used for filtering courses // -// Validation of model.Semester format is the caller's responsibility +// # Validation of model.Semester format is the caller's responsibility +// +// ignores either semesters or firstSemester/lastSemester, depending on semesters == nil func meiliCourseFilter(c *gin.Context, user *model.User, firstSemester model.Semester, lastSemester model.Semester, semesters []model.Semester) string { semesterFilter := meiliSemesterFilter(firstSemester, lastSemester, semesters) if user != nil && user.Role == model.AdminType { @@ -409,12 +404,22 @@ func meiliCourseFilter(c *gin.Context, user *model.User, firstSemester model.Sem if user == nil { permissionFilter = "(visibility = \"public\")" } else { - enrolledCourses := user.CoursesForSemestersWithoutAdministeredCourses(firstSemester, lastSemester, semesters) + var enrolledCourses []model.Course + if semesters == nil { + enrolledCourses = user.CoursesBetweenSemestersWithoutAdministeredCourses(firstSemester, lastSemester) + } else { + enrolledCourses = user.CoursesForSemestersWithoutAdministeredCourses(semesters) + } enrolledCoursesFilter := courseSliceToString(enrolledCourses) if len(user.AdministeredCourses) == 0 { permissionFilter = fmt.Sprintf("(visibility = \"loggedin\" OR visibility = \"public\" OR (visibility = \"enrolled\" AND ID IN %s))", enrolledCoursesFilter) } else { - administeredCourses := user.AdministeredCoursesForSemesters(firstSemester, lastSemester, semesters) + var administeredCourses []model.Course + if semesters == nil { + enrolledCourses = user.CoursesBetweenSemestersWithoutAdministeredCourses(firstSemester, lastSemester) + } else { + enrolledCourses = user.CoursesForSemestersWithoutAdministeredCourses(semesters) + } administeredCoursesFilter := courseSliceToString(administeredCourses) permissionFilter = fmt.Sprintf("(visibility = \"loggedin\" OR visibility = \"public\" OR (visibility = \"enrolled\" AND ID IN %s) OR ID IN %s)", enrolledCoursesFilter, administeredCoursesFilter) } @@ -428,7 +433,9 @@ func meiliCourseFilter(c *gin.Context, user *model.User, firstSemester model.Sem // meiliSemesterFilter returns a filter conforming to MeiliSearch filter format // -// Validation of model.Semester format is the caller's responsibility +// # Validation of model.Semester format is the caller's responsibility +// +// ignores either semesters or firstSemester/lastSemester, depending on semesters == nil func meiliSemesterFilter(firstSemester model.Semester, lastSemester model.Semester, semesters []model.Semester) string { if len(semesters) == 0 && firstSemester.Year < 1900 && lastSemester.Year > 2800 { return "" @@ -489,6 +496,7 @@ func responseToMap(res *meilisearch.MultiSearchResponse) MeiliSearchMap { return msm } +// parseSemesters parses the URL Parameter semester and returns a slice containing every semester in the parameter or an error code func parseSemesters(semestersParam string) ([]model.Semester, error) { if semestersParam == "" { return nil, errors.New("empty semestersParam") @@ -568,6 +576,19 @@ func uintSliceToString(ids []uint) string { return filter } +func determineSingleSemester(firstSemester model.Semester, lastSemester model.Semester, semesters []model.Semester) (bool, model.Semester) { + if semesters == nil { + if firstSemester.IsEqual(lastSemester) { + return true, firstSemester + } + return false, model.Semester{} + } + if len(semesters) == 1 { + return true, semesters[0] + } + return false, model.Semester{} +} + // ToSearchCourseDTO converts Courses to slice of SearchCourseDTO func ToSearchCourseDTO(cs ...model.Course) []SearchCourseDTO { res := make([]SearchCourseDTO, len(cs)) diff --git a/model/semester.go b/model/semester.go index 8a27460a6..2e011e93b 100644 --- a/model/semester.go +++ b/model/semester.go @@ -5,14 +5,8 @@ type Semester struct { Year int } -// InRangeOfSemesters checks if s is between firstSemester (inclusive) and lastSemester (inclusive) or is element of semesters slice -func (s *Semester) InRangeOfSemesters(firstSemester Semester, lastSemester Semester, semesters []Semester) bool { - if semesters == nil { - if firstSemester.Year == lastSemester.Year && firstSemester.TeachingTerm == lastSemester.TeachingTerm { - return s.Year == firstSemester.Year && s.TeachingTerm == firstSemester.TeachingTerm - } - return s.GreaterEqualThan(firstSemester) && lastSemester.GreaterEqualThan(*s) - } +// IsInRangeOfSemesters checks if s is element of semesters slice +func (s *Semester) IsInRangeOfSemesters(semesters []Semester) bool { for _, semester := range semesters { if s.Year == semester.Year && s.TeachingTerm == semester.TeachingTerm { return true @@ -21,7 +15,20 @@ func (s *Semester) InRangeOfSemesters(firstSemester Semester, lastSemester Semes return false } -// GreaterEqualThan checks if s comes after or is equal to s1 -func (s *Semester) GreaterEqualThan(s1 Semester) bool { +// IsBetweenSemesters checks if s is between firstSemester (inclusive) and lastSemester (inclusive) +func (s *Semester) IsBetweenSemesters(firstSemester Semester, lastSemester Semester) bool { + if firstSemester.Year == lastSemester.Year && firstSemester.TeachingTerm == lastSemester.TeachingTerm { + return s.Year == firstSemester.Year && s.TeachingTerm == firstSemester.TeachingTerm + } + return s.IsGreaterEqualThan(firstSemester) && lastSemester.IsGreaterEqualThan(*s) +} + +// IsEqual checks if s is equal to otherSemester +func (s *Semester) IsEqual(otherSemester Semester) bool { + return s.Year == otherSemester.Year && s.TeachingTerm == otherSemester.TeachingTerm +} + +// IsGreaterEqualThan checks if s comes after or is equal to s1 +func (s *Semester) IsGreaterEqualThan(s1 Semester) bool { return s.Year > s1.Year || (s.Year == s1.Year && (s.TeachingTerm == "W" || s1.TeachingTerm == "S")) } diff --git a/model/user.go b/model/user.go index 30d1be4c0..408296b21 100755 --- a/model/user.go +++ b/model/user.go @@ -303,12 +303,25 @@ func (u *User) CoursesForSemester(year int, term string, context context.Context } // AdministeredCoursesForSemesters returns all courses, that the user is a course admin of, in the given semester range or semesters -func (u *User) AdministeredCoursesForSemesters(firstSemester Semester, lastSemester Semester, semesters []Semester) []Course { +func (u *User) AdministeredCoursesForSemesters(semesters []Semester) []Course { var semester Semester administeredCourses := make([]Course, 0) for _, c := range u.AdministeredCourses { semester = Semester{TeachingTerm: c.TeachingTerm, Year: c.Year} - if semester.InRangeOfSemesters(firstSemester, lastSemester, semesters) { + if semester.IsInRangeOfSemesters(semesters) { + administeredCourses = append(administeredCourses, c) + } + } + return administeredCourses +} + +// AdministeredCoursesBetweenSemesters returns all courses, that the user is a course admin of, between firstSemester and lasSemester +func (u *User) AdministeredCoursesBetweenSemesters(firstSemester Semester, lastSemester Semester) []Course { + var semester Semester + administeredCourses := make([]Course, 0) + for _, c := range u.AdministeredCourses { + semester = Semester{TeachingTerm: c.TeachingTerm, Year: c.Year} + if semester.IsBetweenSemesters(firstSemester, lastSemester) { administeredCourses = append(administeredCourses, c) } } @@ -316,12 +329,25 @@ func (u *User) AdministeredCoursesForSemesters(firstSemester Semester, lastSemes } // CoursesForSemestersWithoutAdministeredCourses returns all courses of the user in the given semester range or semesters excluding administered courses -func (u *User) CoursesForSemestersWithoutAdministeredCourses(firstSemester Semester, lastSemester Semester, semesters []Semester) []Course { +func (u *User) CoursesForSemestersWithoutAdministeredCourses(semesters []Semester) []Course { + var semester Semester + courses := make([]Course, 0) + for _, c := range u.Courses { + semester = Semester{TeachingTerm: c.TeachingTerm, Year: c.Year} + if semester.IsInRangeOfSemesters(semesters) && !u.IsAdminOfCourse(c) { + courses = append(courses, c) + } + } + return courses +} + +// CoursesBetweenSemestersWithoutAdministeredCourses returns all courses of the user in the given semester range or semesters excluding administered courses +func (u *User) CoursesBetweenSemestersWithoutAdministeredCourses(firstSemester Semester, lastSemester Semester) []Course { var semester Semester courses := make([]Course, 0) for _, c := range u.Courses { semester = Semester{TeachingTerm: c.TeachingTerm, Year: c.Year} - if semester.InRangeOfSemesters(firstSemester, lastSemester, semesters) && !u.IsAdminOfCourse(c) { + if semester.IsBetweenSemesters(firstSemester, lastSemester) && !u.IsAdminOfCourse(c) { courses = append(courses, c) } }