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

Enable searching courses by teacher name #142

Merged
merged 10 commits into from
Dec 19, 2024

Conversation

HydrogenC
Copy link
Contributor

@HydrogenC HydrogenC commented Aug 8, 2024

Enable searching by teacher name on server side, resolves DanXi-Dev/DanXi#375.
In order to make the new code work, there are something changes to do on the database.

  • Run the following SQL code to create the teacher table (for the sake of query performance)
CREATE TABLE `teacher`  (
  `id` bigint NOT NULL AUTO_INCREMENT,
  `name` varchar(255),
  PRIMARY KEY (`id`)
)
CREATE TABLE `teacher_course_groups`  (
  `teacher_id` bigint NOT NULL,
  `course_group_id` bigint NOT NULL,
  PRIMARY KEY (`teacher_id`, `course_group_id`),
  FOREIGN KEY (`course_group_id`) REFERENCES `course_group` (`id`),
  FOREIGN KEY (`teacher_id`) REFERENCES `teacher` (`id`)
)

@HydrogenC
Copy link
Contributor Author

Also included a fix for #134.

danke_utils/teacher_table.go Outdated Show resolved Hide resolved
danke_utils/teacher_table.go Outdated Show resolved Hide resolved
@HydrogenC
Copy link
Contributor Author

Resolved.

@HydrogenC
Copy link
Contributor Author

Do I have to separate a new API for teacher name searching and use db.Table("teacher").Preload("CourseGroups").Find(...) to find related course groups?

@JingYiJun
Copy link
Member

Do I have to separate a new API for teacher name searching and use db.Table("teacher").Preload("CourseGroups").Find(...) to find related course groups?

或许这样是更好的,不过需要前端适配两种不同的搜索模式。现在这样的策略对于用户来说,搜索 Teacher 但是显示 CourseGroup 感觉比较奇怪😂

@HydrogenC
Copy link
Contributor Author

Do I have to separate a new API for teacher name searching and use db.Table("teacher").Preload("CourseGroups").Find(...) to find related course groups?

或许这样是更好的,不过需要前端适配两种不同的搜索模式。现在这样的策略对于用户来说,搜索 Teacher 但是显示 CourseGroup 感觉比较奇怪😂

Is there an advantage for using Preload over Join? Should I modify my current implementation then?

@JingYiJun
Copy link
Member

Do I have to separate a new API for teacher name searching and use db.Table("teacher").Preload("CourseGroups").Find(...) to find related course groups?

或许这样是更好的,不过需要前端适配两种不同的搜索模式。现在这样的策略对于用户来说,搜索 Teacher 但是显示 CourseGroup 感觉比较奇怪😂

Is there an advantage for using Preload over Join? Should I modify my current implementation then?

Maybe in this situation Join is better since Join means only one load from database while Preload implies 3 loads, and there is no need to get the intermediate information of teachers. If there you add a new api to load the teachers user search, Preload may better, but in this pr you've done enough on changing SearchCourseGroups.

@JingYiJun JingYiJun merged commit 64555dd into OpenTreeHole:main Dec 19, 2024
HydrogenC added a commit to DanXi-Dev/DanXi that referenced this pull request Dec 19, 2024
Regarding to changes merged in <OpenTreeHole/backend#142>
PS: I believe this is too minor for a PR
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.

[Feature Request] 课评支持按照课程代码或老师名搜索
2 participants