-
Notifications
You must be signed in to change notification settings - Fork 4
added dbml file and updated dbml-schema #1123
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
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughA comprehensive new database schema is introduced for an academic and user management system. The schema defines tables for users, permissions, academic entities (schools, departments, courses, instructors, semesters), clubs, scheduling, enrollments, grades, reviews, Q&A, and voting, with extensive foreign key relationships and many-to-many associations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Course
participant Instructor
participant Review
participant Vote
User->>Course: Enrolls / Views course
User->>Instructor: Views instructor info
User->>Review: Submits review for Course/Instructor
User->>Vote: Votes on Review
Review->>Course: Linked to course
Review->>Instructor: Optionally linked to instructor
Vote->>Review: Vote references review
sequenceDiagram
participant User
participant Schedule
participant ScheduledCourse
participant Section
participant Instructor
User->>Schedule: Creates schedule
Schedule->>ScheduledCourse: Adds course to schedule
ScheduledCourse->>Section: Links to section
ScheduledCourse->>Instructor: Optionally links to instructor
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (3)
db/dbschema.dbml (3)
21-21
: Document many-to-many junction tablesThe schema shows many-to-many relationships but doesn't explicitly define the junction tables that will be created. This makes the actual database structure unclear.
Consider explicitly defining junction tables for clarity:
table Group_Permission { group_id foreign_key permission_id foreign_key } table User_Group { user_id foreign_key group_id foreign_key } table Instructor_Department { instructor_id foreign_key department_id foreign_key }
Also applies to: 91-92, 101-101, 121-121, 172-172
1-324
: Consider documenting database indexesThe schema lacks index definitions for foreign keys and commonly queried fields. This could lead to performance issues in production.
Consider adding index documentation for:
- All foreign key fields (automatically created by most databases)
- Composite indexes for common query patterns (e.g.,
semester.year + semester.season
)- Fields used in WHERE clauses (e.g.,
course.number
,user.computing_id
)- Fields used in JOIN operations
135-168
: Reduce duplication in grade tables
CourseGrade
andCourseInstructorGrade
have identical grade distribution fields, violating the DRY principle.Consider creating a base grade distribution structure:
table GradeDistribution { average float a_plus int a int a_minus int b_plus int b int b_minus int c_plus int c int c_minus int dfw int total_enrolled int id int [primary_key] } table CourseGrade { course foreign_key grade_distribution foreign_key id int [primary_key] } table CourseInstructorGrade { instructor foreign_key course foreign_key grade_distribution foreign_key id int [primary_key] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
db/dbschema.jpg
is excluded by!**/*.jpg
📒 Files selected for processing (1)
db/dbschema.dbml
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: django
- GitHub Check: pylint
ref: LogEntry.user > User.id | ||
ref: LogEntry.content_type > ContentType.id | ||
ref: Permission.content_type > ContentType.id | ||
ref: Group.permissions > Permission.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Foreign key references should use primary keys
Foreign key references are incorrectly pointing to non-primary key fields (e.g., name
, first_name
) instead of id
fields. This violates referential integrity best practices and can cause performance and data consistency issues.
Apply these corrections to use primary keys:
-ref: Group.permissions > Permission.name
+ref: Group.permissions > Permission.id
-ref: User.groups > Group.name
+ref: User.groups > Group.id
-ref: User.user_permissions > Permission.name
+ref: User.user_permissions > Permission.id
-ref: Instructor.departments > Department.name
+ref: Instructor.departments > Department.id
-ref: Course.disciplines > Discipline.name
+ref: Course.disciplines > Discipline.id
-ref: Section.instructors > Instructor.first_name
+ref: Section.instructors > Instructor.id
Also applies to: 289-293, 299-299
🤖 Prompt for AI Agents
In db/dbschema.dbml at line 285 and also lines 289-293 and 299, foreign key
references currently point to non-primary key fields like 'name' or
'first_name'. Update these foreign key references to point to the corresponding
primary key fields, typically 'id', to ensure proper referential integrity and
data consistency.
|
||
table User { | ||
computing_id varchar [note: "primary_key : False<br>max_length : 20<br>nullable : False<br>blank : True"] | ||
graduation_year int [note: "primary_key : False<br>nullable : True<br>blank : True"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation constraints for graduation_year
The graduation_year
field lacks validation constraints. Consider adding minimum and maximum year boundaries to prevent invalid data entry.
Add constraints in your Django model:
graduation_year = models.IntegerField(
validators=[MinValueValidator(1900), MaxValueValidator(2100)],
null=True, blank=True
)
🤖 Prompt for AI Agents
In db/dbschema.dbml at line 80, the graduation_year field lacks validation
constraints to ensure valid year values. Add minimum and maximum year boundaries
by updating the Django model to include validators MinValueValidator(1900) and
MaxValueValidator(2100) on the graduation_year IntegerField, while keeping null
and blank as True.
course foreign_key | ||
club foreign_key | ||
instructor foreign_key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider separating course and club reviews
The Review table has foreign keys to both course
and club
, suggesting it serves dual purposes. This violates the single responsibility principle and could complicate queries and business logic.
Consider splitting into separate tables:
CourseReview
for course-related reviewsClubReview
for club-related reviews
This would improve maintainability and allow for type-specific fields and validations.
🤖 Prompt for AI Agents
In db/dbschema.dbml around lines 207 to 209, the Review table currently has
foreign keys to both course and club, mixing two distinct review types. To fix
this, split the Review table into two separate tables: CourseReview with a
foreign key to course, and ClubReview with a foreign key to club. This
separation will clarify responsibilities, simplify queries, and allow for
specific fields and validations per review type.
club foreign_key | ||
instructor foreign_key | ||
semester foreign_key | ||
email varchar [note: "primary_key : False<br>nullable : True<br>blank : True"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: Redundant email storage
The email
field appears redundant since reviews are already linked to users who have email addresses. Storing emails in multiple places increases PII exposure risk.
Remove the email field and retrieve it through the user relationship when needed:
-email varchar [note: "primary_key : False<br>nullable : True<br>blank : True"]
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
email varchar [note: "primary_key : False<br>nullable : True<br>blank : True"] |
🤖 Prompt for AI Agents
In db/dbschema.dbml at line 211, the email field is redundant because user
emails can be accessed via the user relationship in reviews. Remove the email
field from this table to reduce PII exposure and ensure any email data is
retrieved through the associated user entity instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool! can you turn what you did into a short script? db changes often...
Yeah yeah, ima see what the coderabbit is saying about the foreign key thing tho, it is a very old package. Ima look into it a bit more once I got the time. I don't want to generate wrong diagrams xD |
GitHub Issues addressed
What I did
How to use:
Copy paste the dbml file into https://dbdiagram.io/
How is it done:
https://github.com/hamedsj/DbmlForDjango
Screenshots
Testing
Questions/Discussions/Notes
Summary by CodeRabbit