feat(portal): create initial portal database schemas #134
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request involve the introduction of new database schemas and modifications to existing schemas using the Drizzle ORM for PostgreSQL. New files define schemas for Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Outside diff range and nitpick comments (17)
libs/db/src/schema/session.ts (1)
Line range hint
1-14: Overall schema changes align well with PR objectives.The renaming of fields from snake_case to camelCase improves consistency with JavaScript naming conventions, which is beneficial for a TypeScript project. These changes, while minor, contribute to the creation of initial portal database schemas as outlined in the PR objectives.
The session table structure appears well-suited for managing user sessions, which is crucial for the portal user profiles. The relationship between the session and user tables is maintained, ensuring data integrity.
As you continue to develop the schema, consider the following:
- Ensure that all related tables follow the same naming convention for consistency.
- If you plan to merge the UserPreferences table with the User table as mentioned in the PR description, make sure to update any references to UserPreferences in other parts of the codebase.
- As you're using Drizzle ORM, take advantage of its migration capabilities when making future schema changes to ensure smooth updates and backwards compatibility.
libs/db/src/schema/resume.ts (1)
1-8: Overall, good initial schema implementation with room for improvement.This schema provides a solid foundation for storing resume information using Drizzle ORM, aligning well with the PR objectives. It offers type safety and ease of querying as mentioned in the PR description. However, consider the suggested enhancements to improve data integrity, query performance, and user association.
As this is described as a temporary, rough draft in the PR objectives, these improvements can be incorporated in future iterations. Remember to update the database design screenshots when making these changes.
libs/db/src/schema/school.ts (1)
5-9: LGTM: Table structure is appropriate. Consider additional fields.The 'school' table structure is well-defined with a primary key, name, and level of study. However, consider adding more fields to enrich the school information:
location: To store the school's address or city.establishedDate: To record when the school was founded.website: To store the school's official website URL.These additional fields could provide more comprehensive information about each school.
Here's an example of how you might expand the table definition:
export const school = pgTable('school', { id: serial('id').primaryKey(), name: varchar('name', { length: 128 }), levelOfStudy: levelOfStudyEnum('levelOfStudy'), location: varchar('location', { length: 255 }), establishedDate: date('established_date'), website: varchar('website', { length: 255 }) })libs/db/src/schema/socialMedia.ts (2)
3-3: Consider using PascalCase for imported table names.The import of the 'user' table is correct and follows good practices by separating table schemas into different files. However, to align with common TypeScript conventions, consider using PascalCase for imported table names.
You could update the import statement as follows:
-import { user } from './user' +import { user as User } from './user'Then use
Userinstead ofuserin the foreign key reference.
5-11: Approve schema with suggestions for improvement.The overall structure of the
socialMediatable schema looks good. Here are some suggestions for improvement:
- Consider adding NOT NULL constraints to required fields.
- It might be beneficial to add a unique constraint on the combination of
userIdandplatformNameto prevent duplicate entries for the same user and platform.- Verify if the varchar length limits are sufficient for your use case.
Here's a suggested improvement to the schema:
export const socialMedia = pgTable('socialMedia', { id: serial('id').primaryKey(), userId: integer('userId').notNull() .references(() => user.id), platformName: varchar('platformName', { length: 16 }).notNull(), profileUrl: varchar('profileUrl', { length: 128 }).notNull(), }, (table) => ({ userPlatformUnique: unique().on(table.userId, table.platformName), }));This adds NOT NULL constraints and a unique constraint on userId and platformName.
libs/db/src/schema/program.ts (2)
4-4: LGTM: Enum definition is clear and covers common program types.The 'programTypeEnum' is well-defined and exported for use throughout the application. It covers the most common program types: bachelor, master, diploma, and certificate.
Consider if there are any other program types that might need to be included in the future, such as 'associate' or 'doctorate'. If not immediately necessary, this can be addressed in future iterations.
6-11: LGTM: Table structure is well-defined and establishes proper relationships.The 'program' table is well-structured with appropriate field types:
- 'id' as a serial primary key
- 'name' as a varchar with a reasonable length limit
- 'schoolId' establishing a foreign key relationship with the 'school' table
- 'programType' utilizing the defined enum
Consider the following suggestions for potential improvements:
- Add a unique constraint to the 'name' field if program names are expected to be unique.
- Consider adding a 'description' field for more detailed program information.
- If applicable, add a 'createdAt' and 'updatedAt' timestamp fields for tracking record changes.
Example implementation:
export const program = pgTable('program', { id: serial('id').primaryKey(), name: varchar('name', { length: 128 }).unique(), description: varchar('description', { length: 500 }), schoolId: integer('schoolId').references(() => school.id), programType: programTypeEnum('programType'), createdAt: timestamp('created_at').defaultNow(), updatedAt: timestamp('updated_at').defaultNow(), })These suggestions can be implemented based on the specific requirements of your application.
libs/db/src/schema/emergencyContact.ts (3)
3-3: LGTM: Comprehensive relationship enum, consider additional options.The
relationshipEnumcovers common relationship types and includes an 'other' option for flexibility.Consider if there are any other common relationships that should be explicitly included, such as 'guardian' or 'spouse'. This could improve data consistency and make it easier to filter or group by relationship type in the future.
5-11: LGTM: Well-structured schema, consider some enhancements.The
emergencyContacttable schema is well-designed for storing essential emergency contact information. Here are some suggestions for potential improvements:
The
namefield length (32 characters) might be too short for some names, especially those with middle names or from cultures with longer name formats. Consider increasing this to 64 or 128 characters.For the
phoneNumberfield, ensure that the length of 16 characters is sufficient for international number formats, including the '+' sign and country code.Consider adding an email address field, as it can be a useful alternative contact method in emergencies.
There's currently no field linking this contact to a user. You might want to add a foreign key reference to the user table, unless this relationship is managed elsewhere.
Here's a suggested enhancement to the schema:
export const emergencyContact = pgTable('emergencyContact', { id: serial('id').primaryKey(), userId: integer('userId').notNull().references(() => user.id), // Add this line name: varchar('name', { length: 64 }), // Increased length relationship: relationshipEnum('relationship'), phoneNumber: varchar('phoneNumber', { length: 20 }).unique(), // Increased length isPhoneNumberVerified: boolean('isPhoneNumberVerified').default(false), email: varchar('email', { length: 255 }), // Add this line })Note: Ensure to import the
integertype and reference theusertable correctly if you implement these changes.
1-11: Overall assessment: Good initial implementation with room for enhancements.This file successfully implements a basic schema for emergency contacts using Drizzle ORM, aligning well with the PR objectives. The code is well-structured and achieves its primary goal of setting up the initial schema for Portal User Profiles.
The suggestions provided earlier aim to enhance the robustness and flexibility of the schema, particularly in terms of accommodating various name formats, international phone numbers, and additional useful information like email addresses.
As mentioned in the PR description, this is a temporary, rough draft subject to change. The current implementation provides a solid foundation that can be easily modified or expanded using Drizzle ORM's migration capabilities.
As you continue to develop this schema, consider the following architectural points:
Data normalization: Evaluate whether the emergency contact information should be in a separate table or part of the user table, based on your application's requirements and expected query patterns.
Indexing: As the database grows, you might need to add indexes to frequently queried fields for better performance.
Validation: Implement server-side validation for fields like phone numbers and email addresses to ensure data integrity.
Privacy and security: Ensure that access to this sensitive information is properly restricted and consider encryption for highly sensitive data.
libs/db/src/index.ts (2)
13-21: LGTM! Consider grouping related schemas.The new schema exports align well with the PR objectives of creating initial portal database schemas. The alphabetical organization improves readability. Great job on implementing a comprehensive set of schemas for the Portal User Profiles!
As a minor suggestion for improved organization, consider grouping related schemas together. For example, you could group user-related schemas (user, userPreferences) and education-related schemas (school, program) next to each other.
Here's a suggested regrouping:
export * from './schema/user' export * from './schema/userPreferences' export * from './schema/emergencyContact' export * from './schema/socialMedia' export * from './schema/resume' export * from './schema/school' export * from './schema/program' export * from './schema/team' export * from './schema/session'This grouping might make it easier to understand the relationships between different schemas at a glance.
13-21: Consider adding documentation and planning for future changes.The new schema exports provide a solid foundation for the Portal User Profiles, aligning well with the PR objectives. To further improve the implementation:
Consider adding comments or documentation for each schema export, briefly describing its purpose and any important relationships with other schemas. This will be helpful for other developers and for future maintenance.
Given the plan to merge the UserPreferences table with the User table in the future, it might be beneficial to add a TODO comment near these exports to remind about this future change.
Since this is described as a temporary, rough draft of the schema, consider adding a comment at the top of the file indicating its draft status and that it's subject to change.
Here's an example of how you might implement these suggestions:
// TODO: This is a draft schema and is subject to change // User-related schemas export * from './schema/user' export * from './schema/userPreferences' // TODO: Plan to merge with User schema export * from './schema/emergencyContact' export * from './schema/socialMedia' // Education-related schemas export * from './schema/school' export * from './schema/program' // Other schemas export * from './schema/resume' export * from './schema/team' export * from './schema/session'These additions will help maintain clarity as the schema evolves and make it easier for the team to track planned changes.
libs/db/src/schema/userPreferences.ts (2)
14-14: Align table naming conventions to snake_case.The table name
userPreferencesis in camelCase. For consistency and adherence to SQL best practices, consider renaming it touser_preferences.
21-21: Consider setting a default value or makingshirtSizeoptional.To handle cases where users may not wish to provide their shirt size, consider providing a default value or allowing nulls.
libs/db/src/schema/user.ts (3)
24-24: Validate and format thephoneNumberfieldWhile the
phoneNumberfield is defined with a length limit and uniqueness, adding validation to ensure numbers are correctly formatted can prevent errors and improve data quality.Consider integrating a phone number validation library or implementing a regex pattern to validate the format.
19-20: Implement content guidelines forprofileDescriptionAllowing free-form text in
profileDescriptioncan lead to the entry of sensitive or inappropriate information.Consider:
- Adding content moderation.
- Providing users with guidance on acceptable content.
- Implementing length limits to prevent excessive input.
28-28: Clarify the purpose ofisDomesticThe
isDomesticboolean may need additional context or documentation to ensure consistent usage across the application.Provide a comment or documentation explaining what qualifies as "domestic" in your context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
- libs/db/src/index.ts (1 hunks)
- libs/db/src/schema/emergencyContact.ts (1 hunks)
- libs/db/src/schema/index.ts (1 hunks)
- libs/db/src/schema/program.ts (1 hunks)
- libs/db/src/schema/resume.ts (1 hunks)
- libs/db/src/schema/school.ts (1 hunks)
- libs/db/src/schema/session.ts (1 hunks)
- libs/db/src/schema/socialMedia.ts (1 hunks)
- libs/db/src/schema/team.ts (1 hunks)
- libs/db/src/schema/user.ts (1 hunks)
- libs/db/src/schema/userPreferences.ts (1 hunks)
🔇 Additional comments (18)
libs/db/src/schema/index.ts (1)
1-9: LGTM! Consider organizing exports alphabetically.The new exports align well with the PR objective of creating initial portal database schemas for user profiles. The inclusion of various aspects like emergency contacts, programs, resumes, etc., provides a comprehensive structure for user data.
To improve readability and maintainability, consider organizing the exports alphabetically. This would make it easier to locate specific modules in the future, especially as the number of exports grows.
Here's a suggested reorganization:
export * from './emergencyContact' export * from './program' export * from './resume' export * from './school' export * from './session' export * from './socialMedia' export * from './team' export * from './user' export * from './userPreferences'Let's verify the existence of the newly exported modules:
This script will help ensure that all the exported schema modules have corresponding files in the expected location.
✅ Verification successful
All exported schema modules verified successfully.
All the exported schema modules have been confirmed to exist as expected.
To improve readability and maintainability, consider organizing the exports alphabetically.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the newly exported schema modules # Test: Check if all the exported schema files exist for schema in emergencyContact program resume school session socialMedia team user userPreferences; do if [ ! -f "libs/db/src/schema/${schema}.ts" ]; then echo "Error: ${schema}.ts does not exist" exit 1 fi done echo "All schema files exist"Length of output: 1399
libs/db/src/schema/session.ts (2)
Line range hint
10-14: Approve the renaming ofexpires_attoexpiresAtand request clarification on timestamp mode.The change to camelCase is consistent with JavaScript conventions and the previous field change. The use of a timestamp with timezone is appropriate for session expiration.
Please run the following script to check for any remaining references to
expires_at:#!/bin/bash # Search for any remaining references to 'expires_at' in TypeScript files rg --type typescript 'expires_at' -g '!libs/db/src/schema/session.ts'Could you please clarify the purpose of setting
mode: 'date'for the timestamp? It's not immediately clear what this setting does in the context of Drizzle ORM and how it affects the stored expiration time.
7-9: Approve the renaming ofuser_idtouserId.The change from snake_case to camelCase improves consistency with JavaScript naming conventions while maintaining the field's properties and relationships.
To ensure this change doesn't break existing code, please run the following script to check for any remaining references to
user_id:✅ Verification successful
Verified the renaming of
user_idtouserId.The remaining references to
'user_id'indrizzle/schema.tsare part of the schema definitions and correctly map the ORM fielduserIdto the database columnuser_id. This change maintains consistency in the codebase without introducing issues.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to 'user_id' in TypeScript files rg --type typescript 'user_id' -g '!libs/db/src/schema/session.ts'Length of output: 104
Script:
#!/bin/bash # Search for any remaining references to 'user_id' in TypeScript files rg --type ts 'user_id' -g '!libs/db/src/schema/session.ts'Length of output: 173
libs/db/src/schema/resume.ts (1)
1-1: LGTM: Imports are appropriate and fully utilized.The imports from 'drizzle-orm/pg-core' are correct and all imported types (boolean, pgTable, serial, timestamp, varchar) are used in the schema definition.
libs/db/src/schema/school.ts (2)
1-1: LGTM: Imports are correct and necessary.The imports from 'drizzle-orm/pg-core' are appropriate for defining PostgreSQL tables and enums using Drizzle ORM.
1-9: Overall implementation aligns well with PR objectives.This schema definition for the 'school' table is a solid starting point for the portal database. It successfully uses Drizzle ORM, aligning with the PR objectives of leveraging an ORM for type safety and efficient migrations. The basic structure is in place, allowing for easy expansion as requirements evolve.
Consider the suggestions for expanding the level of study enum and adding more fields to the school table to make the schema more comprehensive. These changes can be implemented now or in future iterations, depending on your immediate needs and timeline.
Remember that this is part of the initial schema setup, and as mentioned in the PR description, further updates and refinements are anticipated. The current implementation provides a good foundation for these future changes.
libs/db/src/schema/socialMedia.ts (1)
1-1: LGTM: Appropriate imports for Drizzle ORM schema definition.The imports from 'drizzle-orm/pg-core' are correct and all imported items are utilized in the schema definition.
libs/db/src/schema/program.ts (2)
1-2: LGTM: Imports are appropriate for the schema definition.The imports from 'drizzle-orm/pg-core' provide the necessary tools for defining the PostgreSQL table schema. The import of 'school' indicates a relationship between the 'program' and 'school' tables, which aligns with the expected database structure.
1-11: Overall: Well-structured schema implementation for the 'program' table.This file successfully implements the initial schema for the 'program' table using Drizzle ORM, aligning with the PR objectives. The structure is clear, establishes necessary relationships, and provides a solid foundation for the portal database.
Key points:
- Appropriate use of Drizzle ORM for type-safe schema definition.
- Clear enum definition for program types.
- Well-structured table with primary key, foreign key relationship, and use of custom enum.
The implementation serves as a good starting point for the portal database schemas. As mentioned in the PR description, this is a temporary draft and subject to change, which allows for future refinements based on evolving requirements.
To ensure consistency with other parts of the schema, let's verify the 'school' table structure:
This will help confirm that the 'school' table exists and has a compatible structure for the foreign key relationship.
libs/db/src/schema/team.ts (3)
1-1: LGTM: Imports are appropriate and concise.The imports from 'drizzle-orm/pg-core' are correctly selected for defining a PostgreSQL table schema. All imported types are utilized in the schema definition, maintaining a clean and efficient import statement.
1-13: Overall, good initial implementation of the team schema.This file successfully introduces the 'team' table schema using Drizzle ORM, aligning with the PR objectives. The implementation provides a solid foundation for the portal database, allowing for easy future refinements as mentioned in the PR description.
Key points:
- The schema covers essential team attributes.
- Use of Drizzle ORM facilitates type safety and efficient migrations.
- The structure allows for future updates, such as merging with a potential UserPreferences table.
Consider the suggested improvements in the previous comment to enhance data integrity and reduce redundancy. As this is described as a temporary, rough draft, these changes can be incorporated in future iterations of the schema.
3-13: 🛠️ Refactor suggestionConsider additional constraints and potential redundancy in the schema.
The schema is well-structured, but consider the following improvements:
- Add a 'not null' constraint to the 'name' field, as every team should have a name.
- There seems to be redundancy between 'profileImageUrl' and 'teamProfileImageUrl'. Consider consolidating these into a single field.
- If there's a corresponding 'user' table, add a foreign key constraint to 'teamOwnerId'.
- Consider adding unique constraints where appropriate (e.g., 'name' field).
Here's a suggested revision:
export const team = pgTable('team', { id: serial('id').primaryKey(), name: varchar('name', { length: 64 }).notNull().unique(), profileImageUrl: varchar('profileImageUrl', { length: 128 }), projectLinkUrl: varchar('projectLinkUrl', { length: 256 }), teamOwnerId: integer('teamOwnerId').references(() => user.id), // Assuming a 'user' table exists hasSubmitted: boolean('hasSubmitted').default(false), });This revision addresses the mentioned points and aligns with the PR objectives of establishing an initial, changeable schema.
To verify the existence of a 'user' table and its structure, run:
✅ Verification successful
Verification Successful: Foreign key constraint is valid.
The 'user' table exists with an 'id' field defined as a primary key, confirming that adding a foreign key constraint to 'teamOwnerId' is appropriate.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for 'user' table definition # Expected result: A 'user' table definition with an 'id' field rg --type typescript 'export const user = pgTable\(' libs/db/src/schemaLength of output: 108
Script:
#!/bin/bash # Description: Check for 'user' table definition using the correct type identifier # Expected result: A 'user' table definition with an 'id' field rg --type ts 'export const user = pgTable\(' libs/db/src/schemaLength of output: 130
Script:
#!/bin/bash # Description: Verify the 'id' field in the 'user' table definition # Expected result: The 'id' field is defined as a primary key rg "id\s*:\s*serial\('id'\)\.primaryKey\(\)" libs/db/src/schema/user.tsLength of output: 112
libs/db/src/schema/emergencyContact.ts (1)
1-1: LGTM: Imports are appropriate and used correctly.The necessary functions from 'drizzle-orm/pg-core' are imported for defining the PostgreSQL table schema using Drizzle ORM.
libs/db/src/schema/userPreferences.ts (2)
17-17: Ensure default value forpreferredLanguageaccommodates all users.While 'EN' is set as the default, consider if this is appropriate for your user base. If the application supports multiple languages, users might prefer to set their language during registration.
23-27: Verify handling of empty arrays for preference fields.Ensure that the application and database correctly handle scenarios where users have not selected any options for
dietRestrictions,trackPreferences,interests,disabilities, andapplicableSkills. This might involve setting default empty arrays or allowing null values.Consider adding default empty arrays:
- dietRestrictions: dietRestrictionsEnum('dietRestrictions').array(), + dietRestrictions: dietRestrictionsEnum('dietRestrictions').array().default([]),Apply similar changes to the other array fields if appropriate.
libs/db/src/schema/user.ts (3)
32-35: Ensure foreign key constraints are correctly enforcedThe foreign keys
resumeId,schoolId,emergencyContactId, andteamIdmay require.notNull()if these relationships are mandatory.Please verify if these associations are optional. If they are required, consider:
-resumeId: integer('resumeId').references(() => resume.id), +resumeId: integer('resumeId').references(() => resume.id).notNull(),Repeat for the other foreign keys as needed.
21-21: HandledateOfBirthwith care for complianceCollecting
dateOfBirthmay have implications under data protection regulations like GDPR or COPPA, especially for users under certain ages.Ensure that:
- Appropriate consent is obtained.
- Data is stored and processed in compliance with regulations.
- Age-related access controls are implemented if necessary.
26-26: Check the datatype fornumHackathonsAttendedUsing
smallintlimits the maximum value to 32,767. If users might attend more hackathons, or if this field could be misused, consider using a larger integer type.
| export const resume = pgTable('resume', { | ||
| id: serial('id').primaryKey(), | ||
| fileLink: varchar('fileLink', { length: 128 }), | ||
| hasPrivacyToggle: boolean('hasPrivacyToggle').default(false), | ||
| uploadedAt: timestamp('uploadedAt').defaultNow(), | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing the resume table structure.
The current schema is a good start, but consider the following improvements:
- Add a foreign key to link resumes with users:
userId: integer('userId').notNull().references(() => users.id),- Make the fileLink field required:
fileLink: varchar('fileLink', { length: 128 }).notNull(),- Add an index on the fileLink field for better query performance:
{ name: 'fileLink_idx', columns: [resume.fileLink] }- Consider adding a 'name' field for the resume, which could be useful for display purposes:
name: varchar('name', { length: 255 }).notNull(),These changes would enhance data integrity and query performance.
| @@ -0,0 +1,9 @@ | |||
| import { pgEnum, pgTable, serial, varchar } from 'drizzle-orm/pg-core' | |||
|
|
|||
| export const levelOfStudyEnum = pgEnum('levelOfStudy', ['graduateSchool', 'highSchool']) | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider expanding the levelOfStudyEnum.
The current enum covers graduate and high school levels, which might not be comprehensive enough for all potential use cases. Consider including additional levels such as 'undergraduate', 'middleSchool', or 'elementary' to make the schema more flexible for future requirements.
Here's a suggested expansion of the enum:
export const levelOfStudyEnum = pgEnum('levelOfStudy', [
'elementary',
'middleSchool',
'highSchool',
'undergraduate',
'graduateSchool'
])| export const socialMedia = pgTable('socialMedia', { | ||
| id: serial('id').primaryKey(), | ||
| userId: integer('userId') | ||
| .references(() => user.id), | ||
| platformName: varchar('platformName', { length: 16 }), | ||
| profileUrl: varchar('profileUrl', { length: 128 }), | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using snake_case for table and column names.
While the current naming convention is consistent, it's generally recommended to use snake_case for table and column names in PostgreSQL. This aligns better with PostgreSQL conventions and avoids potential issues with case-sensitivity in queries.
Here's how you could modify the schema to use snake_case:
export const socialMedia = pgTable('social_media', {
id: serial('id').primaryKey(),
user_id: integer('user_id').notNull()
.references(() => user.id),
platform_name: varchar('platform_name', { length: 16 }).notNull(),
profile_url: varchar('profile_url', { length: 128 }).notNull(),
}, (table) => ({
userPlatformUnique: unique().on(table.user_id, table.platform_name),
}));Remember to update any references to these fields in other parts of your codebase if you make this change.
🛠️ Refactor suggestion
Consider adding additional useful fields to the schema.
The current schema covers the basic needs for a social media link table. However, consider adding the following fields to enhance functionality and data tracking:
created_at: A timestamp to track when the social media link was added.updated_at: A timestamp to track when the link was last modified.visibility: An enum field to allow users to control the visibility of their social media links (e.g., 'public', 'private', 'friends_only').
Here's an example of how you might implement these additions:
import { integer, pgTable, serial, varchar, timestamp, pgEnum } from 'drizzle-orm/pg-core'
export const visibilityEnum = pgEnum('visibility', ['public', 'private', 'friends_only']);
export const socialMedia = pgTable('social_media', {
id: serial('id').primaryKey(),
user_id: integer('user_id').notNull()
.references(() => user.id),
platform_name: varchar('platform_name', { length: 16 }).notNull(),
profile_url: varchar('profile_url', { length: 128 }).notNull(),
visibility: visibilityEnum('visibility').default('public').notNull(),
created_at: timestamp('created_at').defaultNow().notNull(),
updated_at: timestamp('updated_at').defaultNow().notNull(),
}, (table) => ({
userPlatformUnique: unique().on(table.user_id, table.platform_name),
}));These additions would provide more comprehensive data management and user control over their social media links.
| export const preferredLanguageEnum = pgEnum('preferredLanguage', ['EN']) | ||
| export const eventPreferencesEnum = pgEnum('eventPreferences', ['hardware', 'software']) | ||
| export const shirtSizeEnum = pgEnum('shirtSize', ['XS', 'S', 'M', 'L', 'XL', 'XXL']) | ||
| export const pronounsEnum = pgEnum('pronouns', ['he/him', 'she/her', 'they/them', 'other']) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a 'Prefer not to say' option to pronounsEnum.
Including a 'Prefer not to say' option allows users who do not wish to disclose their pronouns to have an appropriate selection.
| export const eventPreferencesEnum = pgEnum('eventPreferences', ['hardware', 'software']) | ||
| export const shirtSizeEnum = pgEnum('shirtSize', ['XS', 'S', 'M', 'L', 'XL', 'XXL']) | ||
| export const pronounsEnum = pgEnum('pronouns', ['he/him', 'she/her', 'they/them', 'other']) | ||
| export const dietRestrictionsEnum = pgEnum('dietRestrictions', ['allergies', 'vegan', 'none']) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expand dietRestrictionsEnum to include more dietary options.
Currently, dietRestrictionsEnum includes ['allergies', 'vegan', 'none']. To accommodate a wider range of dietary needs, consider adding options such as 'vegetarian', 'gluten-free', 'halal', 'kosher', 'nut-free', etc.
|
|
||
| export const userPreferences = pgTable('userPreferences', { | ||
| id: serial('id').primaryKey(), | ||
| userId: integer('userId').references(() => user.id), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Specify onDelete and onUpdate actions for the foreign key userId.
To maintain referential integrity, it's important to define what happens to userPreferences entries when a referenced user is deleted or updated. Consider specifying onDelete and onUpdate behaviors.
Apply the following change:
- userId: integer('userId').references(() => user.id),
+ userId: integer('userId').references(() => user.id, { onDelete: 'cascade', onUpdate: 'cascade' }),📝 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.
| userId: integer('userId').references(() => user.id), | |
| userId: integer('userId').references(() => user.id, { onDelete: 'cascade', onUpdate: 'cascade' }), |
| interests: interestsEnum('interests').array(), | ||
| disabilities: disabilitiesEnum('disabilities').array(), | ||
| applicableSkills: applicableSkillsEnum('applicableSkills').array(), | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add timestamps for record tracking.
Including createdAt and updatedAt timestamps can be beneficial for tracking when records are created and modified.
Add the following fields to your table definition:
+ createdAt: timestamp('createdAt').defaultNow(),
+ updatedAt: timestamp('updatedAt').defaultNow().onUpdateNow(),Ensure you import timestamp from drizzle-orm/pg-core:
import { boolean, integer, pgEnum, pgTable, serial, timestamp } from 'drizzle-orm/pg-core'Committable suggestion was skipped due to low confidence.
| import { emergencyContact } from './emergencyContact' | ||
| import { team } from './team' | ||
|
|
||
| export const genderEnum = pgEnum('gender', ['male', 'female', 'other']) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider expanding genderEnum for inclusivity
Currently, genderEnum includes 'male', 'female', and 'other'. To be more inclusive of all gender identities, consider expanding this enum or allowing users to self-identify their gender.
Possible code change:
-export const genderEnum = pgEnum('gender', ['male', 'female', 'other'])
+export const genderEnum = pgEnum('gender', ['male', 'female', 'non-binary', 'prefer not to say', 'other'])Alternatively, you might use a varchar field with validation to allow for more flexibility.
📝 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.
| export const genderEnum = pgEnum('gender', ['male', 'female', 'other']) | |
| export const genderEnum = pgEnum('gender', ['male', 'female', 'non-binary', 'prefer not to say', 'other']) |
There was a problem hiding this comment.
Might as well do this now, agree with this 👍
There was a problem hiding this comment.
@HasithDeAlwis, great! Happy to hear that you're making this update. 😊
(\_/)
(•ㅅ•)
/ づ
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| import { team } from './team' | ||
|
|
||
| export const genderEnum = pgEnum('gender', ['male', 'female', 'other']) | ||
| export const phoneNumberCountryCodeEnum = pgEnum('phoneNumberCountryCode', ['+1']) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Re-evaluate phoneNumberCountryCodeEnum for international support
The phoneNumberCountryCodeEnum currently only includes '+'1', limiting users to those with North American phone numbers. If international users are expected, consider expanding this enum or using a varchar field.
Possible code change:
-export const phoneNumberCountryCodeEnum = pgEnum('phoneNumberCountryCode', ['+1'])
+export const phoneNumberCountryCodeEnum = varchar('phoneNumberCountryCode', { length: 5 }).notNull()This change allows for a wider range of country codes while ensuring the field is not null.
📝 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.
| export const phoneNumberCountryCodeEnum = pgEnum('phoneNumberCountryCode', ['+1']) | |
| export const phoneNumberCountryCodeEnum = varchar('phoneNumberCountryCode', { length: 5 }).notNull() |
| isDomestic: boolean('isDomestic'), | ||
| ethnicity: ethnicityEnum('ethnicity'), | ||
| estimatedGradYear: integer('estimatedGradYear'), | ||
| agreeToMlhReqs: boolean('agreeToMlhReqs').default(false), |
There was a problem hiding this comment.
Reconsider default value for agreeToMlhReqs
The agreeToMlhReqs field defaults to false, which might not be the desired behavior if agreeing is mandatory for participation.
Possible code change:
-agreeToMlhReqs: boolean('agreeToMlhReqs').default(false),
+agreeToMlhReqs: boolean('agreeToMlhReqs').notNull(),By removing the default and adding .notNull(), you ensure that the user must explicitly agree.
📝 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.
| agreeToMlhReqs: boolean('agreeToMlhReqs').default(false), | |
| agreeToMlhReqs: boolean('agreeToMlhReqs').notNull(), |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/docs/next.config.mjs (1)
19-26: LGTM! Consider future-proofing the configuration.The addition of the
imagesconfiguration is correct and well-structured. It properly allows remote images from GitHub avatars, which is suitable for a documentation site.For future-proofing, consider using an environment variable for the hostname. This would make it easier to change or add additional hostnames in different environments. For example:
images: { remotePatterns: [ { protocol: 'https', hostname: process.env.ALLOWED_IMAGE_HOSTNAME || 'avatars.githubusercontent.com', }, ], },This change is optional and can be implemented in a future update if needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/docs/src/content/docs/libraries/db.mdx (1)
13-116: Excellent addition of the Mermaid ERD!The Entity-Relationship Diagram provides a clear and comprehensive visual representation of the database schema, which is crucial for understanding the system's structure. This aligns well with the PR objectives of creating initial portal database schemas.
However, I have a few suggestions for improvement:
Consider merging
UserPreferencesinto theUsertable as mentioned in the PR objectives. This could simplify the schema and reduce the number of joins needed in queries.The
Teamentity has bothprofileImageUrlandteamProfileImageUrl. This seems redundant and could be consolidated.In the
Userentity,resumeId,schoolId,emergencyContactId, andteamIdare marked as mandatory relationships. Consider if all of these should be required upon user creation or if some could be optional.The
SocialMediaentity has a many-to-one relationship withUser, but theuserIdis marked as optional. Consider if this should be mandatory or if there's a use case for social media entries not associated with a user.In the relationships section,
Userhas a one-to-many relationship withEmergencyContact, but typically a user would have only one emergency contact. Consider if this should be a one-to-one relationship.Would you like assistance in implementing any of these suggestions?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apps/docs/src/content/docs/libraries/db.mdx (3 hunks)
🧰 Additional context used
🔇 Additional comments (2)
apps/docs/src/content/docs/libraries/db.mdx (2)
176-179: Improved clarity in Drizzle-Kit commands sectionThe changes in this section enhance the documentation by providing more specific information about Drizzle-Kit commands and including an example of how to run a migration. This improvement in clarity will be helpful for developers using the library.
229-229: Helpful addition of troubleshooting resourceThe inclusion of a link to DrizzleORM's troubleshooting page is a valuable addition to the documentation. It provides users with an official resource for resolving issues they may encounter while working with DrizzleORM.
JeremyFriesenGitHub
force-pushed
the
jowi/feat/133-create-initial-portal-database-schemas
branch
from
3889133
to
9c3fae1
Compare
JeremyFriesenGitHub
force-pushed
the
jowi/feat/133-create-initial-portal-database-schemas
branch
from
9c3fae1
to
33a5186
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/docs/next.config.mjs (2)
20-27: LGTM: Image optimization configuration added.The
imagesconfiguration is well-structured and allows for flexible remote image optimization. Using an environment variable for the hostname enhances security and configurability.Consider adding validation for the
ALLOWED_IMAGE_HOSTNAMEenvironment variable to ensure it's a valid hostname. This can prevent potential security issues if the environment variable is misconfigured. Here's a suggested implementation:const validateHostname = (hostname) => { const hostnameRegex = /^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$/; return hostnameRegex.test(hostname); }; const allowedImageHostname = process.env.ALLOWED_IMAGE_HOSTNAME || 'avatars.githubusercontent.com'; if (!validateHostname(allowedImageHostname)) { throw new Error(`Invalid ALLOWED_IMAGE_HOSTNAME: ${allowedImageHostname}`); } // ... rest of the config images: { remotePatterns: [ { protocol: 'https', hostname: allowedImageHostname, }, ], },This addition will validate the hostname before using it in the configuration, providing an extra layer of security.
Line range hint
1-27: Summary: Image optimization configuration enhances flexibility and security.The changes to this file are minimal but impactful. They introduce image optimization configuration that aligns with Next.js best practices. The use of environment variables for the hostname adds flexibility and enhances security. These changes support the PR's objective of setting up initial configurations for the portal.
As the project evolves, consider the following:
- Implement a centralized configuration management system if more environment-based configs are added in the future.
- Document the
ALLOWED_IMAGE_HOSTNAMEenvironment variable in your project's documentation or README to ensure other developers are aware of its usage and importance.apps/docs/src/content/docs/libraries/db.mdx (1)
13-116: Great addition of the Mermaid ERD! A few suggestions for improvement:The Entity-Relationship Diagram provides a clear overview of the database schema. Here are some suggestions to enhance it further:
Consider adding more specific enumerations for fields like
trackPreferencesandapplicableSkillsin the UserPreferences entity. This will help maintain consistency and facilitate easier querying.The
agreeToMlhReqsfield in the User entity might benefit from a more descriptive name or additional documentation to clarify its purpose.Review the cardinality of relationships:
- The User to EmergencyContact relationship is currently many-to-one. Consider if a user should have multiple emergency contacts (one-to-many).
- The User to Team relationship is currently optional many-to-one. Clarify if users can be part of multiple teams or if teams can have multiple users.
Consider adding created_at and updated_at timestamp fields to entities for audit purposes.
Would you like me to propose specific changes to address these points?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- apps/docs/next.config.mjs (2 hunks)
- apps/docs/src/content/docs/knowledge-base/cuHacking-2020-wiki/meta.json (0 hunks)
- apps/docs/src/content/docs/libraries/db.mdx (3 hunks)
💤 Files with no reviewable changes (1)
- apps/docs/src/content/docs/knowledge-base/cuHacking-2020-wiki/meta.json
🧰 Additional context used
🔇 Additional comments (3)
apps/docs/next.config.mjs (1)
1-1: LGTM: Explicit import ofprocessmodule.The explicit import of the
processmodule is correct and follows best practices for Next.js configurations, especially in newer versions where Node.js APIs might need to be imported explicitly.apps/docs/src/content/docs/libraries/db.mdx (2)
176-177: Improved clarity and added helpful link.The text adjustments enhance readability and provide more comprehensive information about available Drizzle-Kit commands. The addition of the link to the official documentation is particularly useful for users who need more detailed information.
229-229: Helpful addition of troubleshooting resource.The inclusion of a link to DrizzleORM's official troubleshooting page is a valuable addition. It provides users with an authoritative source for resolving issues they might encounter while working with the library.
|
@JeremyFriesenGitHub Is it possible to link a publicly visible live link to the actual mermaid code? |
i have no idea atp, im not sure. |
JowiAoun
force-pushed
the
jowi/feat/133-create-initial-portal-database-schemas
branch
from
33a5186
to
4dc2a04
Compare
✅ Deploy Preview for website-cuhacking ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (14)
- apps/docs/next.config.mjs (2 hunks)
- apps/docs/src/content/docs/knowledge-base/cuHacking-2020-wiki/meta.json (0 hunks)
- apps/docs/src/content/docs/libraries/db.mdx (3 hunks)
- libs/db/src/index.ts (1 hunks)
- libs/db/src/schema/emergencyContact.ts (1 hunks)
- libs/db/src/schema/index.ts (1 hunks)
- libs/db/src/schema/program.ts (1 hunks)
- libs/db/src/schema/resume.ts (1 hunks)
- libs/db/src/schema/school.ts (1 hunks)
- libs/db/src/schema/session.ts (1 hunks)
- libs/db/src/schema/socialMedia.ts (1 hunks)
- libs/db/src/schema/team.ts (1 hunks)
- libs/db/src/schema/user.ts (1 hunks)
- libs/db/src/schema/userPreferences.ts (1 hunks)
💤 Files with no reviewable changes (1)
- apps/docs/src/content/docs/knowledge-base/cuHacking-2020-wiki/meta.json
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/docs/next.config.mjs
- apps/docs/src/content/docs/libraries/db.mdx
- libs/db/src/index.ts
- libs/db/src/schema/emergencyContact.ts
- libs/db/src/schema/index.ts
- libs/db/src/schema/program.ts
- libs/db/src/schema/resume.ts
- libs/db/src/schema/school.ts
- libs/db/src/schema/session.ts
- libs/db/src/schema/socialMedia.ts
- libs/db/src/schema/team.ts
🧰 Additional context used
🔇 Additional comments (10)
libs/db/src/schema/userPreferences.ts (9)
1-2: Imports look good!The necessary functions from 'drizzle-orm/pg-core' are imported, and the 'user' import suggests proper modularization of the schema.
1-28: Overall assessment: Good foundation with room for improvementThe
userPreferencesschema provides a solid foundation for storing user preferences. The suggested improvements will enhance the schema's inclusivity, flexibility, and data integrity. Key areas for improvement include:
- Expanding enum options to be more inclusive and comprehensive.
- Adding referential integrity constraints to the foreign key.
- Considering array types for multi-select preferences.
- Adding timestamps for better record tracking.
- Implementing a unique constraint on
userId.These changes will result in a more robust and user-friendly schema. Great work on setting up this initial structure!
28-28: 🛠️ Refactor suggestionAdd timestamps for record tracking.
Including
createdAtandupdatedAttimestamps can be beneficial for tracking when records are created and modified.applicableSkills: applicableSkillsEnum('applicableSkills').array(), + createdAt: timestamp('createdAt').defaultNow(), + updatedAt: timestamp('updatedAt').defaultNow().onUpdateNow(), })Don't forget to import
timestampat the top of the file:-import { boolean, integer, pgEnum, pgTable, serial } from 'drizzle-orm/pg-core' +import { boolean, integer, pgEnum, pgTable, serial, timestamp } from 'drizzle-orm/pg-core'Likely invalid or redundant comment.
16-16: 🛠️ Refactor suggestionSpecify
onDeleteandonUpdateactions for the foreign keyuserId.To maintain referential integrity, it's important to define what happens to
userPreferencesentries when a referenceduseris deleted or updated.- userId: integer('userId').references(() => user.id), + userId: integer('userId').references(() => user.id, { onDelete: 'cascade', onUpdate: 'cascade' }),Likely invalid or redundant comment.
7-7: 🛠️ Refactor suggestionAdd a 'Prefer not to say' option to
pronounsEnum.Including a 'Prefer not to say' option allows users who do not wish to disclose their pronouns to have an appropriate selection.
-export const pronounsEnum = pgEnum('pronouns', ['he/him', 'she/her', 'they/them', 'other']) +export const pronounsEnum = pgEnum('pronouns', ['he/him', 'she/her', 'they/them', 'other', 'prefer not to say'])Likely invalid or redundant comment.
18-18: 🛠️ Refactor suggestionConsider allowing multiple selections for
eventPreferences.If users can select multiple event preferences, you should define
eventPreferencesas an array type.- eventPreferences: eventPreferencesEnum('eventPreferences'), + eventPreferences: eventPreferencesEnum('eventPreferences').array(),Likely invalid or redundant comment.
11-11: 🛠️ Refactor suggestionConsider adding 'Other' to
disabilitiesEnum.Including an 'Other' option allows users with disabilities not listed to accurately represent their needs.
-export const disabilitiesEnum = pgEnum('disabilities', ['mobility', 'visual', 'hearing', 'cognitive', 'mental']) +export const disabilitiesEnum = pgEnum('disabilities', ['mobility', 'visual', 'hearing', 'cognitive', 'mental', 'other'])Likely invalid or redundant comment.
8-8: 🛠️ Refactor suggestionExpand
dietRestrictionsEnumto include more dietary options.To accommodate a wider range of dietary needs, consider adding more options.
-export const dietRestrictionsEnum = pgEnum('dietRestrictions', ['allergies', 'vegan', 'none']) +export const dietRestrictionsEnum = pgEnum('dietRestrictions', ['allergies', 'vegan', 'vegetarian', 'gluten-free', 'halal', 'kosher', 'nut-free', 'none', 'other'])Likely invalid or redundant comment.
9-9: 🛠️ Refactor suggestionBroaden the options in
trackPreferencesEnum.If the event offers additional tracks, consider expanding this enum to provide users with more accurate preference options.
-export const trackPreferencesEnum = pgEnum('trackPreferences', ['hardware', 'software']) +export const trackPreferencesEnum = pgEnum('trackPreferences', ['hardware', 'software', 'design', 'data_science', 'cybersecurity', 'other'])Likely invalid or redundant comment.
libs/db/src/schema/user.ts (1)
1-5: LGTM: Imports are appropriate and used correctlyThe imports from 'drizzle-orm/pg-core' and other schema files are correctly utilized in the schema definition, providing necessary types and references for foreign keys.
| export const userPreferences = pgTable('userPreferences', { | ||
| id: serial('id').primaryKey(), | ||
| userId: integer('userId').references(() => user.id), | ||
| preferredLanguage: preferredLanguageEnum('preferredLanguage').default('EN'), | ||
| eventPreferences: eventPreferencesEnum('eventPreferences'), | ||
| privacyMode: boolean('privacyMode').default(false), | ||
| isSubscribedToNewsletter: boolean('isSubscribedToNewsletter').default(false), | ||
| shirtSize: shirtSizeEnum('shirtSize'), | ||
| pronouns: pronounsEnum('pronouns'), | ||
| dietRestrictions: dietRestrictionsEnum('dietRestrictions').array(), | ||
| trackPreferences: trackPreferencesEnum('trackPreferences').array(), | ||
| interests: interestsEnum('interests').array(), | ||
| disabilities: disabilitiesEnum('disabilities').array(), | ||
| applicableSkills: applicableSkillsEnum('applicableSkills').array(), | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding a unique constraint on userId.
To ensure that each user has only one set of preferences, you might want to add a unique constraint on the userId field.
Add the following after the table definition:
export const userPreferencesConstraints = pgTable('userPreferences', {
userIdUnique: unique('userIdUnique').on(userPreferences.userId),
})Don't forget to import unique at the top of the file:
-import { boolean, integer, pgEnum, pgTable, serial, timestamp } from 'drizzle-orm/pg-core'
+import { boolean, integer, pgEnum, pgTable, serial, timestamp, unique } from 'drizzle-orm/pg-core'| export const pronounsEnum = pgEnum('pronouns', ['he/him', 'she/her', 'they/them', 'other']) | ||
| export const dietRestrictionsEnum = pgEnum('dietRestrictions', ['allergies', 'vegan', 'none']) | ||
| export const trackPreferencesEnum = pgEnum('trackPreferences', ['hardware', 'software']) | ||
| export const interestsEnum = pgEnum('interests', ['languages']) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance interestsEnum with more categories.
The current interestsEnum may be too narrow to capture users' varied interests.
-export const interestsEnum = pgEnum('interests', ['languages'])
+export const interestsEnum = pgEnum('interests', ['languages', 'AI/ML', 'web_development', 'data_science', 'cybersecurity', 'mobile_development', 'game_development', 'iot', 'blockchain', 'other'])📝 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.
| export const interestsEnum = pgEnum('interests', ['languages']) | |
| export const interestsEnum = pgEnum('interests', ['languages', 'AI/ML', 'web_development', 'data_science', 'cybersecurity', 'mobile_development', 'game_development', 'iot', 'blockchain', 'other']) |
| firstName: varchar('firstName', { length: 32 }).notNull(), | ||
| middleName: varchar('middleName', { length: 32 }), | ||
| lastName: varchar('lastName', { length: 32 }).notNull(), | ||
| preferredName: varchar('preferredName', { length: 32 }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding length validation for name fields
While the varchar length is set, consider adding additional validation to ensure minimum lengths and character types for name fields. This can help prevent empty or invalid inputs.
Example:
firstName: varchar('firstName', { length: 32 }).notNull().check(sql`length(firstName) >= 2`),| isEmailVerified: boolean('isEmailVerified').default(false), | ||
| avatarUrl: varchar('avatarUrl', { length: 128 }), | ||
| profileDescription: text('profileDescription'), | ||
| dateOfBirth: date('dateOfBirth'), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add age validation for dateOfBirth
Consider adding a check constraint to ensure the user is of appropriate age:
dateOfBirth: date('dateOfBirth').check(sql`dateOfBirth <= current_date - interval '13 years'`),This example ensures users are at least 13 years old, but adjust the age limit according to your application's requirements.
| anyOtherComments: text('anyOtherComments'), | ||
| isDomestic: boolean('isDomestic'), | ||
| ethnicity: ethnicityEnum('ethnicity'), | ||
| estimatedGradYear: integer('estimatedGradYear'), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate estimatedGradYear
Add a check constraint to ensure estimatedGradYear is within a reasonable range:
estimatedGradYear: integer('estimatedGradYear').check(sql`estimatedGradYear BETWEEN EXTRACT(YEAR FROM CURRENT_DATE) AND EXTRACT(YEAR FROM CURRENT_DATE) + 10`),This ensures the graduation year is between the current year and 10 years in the future.
| id: serial('id').primaryKey(), | ||
| firstName: varchar('firstName', { length: 32 }).notNull(), | ||
| middleName: varchar('middleName', { length: 32 }), | ||
| lastName: varchar('lastName', { length: 32 }).notNull(), | ||
| preferredName: varchar('preferredName', { length: 32 }), | ||
| email: varchar('email', { length: 64 }).unique().notNull(), | ||
| isEmailVerified: boolean('isEmailVerified').default(false), | ||
| avatarUrl: varchar('avatarUrl', { length: 128 }), | ||
| profileDescription: text('profileDescription'), | ||
| dateOfBirth: date('dateOfBirth'), | ||
| gender: genderEnum('gender'), | ||
| phoneNumberCountryCode: phoneNumberCountryCodeEnum('phoneNumberCountryCode'), | ||
| phoneNumber: varchar('phoneNumber', { length: 16 }).unique(), | ||
| isPhoneNumberVerified: boolean('isPhoneNumberVerified').default(false), | ||
| numHackathonsAttended: smallint('numHackathonsAttended').default(0), | ||
| anyOtherComments: text('anyOtherComments'), | ||
| isDomestic: boolean('isDomestic'), | ||
| ethnicity: ethnicityEnum('ethnicity'), | ||
| estimatedGradYear: integer('estimatedGradYear'), | ||
| agreeToMlhReqs: boolean('agreeToMlhReqs').default(false), | ||
| resumeId: integer('resumeId').references(() => resume.id), | ||
| schoolId: integer('schoolId').references(() => school.id), | ||
| emergencyContactId: integer('emergencyContactId').references(() => emergencyContact.id), | ||
| teamId: integer('teamId').references(() => team.id), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding created_at and updated_at fields
To improve record tracking and auditing capabilities, consider adding created_at and updated_at timestamp fields to the user table:
created_at: timestamp('created_at').defaultNow().notNull(),
updated_at: timestamp('updated_at').defaultNow().notNull(),These fields can be very useful for debugging, auditing, and analyzing user data over time.
| email: varchar('email', { length: 64 }).unique().notNull(), | ||
| isEmailVerified: boolean('isEmailVerified').default(false), | ||
| avatarUrl: varchar('avatarUrl', { length: 128 }), | ||
| profileDescription: text('profileDescription'), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add length constraint to profileDescription
Consider adding a maximum length constraint to the profileDescription field to prevent excessively long inputs:
-profileDescription: text('profileDescription'),
+profileDescription: varchar('profileDescription', { length: 500 }),This change limits the description to 500 characters, which should be sufficient for most use cases while preventing potential abuse.
📝 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.
| profileDescription: text('profileDescription'), | |
| profileDescription: varchar('profileDescription', { length: 500 }), |
| phoneNumberCountryCode: phoneNumberCountryCodeEnum('phoneNumberCountryCode'), | ||
| phoneNumber: varchar('phoneNumber', { length: 16 }).unique(), | ||
| isPhoneNumberVerified: boolean('isPhoneNumberVerified').default(false), | ||
| numHackathonsAttended: smallint('numHackathonsAttended').default(0), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add upper limit to numHackathonsAttended
Consider adding an upper limit to numHackathonsAttended to prevent unrealistic values:
-numHackathonsAttended: smallint('numHackathonsAttended').default(0),
+numHackathonsAttended: smallint('numHackathonsAttended').default(0).check(sql`numHackathonsAttended >= 0 AND numHackathonsAttended <= 100`),This change ensures the value is non-negative and sets a reasonable upper limit of 100 hackathons.
📝 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.
| numHackathonsAttended: smallint('numHackathonsAttended').default(0), | |
| numHackathonsAttended: smallint('numHackathonsAttended').default(0).check(sql`numHackathonsAttended >= 0 AND numHackathonsAttended <= 100`), |
|
|
||
| export const genderEnum = pgEnum('gender', ['male', 'female', 'other']) | ||
| export const phoneNumberCountryCodeEnum = pgEnum('phoneNumberCountryCode', ['+1']) | ||
| export const ethnicityEnum = pgEnum('ethnicity', ['black', 'white', 'asian', 'hispanic', 'middle-eastern']) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider further improvements to ethnicityEnum
While the previous comment about ethnicityEnum is still valid, consider these additional suggestions:
- Add an 'other' option to allow for ethnicities not listed.
- Consider using broader categories or allowing multiple selections.
- Consult with diversity and inclusion experts to ensure the categories are appropriate and inclusive.
Example improvement:
-export const ethnicityEnum = pgEnum('ethnicity', ['black', 'white', 'asian', 'hispanic', 'middle-eastern'])
+export const ethnicityEnum = pgEnum('ethnicity', ['black', 'white', 'asian', 'hispanic', 'middle_eastern', 'indigenous', 'pacific_islander', 'multiracial', 'other', 'prefer_not_to_say'])📝 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.
| export const ethnicityEnum = pgEnum('ethnicity', ['black', 'white', 'asian', 'hispanic', 'middle-eastern']) | |
| export const ethnicityEnum = pgEnum('ethnicity', ['black', 'white', 'asian', 'hispanic', 'middle_eastern', 'indigenous', 'pacific_islander', 'multiracial', 'other', 'prefer_not_to_say']) |
After looking at this for a while, I have no idea what u mean by this. Could you clarify @MFarabi619 |
|
chore(blocking): elaborate on behaviour for fix commits I know it's strange that I nitpick on commit messages, it's because I review code very differently from most others; and in extremely close adherence to how I expect our code to change for trunk-based dev. When I review code, I go by scanning commit history.
If the commit message is redundant/misleading, that makes it very difficult to review code.
For You also want to avoid mentioning the types and scopes in the description of the message
When you're scanning the commit history, which one is more helpful?
Or
The tools-overview page was completely crashed and non functional for a bit due to the missing image loader, where was the breaking change introduced and how'd that make it past tests? :/ Avoid making bugfixes from unmerged features on the PR unless in dire emergencies that require velocity. Just squash/amend those fixes to your feature. A Essentially answering who, what, where, when, why as much as possible. The commit message metadata already has the who, and when. The Issue+PR covers most of the 'why'. With your commit message you want to answer the what, and where, and it's EXTREMELY important for this to be laser-precise. This is the final level at which I decide whether to jump into code or not and find out. Commit messages are like road signs. They simply cannot be misleading/unhelpful. If you tell me to turn left/right, then I will turn left/right.
Codeowners are expected to look after the conventions and best practices of their respective areas to ensure they scale manageably and are consistent with the rest of the ecosystem. Conventions aren't set in stone and you can propose to change them. Haven't completed my review, will do another pass later. Let me know what you guys think. |
There was a problem hiding this comment.
Can we add TODO comments to add checks. Read more here.
I think all URLs need this.
Other than that good work on this, let's clean up a tiny bit of things then get this merged in.
| @@ -16,6 +17,14 @@ const nextConfig = { | |||
| // reactStrictMode: true, | |||
| // output: 'standalone', | |||
| }, | |||
| images: { | |||
There was a problem hiding this comment.
might be worth adding a comment here explaining why we need this!
| import { emergencyContact } from './emergencyContact' | ||
| import { team } from './team' | ||
|
|
||
| export const genderEnum = pgEnum('gender', ['male', 'female', 'other']) |
There was a problem hiding this comment.
Might as well do this now, agree with this 👍
| @@ -4,10 +4,10 @@ import { user } from './user' | |||
|
|
|||
| export const session = pgTable('session', { | |||
There was a problem hiding this comment.
are we good to make this table before finalizing the auth provider?
| @@ -0,0 +1,9 @@ | |||
| import { pgEnum, pgTable, serial, varchar } from 'drizzle-orm/pg-core' | |||
|
|
|||
| export const levelOfStudyEnum = pgEnum('levelOfStudy', ['graduateSchool', 'highSchool']) | |||
There was a problem hiding this comment.
no need for highschool right now, should be added on later imo
|
|
||
| export const levelOfStudyEnum = pgEnum('levelOfStudy', ['graduateSchool', 'highSchool']) | ||
|
|
||
| export const school = pgTable('school', { |
There was a problem hiding this comment.
Good idea to add a location to this as well
| id: serial('id').primaryKey(), | ||
| userId: integer('userId') | ||
| .references(() => user.id), | ||
| platformName: varchar('platformName', { length: 16 }), |
There was a problem hiding this comment.
this should reference another Platform table so that we can regulate the social medias that we accept
| export const resume = pgTable('resume', { | ||
| id: serial('id').primaryKey(), | ||
| fileLink: varchar('fileLink', { length: 128 }), | ||
| hasPrivacyToggle: boolean('hasPrivacyToggle').default(false), |
MFarabi619
force-pushed
the
jowi/feat/133-create-initial-portal-database-schemas
branch
from
4dc2a04
to
479b8a2
Compare
✅ Deploy Preview for docs-cuhacking ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Closing as stale |
🚩 Linked issue
Here is the linked issue.
🛠 How does this PR address the issue(s)? How does it solve the problem?
This PR implements a Drizzle PSQL DB for the Portal User Profile creation. For now, this is a temporary, rough-draft, tentative schema, that can be changed at any time.
🧠 Describe any alternatives you've considered
Drizzle is type safe, we can get direct access to attributes, we can make queries easily and we are able to migrate easily and efficiently towards other databases or make/incorporate changes easily into the db if needed. This is why setting up an ORM is extremely critical for our backend infrastructure and will save a lot of time.
✨ Dependency Changes (if applicable)
N/A
📸 Before and After Screenshots (if applicable)
✍️ Additional comments (if applicable)
We updated a bit of the database design. There are more changes to come for that design (for instance, UserPreferances table should be merged with the User table). But for now, we are going to set everything up and we will be able to run migrations later thanks to DrizzleORM.
Summary by CodeRabbit
Release Notes
New Features
emergencyContact,program,resume,school,socialMedia,team, anduserPreferences, enhancing data management capabilities.Improvements
These updates improve the overall functionality and user experience within the application.