Skip to content

fix projectQfRound entity#2227

Open
CarlosQ96 wants to merge 2 commits intostagingfrom
hotfix_entity
Open

fix projectQfRound entity#2227
CarlosQ96 wants to merge 2 commits intostagingfrom
hotfix_entity

Conversation

@CarlosQ96
Copy link
Collaborator

@CarlosQ96 CarlosQ96 commented Oct 8, 2025

Summary by CodeRabbit

  • Refactor
    • Project–round records now use a single generated primary identifier, with project and round stored as indexed fields; join table naming made explicit.
  • Bug Fixes
    • Enforced uniqueness of each project–round pairing to prevent duplicate associations.
  • Chores
    • Migration steps now skip execution in test environments to improve test stability.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Walkthrough

Replaces the composite primary key on ProjectQfRound (projectId, qfRoundId) with a single auto-generated primary key id and a composite unique constraint; converts projectId and qfRoundId to regular indexed columns; marks the entity with synchronize: false; specifies explicit join table name on Project.qfRounds; migration adds environment guards to skip execution in test environments for both up and down.

Changes

Cohort / File(s) Summary
Entity: ProjectQfRound
src/entities/projectQfRound.ts
Replace incremental non-primary id column with @PrimaryGeneratedColumn(); change projectId and qfRoundId from @PrimaryColumn() to @Column() with @Index(); add class-level @Unique(['projectId','qfRoundId']); update imports (add PrimaryGeneratedColumn, Unique, remove PrimaryColumn); set @Entity(..., { synchronize: false }).
Entity: Project (join table)
src/entities/project.ts
Update @JoinTable() on qfRounds relation to @JoinTable({ name: 'project_qf_rounds_qf_round' }) to explicitly name the join table.
Migration: test guards
migration/1779182511002-FixProjectQfRoundsPrimaryKey.ts
Add early-return guard clauses in up and down to skip DDL when NODE_ENV or ENVIRONMENT equals test, preventing migration/rollback during tests.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer
  participant ORM as TypeORM Entity
  participant DB as Database
  rect #F0FBFF
    Note left of ORM: Entity mapping changes\n(new PK id, unique (projectId,qfRoundId), synchronize:false)
  end
  Dev->>ORM: update decorators (PrimaryGeneratedColumn, Unique, Column/Index, Entity options)
  ORM->>DB: subsequent migration/schema change will add `id`, remove composite PK, add unique constraint
Loading
sequenceDiagram
  participant Runner as Migration Runner
  participant Env as Runtime Env
  participant Migration as MigrationScript
  participant DB as Database
  Env->>Runner: start migration
  Runner->>Migration: call up()
  Migration->>Env: check NODE_ENV / ENVIRONMENT
  alt environment == 'test'
    Migration-->>Runner: return early (skip DDL)
  else
    Migration->>DB: execute DDL (alter table, indexes, constraints)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • kkatusic

Poem

"I hopped through code beneath the moonlit log,
Swapped twin-pebbles for one shiny cog,
Unique pairs tucked in neat little rows,
Guards keep test runs gentle as they doze,
Indices crunch carrots — schema snug and smart." 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly identifies that the pull request fixes the projectQfRound entity and accurately reflects the core change without unnecessary detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hotfix_entity

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ba9d65 and 4c5de91.

📒 Files selected for processing (2)
  • src/entities/project.ts (1 hunks)
  • src/entities/projectQfRound.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test
🔇 Additional comments (5)
src/entities/project.ts (1)

238-238: LGTM! Explicit join table naming improves maintainability.

Explicitly specifying the join table name ensures consistency with the ProjectQfRound entity definition and prevents potential naming conflicts or confusion.

src/entities/projectQfRound.ts (4)

3-3: LGTM! Import updates match the entity refactoring.

The addition of PrimaryGeneratedColumn and Unique imports aligns with the new entity structure using an auto-generated primary key and composite unique constraint.

Also applies to: 11-11


16-16: Good practice: synchronize: false prevents unintended schema changes.

Setting synchronize: false ensures that TypeORM will not automatically modify this table's schema, requiring explicit migrations. This is essential for production environments and when making breaking schema changes like this PR.


25-32: LGTM! Column and index definitions support the new schema.

The conversion of projectId and qfRoundId from @PrimaryColumn() to @Column() with @Index() is correct. The indexes will maintain query performance for lookups by these columns, which is essential given they're no longer part of the primary key.


18-22: Migration FixProjectQfRoundsPrimaryKey correctly preserves existing rows, drops the old table, renames the temp table and primary-key constraint to introduce the auto-generated id; composite-key queries remain supported and AdminJS delete now uses id.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant