Skip to content

Comments

Apps#9401

Merged
evereq merged 3 commits intostage-appsfrom
stage
Feb 1, 2026
Merged

Apps#9401
evereq merged 3 commits intostage-appsfrom
stage

Conversation

@evereq
Copy link
Member

@evereq evereq commented Feb 1, 2026

PR

Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.



Summary by cubic

Make relation decorators apply only the active ORM (TypeORM or MikroORM) using getORMType, fixing multi-ORM build/runtime conflicts. Update Nx project configs to forward params in dependsOn for consistent env/config during builds.

  • Bug Fixes
    • ManyToMany, ManyToOne, OneToMany, and OneToOne decorators now conditionally apply the correct ORM decorator.
    • Prevents double registration and conflicting relation metadata.
    • Nx: forward params in ui-config and ui-core dependsOn to ensure proper configuration propagation during builds.

Written for commit ae44709. Summary will update on new commits.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 1, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch stage

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.

@evereq evereq merged commit cb6996a into stage-apps Feb 1, 2026
14 of 17 checks passed
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 1, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
20.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 1, 2026

Greptile Overview

Greptile Summary

This PR updates the multi-ORM relation decorators (MultiORMManyToMany/ManyToOne/OneToMany/OneToOne) to only register relation metadata for the active ORM selected by getORMType(), avoiding conflicts from double-registration (TypeORM + MikroORM). It also updates Nx dependsOn entries in ui-config and ui-core to forward params so build configuration/environment is propagated consistently.

Key issue: the new conditional branches in MultiORMManyToOne and MultiORMOneToOne still pass the original inverseSideOrOptions argument to the TypeORM decorator even when that argument was actually the options object (the overload form without inverse side). That can cause TypeORM relation metadata to be registered incorrectly for calls that omit the inverse side.

Confidence Score: 3/5

  • This PR is mostly safe, but has two high-impact decorator argument bugs that can break TypeORM relation metadata when inverse-side is omitted.
  • The overall change (gating decorators by active ORM) is reasonable and localized, and the Nx config changes are low risk. However, both MultiORMManyToOne and MultiORMOneToOne have an overload-handling bug where the TypeORM branch can receive the options object as the inverse-side argument, which can cause runtime metadata issues in TypeORM deployments.
  • packages/core/src/lib/core/decorators/entity/relations/many-to-one.decorator.ts, packages/core/src/lib/core/decorators/entity/relations/one-to-one.decorator.ts

Important Files Changed

Filename Overview
packages/core/src/lib/core/decorators/entity/relations/many-to-many.decorator.ts Conditionally applies TypeORM vs MikroORM ManyToMany decorator based on getORMType to avoid dual metadata registration.
packages/core/src/lib/core/decorators/entity/relations/many-to-one.decorator.ts Adds getORMType gating for ManyToOne decorators; potential bug: TypeORM branch still uses inverseSideOrOptions (which may be options object) instead of normalized inverseSideProperty.
packages/core/src/lib/core/decorators/entity/relations/one-to-many.decorator.ts Adds getORMType gating so only the active ORM’s OneToMany decorator is applied.
packages/core/src/lib/core/decorators/entity/relations/one-to-one.decorator.ts Adds getORMType gating for OneToOne; potential bug: TypeORM branch uses inverseSideOrOptions (may be options object) rather than normalized inverseSideProperty.
packages/ui-config/project.json Nx build dependsOn now forwards params to setup-env, improving configuration propagation.
packages/ui-core/project.json Nx ui-core build dependsOn config now forwards params; mostly formatting changes plus param forwarding.

Sequence Diagram

sequenceDiagram
  autonumber
  participant E as Entity class (decorated property)
  participant D as MultiORM* relation decorator
  participant O as getORMType()
  participant T as TypeORM decorator
  participant M as MikroORM decorator

  E->>D: @MultiORM*(target, propertyKey)
  D->>O: Determine active ORM
  alt ormType == TypeORM
    D->>T: Apply TypeORM relation metadata
  else ormType == MikroORM
    D->>M: Apply MikroORM relation metadata
  end
  Note over D: Avoids dual registration
  Note over D: Bug risk if normalized args not used

Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +73 to +79
// Apply TypeORM decorator when using TypeORM
if (ormType === MultiORMEnum.TypeORM) {
TypeOrmManyToOne(
typeFunctionOrTarget as TypeORMTarget<T>,
inverseSideOrOptions as TypeORMInverseSide<T>,
options as TypeORMRelationOptions
)(target, propertyKey);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P0] TypeORM ManyToOne decorator can receive the options object as the inverse-side argument.

