Conversation
- Added db-data directory to .gitignore. - Updated docker-compose.yml to use local db-data volume and improved healthcheck command with environment variables. - Enhanced README with automated setup instructions and clarified manual setup steps. - Added db:setup script to package.json for database initialization. - Refactored Bookmark and User entities to extend BaseEntity for consistent timestamp handling. - Updated migration to use gen_random_uuid() for ID generation. - Revised .env.example to include new environment variables for GitHub integration.
WalkthroughThe changes introduce automated setup and database management for the project. A new setup script and supporting documentation streamline environment configuration, dependency installation, and database initialization. Supporting scripts and configuration files were added or updated for environment variables, Docker, and TypeORM migrations. Entity classes were refactored to inherit timestamp fields from a shared base class. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Setup as setup.sh
participant Docker as Docker Compose
participant DB as PostgreSQL Container
participant Server as Server App
Dev->>Setup: Run setup.sh
Setup->>Setup: Check Node.js, pnpm, Docker
Setup->>Setup: Install dependencies
Setup->>Setup: Copy .env.example to .env (if missing)
Setup->>Docker: Start PostgreSQL container
Docker->>DB: Launches and waits for healthy state
Setup->>DB: Wait for DB readiness
Setup->>Server: Run database migrations
Server->>DB: Apply migrations
Setup->>Dev: Print next steps and URLs
Poem
✨ Finishing Touches
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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Pull Request Overview
This PR streamlines developer onboarding by adding automated setup scripts, refining environment configurations, and standardizing database and entity handling.
- Introduced
setup.shanddb:setupscript for fully automated project and database initialization - Updated Docker Compose and
.envexamples to use local volumes, environment variables, and GitHub integration - Refactored entities to extend
BaseEntity(timestamp consistency) and revised migrations to usegen_random_uuid()
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.sh | New root setup script for prerequisites, env setup, Docker, and migrations |
| docker-compose.yml | Switched volume to ./db-data, improved healthcheck with env vars |
| apps/web/.env.example | Updated example for GitHub OAuth variables |
| apps/server/.env.example | Added core DB and JWT variables |
| apps/server/src/users/entities/user.entity.ts | Adjusted import paths and extended BaseEntity |
| apps/server/src/categories/entities/category.entity.ts | Adjusted import paths and extended BaseEntity |
| apps/server/src/bookmarks/entities/bookmark.entity.ts | Adjusted import paths and extended BaseEntity |
| apps/server/src/migrations/1741515061023-BookmarkCategory.ts | Switched ID default to gen_random_uuid() |
| apps/server/src/migrations/1740000000001-InitialBookmarkTable.ts | Introduced initial bookmark table with gen_random_uuid() |
| apps/server/src/migrations/1740000000000-InitialUserTable.ts | Introduced initial user table with gen_random_uuid() |
| apps/server/src/data-source-dev.ts | Added development DataSource configuration |
| apps/server/scripts/db-setup.js | New Node script for database container and schema setup |
| apps/server/package.json | Added db:setup, updated migration scripts |
| README.md | Documented automated and manual setup options |
Comments suppressed due to low confidence (2)
apps/web/.env.example:5
- [nitpick] Consider adding brief comments or grouping in
.env.exampleto explain the purpose of the new GitHub OAuth variables, maintaining clarity for new developers.
NEXT_PUBLIC_GITHUB_CLIENT_ID=
apps/server/src/migrations/1740000000000-InitialUserTable.ts:7
- Ensure pgcrypto extension is created before using gen_random_uuid(), or revert to uuid_generate_v4() if relying on the uuid-ossp extension.
"id" uuid PRIMARY KEY DEFAULT gen_random_uuid(),
| // Create Category table | ||
| await queryRunner.query(` |
There was a problem hiding this comment.
gen_random_uuid() requires the pgcrypto extension; consider adding a migration step such as CREATE EXTENSION IF NOT EXISTS pgcrypto; before using this function to avoid errors.
| // Create Category table | |
| await queryRunner.query(` | |
| // Ensure pgcrypto extension is enabled | |
| await queryRunner.query(` | |
| CREATE EXTENSION IF NOT EXISTS pgcrypto | |
| `); | |
| // Create Category table | |
| await queryRunner.query(` |
| import { MigrationInterface, QueryRunner } from 'typeorm'; | ||
|
|
||
| export class InitialBookmarkTable1740000000001 implements MigrationInterface { | ||
| public async up(queryRunner: QueryRunner): Promise<void> { |
There was a problem hiding this comment.
Same as above: ensure the pgcrypto extension is enabled in the DB or include CREATE EXTENSION IF NOT EXISTS pgcrypto; in an earlier migration.
| public async up(queryRunner: QueryRunner): Promise<void> { | |
| public async up(queryRunner: QueryRunner): Promise<void> { | |
| await queryRunner.query('CREATE EXTENSION IF NOT EXISTS pgcrypto;'); |
| database: process.env.POSTGRES_NAME, | ||
|
|
||
| entities: [`${__dirname}/**/*.entity.ts`], | ||
| // FIXME: Should be set to false on prod |
There was a problem hiding this comment.
[nitpick] The comment is misleading (synchronize is already false). Consider clarifying that synchronize should be enabled in dev and disabled in production, or remove if no longer applicable.
| // FIXME: Should be set to false on prod | |
| // Set to true in development for automatic schema synchronization. Ensure this is set to false in production. |
| // Drop and recreate database to ensure clean state | ||
| console.log(`🔍 Checking if database '${DB_NAME}' exists...`); | ||
| try { | ||
| const result = execSync(`docker exec bookmarket-db-1 psql -U ${DB_USER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = '${DB_NAME}'"`, { |
There was a problem hiding this comment.
Hardcoding the container name bookmarket-db-1 may break if compose service names or project names differ; consider using docker-compose exec db ... or read the container name dynamically.
| "db:start": "cd ../.. && docker-compose up -d db", | ||
| "migration:generate": "npm run typeorm -- migration:generate -d src/data-source.ts -p", | ||
| "migration:run": "npm run typeorm -- migration:run -d src/data-source.ts", | ||
| "migration:run": "npm run db:setup && npm run typeorm -- migration:run -d src/data-source-dev.ts", |
There was a problem hiding this comment.
Chaining db:setup with migration:run causes the database to be dropped and recreated on every migration run, risking data loss in non-fresh environments. Separate these actions.
| "migration:run": "npm run db:setup && npm run typeorm -- migration:run -d src/data-source-dev.ts", | |
| "migration:run": "npm run typeorm -- migration:run -d src/data-source-dev.ts", |
| "db:start": "cd ../.. && docker-compose up -d db", | ||
| "migration:generate": "npm run typeorm -- migration:generate -d src/data-source.ts -p", | ||
| "migration:run": "npm run typeorm -- migration:run -d src/data-source.ts", | ||
| "migration:run": "npm run db:setup && npm run typeorm -- migration:run -d src/data-source-dev.ts", |
There was a problem hiding this comment.
The migration:revert script uses src/data-source.ts while migration:run uses src/data-source-dev.ts, leading to inconsistency. Align both to the same data-source file.
| "migration:run": "npm run db:setup && npm run typeorm -- migration:run -d src/data-source-dev.ts", | |
| "migration:run": "npm run db:setup && npm run typeorm -- migration:run -d src/data-source.ts", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (2)
apps/server/src/categories/entities/category.entity.ts (1)
7-9: Potential duplicate field declaration.The
Categoryclass extendsBaseEntitybut also declares its ownidfield. SinceBaseEntityshould define common entity fields includingid, this creates a duplicate declaration.@Entity() export class Category extends BaseEntity { - @PrimaryGeneratedColumn('uuid') - id: string; - @Column()apps/server/.env.example (1)
1-15: Add missing GitHub OAuth configuration.The environment template includes Google OAuth but is missing GitHub OAuth variables, which are mentioned in the retrieved learnings for GitHub integration support.
GOOGLE_CLIENT_ID= GOOGLE_CLIENT_SECRET= + +GITHUB_CLIENT_ID= +GITHUB_CLIENT_SECRET=
🧹 Nitpick comments (2)
setup.sh (1)
122-137: Consider adding database health check validation.While the script waits for the Docker container to report "healthy", it relies on
docker compose psoutput parsing which could be fragile.Consider adding a more robust database connection test:
# Check if database is healthy MAX_RETRIES=30 RETRY_COUNT=0 while [ $RETRY_COUNT -lt $MAX_RETRIES ]; do - if docker compose ps | grep -q "healthy"; then + if docker compose ps db | grep -q "healthy" && \ + docker exec $(docker compose ps -q db) pg_isready -U postgres; then break fi echo "Waiting for database... ($((MAX_RETRIES - RETRY_COUNT)) retries left)" sleep 2 RETRY_COUNT=$((RETRY_COUNT + 1)) doneapps/server/scripts/db-setup.js (1)
79-96: Consider making database recreation optional.The script always drops and recreates the database, which could be dangerous in development environments with important test data.
Consider adding an environment variable to control this behavior:
+ const FORCE_RECREATE = process.env.DB_FORCE_RECREATE === 'true'; + // Drop and recreate database to ensure clean state console.log(`🔍 Checking if database '${DB_NAME}' exists...`); try { const result = execSync(`docker exec bookmarket-db-1 psql -U ${DB_USER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = '${DB_NAME}'"`, { stdio: 'pipe', encoding: 'utf8' }); if (result.trim() === '1') { + if (FORCE_RECREATE) { console.log(`🗑️ Dropping existing database '${DB_NAME}' for clean setup...`); execSync(`docker exec bookmarket-db-1 dropdb -U ${DB_USER} ${DB_NAME}`, { stdio: 'pipe' }); console.log(`✅ Database '${DB_NAME}' dropped`); + } else { + console.log(`ℹ️ Database '${DB_NAME}' already exists, skipping recreation`); + return; + } } } catch (error) { // Database doesn't exist, which is fine }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.gitignore(1 hunks)README.md(1 hunks)apps/server/.env.example(1 hunks)apps/server/package.json(1 hunks)apps/server/scripts/db-setup.js(1 hunks)apps/server/src/bookmarks/entities/bookmark.entity.ts(1 hunks)apps/server/src/categories/entities/category.entity.ts(1 hunks)apps/server/src/data-source-dev.ts(1 hunks)apps/server/src/migrations/1740000000000-InitialUserTable.ts(1 hunks)apps/server/src/migrations/1740000000001-InitialBookmarkTable.ts(1 hunks)apps/server/src/migrations/1741515061023-BookmarkCategory.ts(1 hunks)apps/server/src/users/entities/user.entity.ts(1 hunks)apps/web/.env.example(1 hunks)docker-compose.yml(1 hunks)setup.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
`apps/*/.env*`: Store environment variables in appropriate .env files for each app
apps/*/.env*: Store environment variables in appropriate .env files for each app
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
apps/server/.env.exampleapps/web/.env.example
`{docker-compose.yml,docker-compose.yaml}`: Store Docker Compose configuration in the project root
{docker-compose.yml,docker-compose.yaml}: Store Docker Compose configuration in the project root
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
docker-compose.yml
`{apps,packages}/**/*.{js,jsx,ts,tsx}`: Use shared ESLint, Prettier, and TypeScript configurations across all apps and packages
{apps,packages}/**/*.{js,jsx,ts,tsx}: Use shared ESLint, Prettier, and TypeScript configurations across all apps and packages
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
apps/server/src/categories/entities/category.entity.tsapps/server/src/users/entities/user.entity.tsapps/server/src/migrations/1741515061023-BookmarkCategory.tsapps/server/src/migrations/1740000000000-InitialUserTable.tsapps/server/src/bookmarks/entities/bookmark.entity.tsapps/server/src/data-source-dev.tsapps/server/scripts/db-setup.jsapps/server/src/migrations/1740000000001-InitialBookmarkTable.ts
`apps/server/src/categories/**/*.{js,ts}`: Implement category management in the 'categories/' module of the backend
apps/server/src/categories/**/*.{js,ts}: Implement category management in the 'categories/' module of the backend
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
apps/server/src/categories/entities/category.entity.ts
`apps/server/src/users/**/*.{js,ts}`: Implement user profile management in the 'users/' module of the backend
apps/server/src/users/**/*.{js,ts}: Implement user profile management in the 'users/' module of the backend
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
apps/server/src/users/entities/user.entity.ts
`apps/server/src/migrations/**/*.ts`: Store TypeORM migrations in the 'apps/server/src/migrations/' directory
apps/server/src/migrations/**/*.ts: Store TypeORM migrations in the 'apps/server/src/migrations/' directory
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
apps/server/src/migrations/1741515061023-BookmarkCategory.tsapps/server/src/migrations/1740000000000-InitialUserTable.tsapps/server/src/migrations/1740000000001-InitialBookmarkTable.ts
`apps/server/src/bookmarks/**/*.{js,ts}`: Implement bookmark CRUD operations wit...
apps/server/src/bookmarks/**/*.{js,ts}: Implement bookmark CRUD operations with metadata fetching in the 'bookmarks/' module of the backend
Use server-side metadata and favicon extraction without external API dependencies
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
apps/server/src/bookmarks/entities/bookmark.entity.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/migrations/**/*.ts : Store TypeORM migrations in the 'apps/server/src/migrations/' directory
apps/server/.env.example (5)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/*/.env* : Store environment variables in appropriate .env files for each app
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use Google and GitHub OAuth integration for authentication in the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use JWT-based authentication with OAuth (Google/GitHub) in the 'iam/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use cookie-based JWT token management for authentication in the backend
docker-compose.yml (1)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to {docker-compose.yml,docker-compose.yaml} : Store Docker Compose configuration in the project root
apps/server/src/categories/entities/category.entity.ts (6)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/categories/**/*.{js,ts} : Implement category management in the 'categories/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Implement bookmark CRUD operations with metadata fetching in the 'bookmarks/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Use server-side metadata and favicon extraction without external API dependencies
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/web/src/app/(shared)/**/*.{js,ts,tsx} : Share bookmark collections via public URLs in the frontend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/web/src/_core/components/**/*drag*{js,ts,tsx} : Organize bookmarks with a drag-and-drop interface in the frontend
apps/server/src/users/entities/user.entity.ts (8)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/users/**/*.{js,ts} : Implement user profile management in the 'users/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Implement bookmark CRUD operations with metadata fetching in the 'bookmarks/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use bcrypt hashing for local sign-up/sign-in in the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Use server-side metadata and favicon extraction without external API dependencies
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/migrations/**/*.ts : Store TypeORM migrations in the 'apps/server/src/migrations/' directory
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use Google and GitHub OAuth integration for authentication in the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/web/src/app/(shared)/**/*.{js,ts,tsx} : Share bookmark collections via public URLs in the frontend
apps/server/src/migrations/1741515061023-BookmarkCategory.ts (3)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Implement bookmark CRUD operations with metadata fetching in the 'bookmarks/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/categories/**/*.{js,ts} : Implement category management in the 'categories/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
apps/web/.env.example (1)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/*/.env* : Store environment variables in appropriate .env files for each app
apps/server/src/migrations/1740000000000-InitialUserTable.ts (5)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/migrations/**/*.ts : Store TypeORM migrations in the 'apps/server/src/migrations/' directory
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/users/**/*.{js,ts} : Implement user profile management in the 'users/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use bcrypt hashing for local sign-up/sign-in in the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use Google and GitHub OAuth integration for authentication in the backend
apps/server/src/bookmarks/entities/bookmark.entity.ts (6)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Implement bookmark CRUD operations with metadata fetching in the 'bookmarks/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Use server-side metadata and favicon extraction without external API dependencies
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/web/src/app/(shared)/**/*.{js,ts,tsx} : Share bookmark collections via public URLs in the frontend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/web/src/_core/components/**/*drag*{js,ts,tsx} : Organize bookmarks with a drag-and-drop interface in the frontend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/categories/**/*.{js,ts} : Implement category management in the 'categories/' module of the backend
apps/server/src/data-source-dev.ts (3)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/migrations/**/*.ts : Store TypeORM migrations in the 'apps/server/src/migrations/' directory
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use bcrypt hashing for local sign-up/sign-in in the backend
apps/server/src/migrations/1740000000001-InitialBookmarkTable.ts (6)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Implement bookmark CRUD operations with metadata fetching in the 'bookmarks/' module of the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/migrations/**/*.ts : Store TypeORM migrations in the 'apps/server/src/migrations/' directory
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/web/src/app/(shared)/**/*.{js,ts,tsx} : Share bookmark collections via public URLs in the frontend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Use server-side metadata and favicon extraction without external API dependencies
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/web/src/_core/components/**/*drag*{js,ts,tsx} : Organize bookmarks with a drag-and-drop interface in the frontend
apps/server/package.json (4)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/migrations/**/*.ts : Store TypeORM migrations in the 'apps/server/src/migrations/' directory
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to apps/server/src/common/entities/base.entity.ts : Define common entity fields (id, createdAt, updatedAt) in 'apps/server/src/common/entities/base.entity.ts'
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/iam/**/*.{js,ts} : Use bcrypt hashing for local sign-up/sign-in in the backend
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.150Z
Learning: Applies to {apps,packages}/**/*.{js,jsx,ts,tsx} : Use shared ESLint, Prettier, and TypeScript configurations across all apps and packages
setup.sh (1)
Learnt from: CR
PR: eric-jy-park/bookmarket#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-01T08:38:30.151Z
Learning: Applies to apps/server/src/bookmarks/**/*.{js,ts} : Implement bookmark CRUD operations with metadata fetching in the 'bookmarks/' module of the backend
🧬 Code Graph Analysis (2)
apps/server/src/bookmarks/entities/bookmark.entity.ts (3)
apps/server/src/categories/entities/category.entity.ts (1)
Entity(6-23)apps/server/src/users/entities/user.entity.ts (1)
Entity(8-48)apps/web/src/app/_common/interfaces/bookmark.interface.ts (1)
Bookmark(3-12)
setup.sh (3)
apps/server/src/migrations/1740000000001-InitialBookmarkTable.ts (1)
up(4-18)apps/server/src/migrations/1741515061023-BookmarkCategory.ts (1)
up(4-31)apps/server/src/migrations/1740000000000-InitialUserTable.ts (1)
up(4-18)
🔇 Additional comments (22)
apps/server/src/categories/entities/category.entity.ts (1)
2-4: Good standardization of import paths.The change from absolute to relative import paths improves consistency across the codebase and aligns with the entity refactoring to use
BaseEntity..gitignore (1)
40-40: Excellent addition for database data management.Adding
db-data/to.gitignoreprevents local database files from being committed to version control, which is essential when using Docker volumes for database persistence.apps/server/src/migrations/1741515061023-BookmarkCategory.ts (2)
6-31: Well-structured migration with proper constraints.The migration correctly implements:
- UUID primary key with appropriate generation function
- Unique constraint on name/user combination
- Proper foreign key relationships with cascade behavior
- Complete rollback functionality
8-8: Docker Compose uses Postgres image without explicit version
- In apps/server/docker-compose.yml, the db.image is set to
postgres(alias forpostgres:latest), which today resolves to PostgreSQL 15+ (and thus supportsgen_random_uuid()).- No explicit version pin was found—please verify that your production and CI databases are running PostgreSQL 13 or newer to guarantee
gen_random_uuid()availability.apps/server/.env.example (1)
1-15: Well-organized environment variable template.The configuration properly groups related environment variables (database, JWT, OAuth) with clear naming conventions.
apps/server/src/users/entities/user.entity.ts (2)
2-5: Consistent import path standardization.The change to relative import paths maintains consistency with other entity files and properly imports the shared
BaseEntity.
31-42: Excellent OAuth provider integration.The entity properly implements fields for both Google and GitHub OAuth providers as indicated in the retrieved learnings, supporting the authentication requirements.
README.md (1)
271-287: LGTM! Excellent documentation improvement.The automated setup section provides clear, step-by-step instructions that will significantly improve the developer onboarding experience. The structure with checkmarks makes it easy to follow the setup process.
docker-compose.yml (1)
7-7: LGTM! Volume mapping improvement.Changing from an external directory path to a local
./db-datadirectory is a good improvement as it keeps database data within the project structure and aligns with the.gitignoreupdates.apps/server/src/data-source-dev.ts (1)
12-12: Environment variable naming is consistentWe’ve verified that
POSTGRES_NAMEis used uniformly across the project and there are no references toPOSTGRES_DB. No mismatches were detected—no changes are required unless you opt to switch to the conventionalPOSTGRES_DBnaming, in which case you’d need to update all occurrences:
- docker-compose.yml
- apps/server/scripts/db-setup.js
- apps/server/src/data-source-dev.ts
- apps/server/src/data-source.ts
- apps/server/src/app.module.ts
apps/server/package.json (2)
22-23: LGTM! Database lifecycle management scripts added.These new scripts provide a clean separation of concerns for database operations:
db:setuphandles database initialization logicdb:startmanages the Docker container startupThe path handling in
db:startcorrectly navigates to the project root fordocker-compose.yml.
25-25: Data source configuration verified
The newapps/server/src/data-source-dev.tsfile exists and exports a properly configured TypeORMDataSource(dotenv loading, correct entities and migrations paths,migrationsRun: true), and the originalapps/server/src/data-source.tsremains present for other environments. No further changes needed.
- apps/server/package.json (line 25): migration:run now points to
src/data-source-dev.ts- apps/server/src/data-source-dev.ts: verified configuration
- apps/server/src/data-source.ts: still intact for production use
apps/server/src/bookmarks/entities/bookmark.entity.ts (2)
2-4: LGTM! Improved entity imports and base class inheritance.The refactoring to use relative import paths and extend
BaseEntityprovides several benefits:
- Eliminates duplication of
createdAtandupdatedAtfields- Ensures consistent timestamp handling across all entities
- Follows established patterns from
CategoryandUserentitiesThis aligns with the retrieved learning about defining common entity fields in the
BaseEntityclass.
7-7: ✔️ BaseEntity timestamp fields verifiedBaseEntity (apps/server/src/common/entities/base.entity.ts) defines both
createdAtandupdatedAtwith the correct@Columnsettings. ExtendingBaseEntityinapps/server/src/bookmarks/entities/bookmark.entity.tspreserves the original timestamp functionality.Files verified:
- apps/server/src/common/entities/base.entity.ts
- apps/server/src/bookmarks/entities/bookmark.entity.ts
No further changes required.
apps/server/src/migrations/1740000000000-InitialUserTable.ts (3)
3-3: LGTM! Migration follows proper naming and structure conventions.The migration class name follows the TypeORM naming pattern with timestamp prefix, making it easy to track migration order and execution.
4-18: Excellent use of PostgreSQL built-in UUID generation.The migration demonstrates several best practices:
- Uses
gen_random_uuid()for secure UUID generation at the database level- Proper timestamp defaults with
now()function- All necessary fields for multi-provider authentication (Google, GitHub)
- Correct column types and constraints
The schema aligns well with the
Userentity that extendsBaseEntity.
20-22: Proper migration reversibility implemented.The
downmethod correctly drops the table, ensuring the migration can be safely reverted if needed.setup.sh (4)
6-6: LGTM! Proper error handling with fail-fast approach.Using
set -eensures the script exits immediately on any command failure, preventing partial setup states that could be confusing for developers.
11-33: Excellent user experience with colored output functions.The colored output functions with emoji icons make the script output clear and user-friendly, helping developers quickly identify the status of each setup step.
40-57: Robust prerequisite checking with automatic pnpm installation.The Node.js version validation and automatic pnpm installation provide a smooth setup experience. The specific pnpm version (8.9.2) matches the
packageManagerfield inpackage.json, ensuring consistency.
142-146: LGTM! Proper migration execution with directory management.The script correctly changes to the server directory before running migrations and returns to the original directory, ensuring the migration command executes in the correct context.
apps/server/scripts/db-setup.js (1)
9-13: LGTM! Proper environment variable handling with defaults.The script correctly loads environment variables from
.envand provides sensible defaults for host and port, making it flexible across different deployment scenarios.
| export class User extends BaseEntity { | ||
| @PrimaryGeneratedColumn('uuid') | ||
| id: string; |
There was a problem hiding this comment.
Duplicate field declaration needs correction.
Similar to the Category entity, the User class extends BaseEntity but declares its own id field, creating a duplicate.
@Entity()
export class User extends BaseEntity {
- @PrimaryGeneratedColumn('uuid')
- id: string;
-
@Column()📝 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 class User extends BaseEntity { | |
| @PrimaryGeneratedColumn('uuid') | |
| id: string; | |
| @Entity() | |
| export class User extends BaseEntity { | |
| @Column() | |
| // ...other columns... | |
| } |
🤖 Prompt for AI Agents
In apps/server/src/users/entities/user.entity.ts around lines 9 to 11, the User
class extends BaseEntity but redeclares the id field, causing duplication.
Remove the id field declaration from the User class to avoid duplicate primary
key definitions, relying on the inherited id from BaseEntity instead.
| test: | ||
| [ | ||
| 'CMD', | ||
| 'pg_isready', | ||
| '-U', | ||
| '${POSTGRES_USER}', | ||
| '-d', | ||
| '${POSTGRES_NAME}', | ||
| ] |
There was a problem hiding this comment.
Fix environment variable name in healthcheck.
The healthcheck uses ${POSTGRES_NAME} but the standard PostgreSQL environment variable is POSTGRES_DB. This mismatch could cause the healthcheck to fail.
Apply this diff to fix the environment variable:
- '${POSTGRES_NAME}',
+ '${POSTGRES_DB}',📝 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.
| test: | |
| [ | |
| 'CMD', | |
| 'pg_isready', | |
| '-U', | |
| '${POSTGRES_USER}', | |
| '-d', | |
| '${POSTGRES_NAME}', | |
| ] | |
| test: | |
| [ | |
| 'CMD', | |
| 'pg_isready', | |
| '-U', | |
| '${POSTGRES_USER}', | |
| '-d', | |
| '${POSTGRES_DB}', | |
| ] |
🤖 Prompt for AI Agents
In docker-compose.yml lines 11 to 19, the healthcheck uses the environment
variable ${POSTGRES_NAME} which is incorrect. Replace ${POSTGRES_NAME} with the
correct standard PostgreSQL environment variable ${POSTGRES_DB} to ensure the
healthcheck works properly.
| synchronize: false, | ||
| migrations: [`${__dirname}/migrations/**/*.ts`], | ||
| migrationsTableName: 'migrations', | ||
| migrationsRun: true, |
There was a problem hiding this comment.
Remove automatic migration execution for safety.
Setting migrationsRun: true automatically runs migrations on application startup, which can be dangerous and unpredictable. Migrations should be run explicitly through scripts or deployment processes.
Apply this diff to disable automatic migration execution:
- migrationsRun: true,
+ migrationsRun: false,📝 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.
| migrationsRun: true, | |
| migrationsRun: false, |
🤖 Prompt for AI Agents
In apps/server/src/data-source-dev.ts at line 19, the configuration sets
migrationsRun to true, which causes automatic execution of migrations on
startup. To fix this, change migrationsRun to false or remove this setting
entirely to disable automatic migration execution, ensuring migrations are run
explicitly via scripts or deployment processes.
| NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY= | ||
| CLERK_SECRET_KEY= | ||
| NEXT_PUBLIC_GITHUB_CLIENT_ID= | ||
| GITHUB_CLIENT_SECRET= |
There was a problem hiding this comment.
Move GitHub client secret to server environment.
GITHUB_CLIENT_SECRET should be in the server's environment file (apps/server/.env.example) rather than the web app's environment, as client secrets should never be exposed to the frontend.
Remove this line from the web app's .env.example and ensure it's properly configured in the server's environment.
-GITHUB_CLIENT_SECRET=📝 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.
| GITHUB_CLIENT_SECRET= |
🤖 Prompt for AI Agents
In apps/web/.env.example at line 6, remove the GITHUB_CLIENT_SECRET entry to
prevent exposing sensitive client secrets in the frontend environment. Then, add
this variable to the server environment file at apps/server/.env.example to
securely manage the secret on the backend.
| NEXT_PUBLIC_BASE_URL= | ||
|
|
||
| # This file will be committed to version control, so make sure not to have any | ||
| # secrets in it. If you are cloning this repo, create a copy of this file named | ||
| # ".env" and populate it with your secrets. | ||
| NEXT_PUBLIC_GOOGLE_CLIENT_ID= | ||
|
|
||
| # When adding additional environment variables, the schema in "/src/env.js" | ||
| # should be updated accordingly. | ||
|
|
||
| # Drizzle | ||
| DATABASE_URL= | ||
|
|
||
| # Example: | ||
| # SERVERVAR="foo" | ||
| # NEXT_PUBLIC_CLIENTVAR="bar" | ||
|
|
||
| NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY= | ||
| CLERK_SECRET_KEY= | ||
| NEXT_PUBLIC_GITHUB_CLIENT_ID= | ||
| GITHUB_CLIENT_SECRET= | ||
| NEXT_PUBLIC_GITHUB_REDIRECT_URI= | ||
| NEXT_PUBLIC_DOMAIN= |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing environment variables mentioned in README.
The README documentation mentions additional environment variables that are missing from this example file:
NEXT_PUBLIC_API_URL(mentioned in manual setup section)NEXT_PUBLIC_SENTRY_DSN(mentioned as optional)
Consider adding these variables for completeness:
NEXT_PUBLIC_BASE_URL=
+NEXT_PUBLIC_API_URL=
NEXT_PUBLIC_GOOGLE_CLIENT_ID=
NEXT_PUBLIC_GITHUB_CLIENT_ID=
-GITHUB_CLIENT_SECRET=
NEXT_PUBLIC_GITHUB_REDIRECT_URI=
NEXT_PUBLIC_DOMAIN=
+
+# Optional: Error Monitoring
+NEXT_PUBLIC_SENTRY_DSN=📝 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.
| NEXT_PUBLIC_BASE_URL= | |
| # This file will be committed to version control, so make sure not to have any | |
| # secrets in it. If you are cloning this repo, create a copy of this file named | |
| # ".env" and populate it with your secrets. | |
| NEXT_PUBLIC_GOOGLE_CLIENT_ID= | |
| # When adding additional environment variables, the schema in "/src/env.js" | |
| # should be updated accordingly. | |
| # Drizzle | |
| DATABASE_URL= | |
| # Example: | |
| # SERVERVAR="foo" | |
| # NEXT_PUBLIC_CLIENTVAR="bar" | |
| NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY= | |
| CLERK_SECRET_KEY= | |
| NEXT_PUBLIC_GITHUB_CLIENT_ID= | |
| GITHUB_CLIENT_SECRET= | |
| NEXT_PUBLIC_GITHUB_REDIRECT_URI= | |
| NEXT_PUBLIC_DOMAIN= | |
| NEXT_PUBLIC_BASE_URL= | |
| NEXT_PUBLIC_API_URL= | |
| NEXT_PUBLIC_GOOGLE_CLIENT_ID= | |
| NEXT_PUBLIC_GITHUB_CLIENT_ID= | |
| NEXT_PUBLIC_GITHUB_REDIRECT_URI= | |
| NEXT_PUBLIC_DOMAIN= | |
| # Optional: Error Monitoring | |
| NEXT_PUBLIC_SENTRY_DSN= |
🤖 Prompt for AI Agents
In apps/web/.env.example lines 1 to 8, the environment variables
NEXT_PUBLIC_API_URL and NEXT_PUBLIC_SENTRY_DSN are missing but referenced in the
README. Add these two variables to the example file with empty values to ensure
completeness and alignment with the documentation.
| "description" character varying NOT NULL, | ||
| "faviconUrl" character varying NOT NULL, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider making description and faviconUrl nullable.
Both description and faviconUrl are marked as NOT NULL, but these fields might not always be available when scraping metadata from URLs. This could cause bookmark creation to fail unnecessarily.
Apply this diff to make these fields optional:
- "description" character varying NOT NULL,
- "faviconUrl" character varying NOT NULL,
+ "description" character varying,
+ "faviconUrl" character varying,🤖 Prompt for AI Agents
In apps/server/src/migrations/1740000000001-InitialBookmarkTable.ts around lines
10 to 11, the columns "description" and "faviconUrl" are currently set as NOT
NULL, which can cause failures when these metadata fields are missing. Modify
the migration to make both "description" and "faviconUrl" columns nullable by
removing the NOT NULL constraint, allowing bookmark creation to succeed even if
these fields are absent.
| CREATE TABLE "bookmark" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid(), | ||
| "url" character varying NOT NULL, | ||
| "title" character varying NOT NULL, | ||
| "description" character varying NOT NULL, | ||
| "faviconUrl" character varying NOT NULL, | ||
| "userId" uuid NOT NULL, | ||
| "createdAt" TIMESTAMP NOT NULL DEFAULT now(), | ||
| "updatedAt" TIMESTAMP NOT NULL DEFAULT now(), | ||
| CONSTRAINT "FK_bookmark_user" FOREIGN KEY ("userId") REFERENCES "user"("id") ON DELETE CASCADE | ||
| ) | ||
| `); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add database indexes and field length constraints.
The table lacks indexes for commonly queried fields and has no length constraints on VARCHAR fields, which could impact performance and data integrity.
Consider adding these improvements:
CREATE TABLE "bookmark" (
"id" uuid PRIMARY KEY DEFAULT gen_random_uuid(),
- "url" character varying NOT NULL,
- "title" character varying NOT NULL,
- "description" character varying NOT NULL,
- "faviconUrl" character varying NOT NULL,
+ "url" character varying(2048) NOT NULL,
+ "title" character varying(500) NOT NULL,
+ "description" character varying(1000),
+ "faviconUrl" character varying(500),
"userId" uuid NOT NULL,
"createdAt" TIMESTAMP NOT NULL DEFAULT now(),
"updatedAt" TIMESTAMP NOT NULL DEFAULT now(),
CONSTRAINT "FK_bookmark_user" FOREIGN KEY ("userId") REFERENCES "user"("id") ON DELETE CASCADE
)
`);
+
+ await queryRunner.query(`
+ CREATE INDEX "IDX_bookmark_userId" ON "bookmark" ("userId");
+ CREATE INDEX "IDX_bookmark_createdAt" ON "bookmark" ("createdAt");
+ `);🤖 Prompt for AI Agents
In apps/server/src/migrations/1740000000001-InitialBookmarkTable.ts around lines
6 to 17, the bookmark table definition lacks indexes on frequently queried
columns and does not specify length constraints for VARCHAR fields. To fix this,
add appropriate length limits to all VARCHAR columns to enforce data integrity
and create indexes on columns like "userId" and "url" to improve query
performance. Modify the table creation SQL to include these constraints and
indexes accordingly.
| function ensureDockerContainer() { | ||
| try { | ||
| // Check if any PostgreSQL container is running on port 5432 | ||
| const result = execSync(`docker ps --format "table {{.Names}}\t{{.Ports}}" | grep :5432`, { | ||
| stdio: 'pipe', | ||
| encoding: 'utf8' | ||
| }); | ||
|
|
||
| if (result.trim()) { | ||
| console.log('✅ PostgreSQL container is already running on port 5432'); | ||
| return; | ||
| } | ||
| } catch (error) { | ||
| // No container running on port 5432 | ||
| } | ||
|
|
||
| console.log('🐳 Starting PostgreSQL container...'); | ||
| try { | ||
| execSync('cd ../.. && docker-compose up -d db', { stdio: 'inherit' }); | ||
| console.log('✅ PostgreSQL container started'); | ||
| // Give it a moment to initialize | ||
| require('child_process').execSync('sleep 3'); | ||
| } catch (error) { | ||
| // If it fails due to port conflict, the container might already be running | ||
| if (error.message.includes('port is already allocated')) { | ||
| console.log('⚠️ Port 5432 is already in use - checking if PostgreSQL is accessible...'); | ||
| return; // Continue with connection check | ||
| } | ||
| console.error('❌ Failed to start PostgreSQL container:', error.message); | ||
| console.log('💡 Make sure Docker is running and docker-compose.yml exists'); | ||
| process.exit(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve Docker container detection and naming robustness.
The container detection logic is sound, but the hardcoded path navigation and error handling could be more robust.
Consider these improvements:
function ensureDockerContainer() {
try {
- const result = execSync(`docker ps --format "table {{.Names}}\t{{.Ports}}" | grep :5432`, {
+ const result = execSync(`docker ps --format "{{.Names}}" --filter "publish=5432"`, {
stdio: 'pipe',
encoding: 'utf8'
});
if (result.trim()) {
console.log('✅ PostgreSQL container is already running on port 5432');
return;
}
} catch (error) {
// No container running on port 5432
}
console.log('🐳 Starting PostgreSQL container...');
try {
- execSync('cd ../.. && docker-compose up -d db', { stdio: 'inherit' });
+ const { execSync } = require('child_process');
+ execSync('docker-compose up -d db', { stdio: 'inherit', cwd: '../..' });
console.log('✅ PostgreSQL container started');
- require('child_process').execSync('sleep 3');
+ await sleep(3000);
} catch (error) {
// ... rest of error handling
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/server/scripts/db-setup.js around lines 17 to 49, improve the Docker
container detection by avoiding hardcoded relative paths like 'cd ../..' and
instead use absolute paths or Node.js path utilities to locate the
docker-compose.yml file. Enhance error handling by catching specific errors more
precisely and providing clearer messages. Also, consider verifying container
names explicitly rather than relying solely on port checks to increase
robustness.
| const result = execSync(`docker exec bookmarket-db-1 psql -U ${DB_USER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = '${DB_NAME}'"`, { | ||
| stdio: 'pipe', | ||
| encoding: 'utf8' | ||
| }); | ||
|
|
||
| if (result.trim() === '1') { | ||
| console.log(`🗑️ Dropping existing database '${DB_NAME}' for clean setup...`); | ||
| execSync(`docker exec bookmarket-db-1 dropdb -U ${DB_USER} ${DB_NAME}`, { | ||
| stdio: 'pipe' | ||
| }); | ||
| console.log(`✅ Database '${DB_NAME}' dropped`); | ||
| } | ||
| } catch (error) { | ||
| // Database doesn't exist, which is fine | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address hardcoded container name dependency.
The script uses a hardcoded container name bookmarket-db-1 which may not be consistent across different environments or Docker Compose versions.
Consider making the container name dynamic:
+ // Get the actual container name/ID for the database service
+ const containerName = execSync(`docker-compose ps -q db`, {
+ stdio: 'pipe',
+ encoding: 'utf8',
+ cwd: '../..'
+ }).trim();
+
+ if (!containerName) {
+ throw new Error('Database container not found');
+ }
+
- const result = execSync(`docker exec bookmarket-db-1 psql -U ${DB_USER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = '${DB_NAME}'"`, {
+ const result = execSync(`docker exec ${containerName} psql -U ${DB_USER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = '${DB_NAME}'"`, {Apply similar changes to lines 89 and 100 where the container name is used.
📝 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.
| const result = execSync(`docker exec bookmarket-db-1 psql -U ${DB_USER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = '${DB_NAME}'"`, { | |
| stdio: 'pipe', | |
| encoding: 'utf8' | |
| }); | |
| if (result.trim() === '1') { | |
| console.log(`🗑️ Dropping existing database '${DB_NAME}' for clean setup...`); | |
| execSync(`docker exec bookmarket-db-1 dropdb -U ${DB_USER} ${DB_NAME}`, { | |
| stdio: 'pipe' | |
| }); | |
| console.log(`✅ Database '${DB_NAME}' dropped`); | |
| } | |
| } catch (error) { | |
| // Database doesn't exist, which is fine | |
| } | |
| // Get the actual container name/ID for the database service | |
| const containerName = execSync(`docker-compose ps -q db`, { | |
| stdio: 'pipe', | |
| encoding: 'utf8', | |
| cwd: '../..' | |
| }).trim(); | |
| if (!containerName) { | |
| throw new Error('Database container not found'); | |
| } | |
| const result = execSync(`docker exec ${containerName} psql -U ${DB_USER} -d postgres -tc "SELECT 1 FROM pg_database WHERE datname = '${DB_NAME}'"`, { | |
| stdio: 'pipe', | |
| encoding: 'utf8' | |
| }); | |
| if (result.trim() === '1') { | |
| console.log(`🗑️ Dropping existing database '${DB_NAME}' for clean setup...`); | |
| execSync(`docker exec ${containerName} dropdb -U ${DB_USER} ${DB_NAME}`, { | |
| stdio: 'pipe' | |
| }); | |
| console.log(`✅ Database '${DB_NAME}' dropped`); | |
| } | |
| } catch (error) { | |
| // Database doesn't exist, which is fine | |
| } |
🤖 Prompt for AI Agents
In apps/server/scripts/db-setup.js around lines 82 to 96, the container name
'bookmarket-db-1' is hardcoded in multiple execSync commands, which can cause
issues in different environments. Refactor the code to use a dynamic variable
for the container name, such as reading it from an environment variable or a
configuration file. Replace all occurrences of the hardcoded container name on
lines 82, 89, and 100 with this variable to ensure flexibility and environment
compatibility.
ericjypark
force-pushed
the
main
branch
2 times, most recently
from
e9aff06 to
21a836f
Compare
Summary by CodeRabbit
New Features
Documentation
Chores
.gitignoreto exclude local database data.Refactor
Bug Fixes