In MultiORMManyToOne, when inverseSideOrOptions is an object it is treated as options and inverseSideProperty remains unset, but the TypeORM branch still calls TypeOrmManyToOne(..., inverseSideOrOptions as TypeORMInverseSide<T>, ...). Under TypeORM, calls like MultiORMManyToOne(User, { nullable: true }) will pass that options object as the inverse side and likely break relation metadata registration.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/lib/core/decorators/entity/relations/many-to-one.decorator.ts
Line: 73:79

Comment:
[P0] TypeORM ManyToOne decorator can receive the *options object* as the inverse-side argument.

In `MultiORMManyToOne`, when `inverseSideOrOptions` is an object it is treated as `options` and `inverseSideProperty` remains unset, but the TypeORM branch still calls `TypeOrmManyToOne(..., inverseSideOrOptions as TypeORMInverseSide<T>, ...)`. Under TypeORM, calls like `MultiORMManyToOne(User, { nullable: true })` will pass that options object as the inverse side and likely break relation metadata registration.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +73 to +79
// Apply TypeORM decorator when using TypeORM
if (ormType === MultiORMEnum.TypeORM) {
TypeOrmOneToOne(
typeFunctionOrTarget as TypeORMTarget<T>,
inverseSideOrOptions as TypeORMInverseSide<T>,
options as TypeORMRelationOptions
)(target, propertyKey);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P0] TypeORM OneToOne decorator can receive the options object as the inverse-side argument.

MultiORMOneToOne normalizes params such that if inverseSideOrOptions is an object it becomes options and inverseSideProperty remains unset, but the TypeORM branch still passes inverseSideOrOptions as the inverse-side arg. Under TypeORM, calls like MultiORMOneToOne(Profile, { cascade: true }) will end up passing the options object as the inverse side, which can break relation metadata.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/lib/core/decorators/entity/relations/one-to-one.decorator.ts
Line: 73:79

Comment:
[P0] TypeORM OneToOne decorator can receive the *options object* as the inverse-side argument.

`MultiORMOneToOne` normalizes params such that if `inverseSideOrOptions` is an object it becomes `options` and `inverseSideProperty` remains unset, but the TypeORM branch still passes `inverseSideOrOptions` as the inverse-side arg. Under TypeORM, calls like `MultiORMOneToOne(Profile, { cascade: true })` will end up passing the options object as the inverse side, which can break relation metadata.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

@augmentcode
Copy link

augmentcode bot commented Feb 1, 2026

🤖 Augment PR Summary

Summary: This PR updates the multi-ORM relation decorators to apply only the active ORM’s decorators at runtime, and adjusts Nx project dependencies to forward parameters.

Changes:

  • Updated MultiORMManyToMany, MultiORMManyToOne, MultiORMOneToMany, and MultiORMOneToOne to detect the active ORM via getORMType() and conditionally apply either the TypeORM or MikroORM decorator.
  • Reduced the chance of both ORMs registering metadata for the same property when only one ORM is intended to be used.
  • Updated Nx dependsOn entries (ui-config/ui-core) to include params: "forward" so command parameters/configurations propagate to dependent targets.

Technical Notes: ORM selection is driven by process.env.DB_ORM (defaulting to TypeORM), so decorator behavior depends on that value at entity-import time.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 1 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

})
)(target, propertyKey);
// Determine which ORM is in use
const ormType = getORMType();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getORMType() is evaluated when decorators run (class definition time), so if DB_ORM isn’t set before entity modules are imported it will default to TypeORM and skip registering MikroORM relation metadata. That can lead to missing relations in MikroORM runtime (or vice‑versa) depending on import order/config initialization.

Other Locations
  • packages/core/src/lib/core/decorators/entity/relations/many-to-one.decorator.ts:71
  • packages/core/src/lib/core/decorators/entity/relations/one-to-many.decorator.ts:60
  • packages/core/src/lib/core/decorators/entity/relations/one-to-one.decorator.ts:71

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

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