Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An In-Class @Scope Decorator #17307

Open
4 of 8 tasks
klondikemarlen opened this issue May 4, 2024 · 1 comment
Open
4 of 8 tasks

An In-Class @Scope Decorator #17307

klondikemarlen opened this issue May 4, 2024 · 1 comment
Labels
pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@klondikemarlen
Copy link

klondikemarlen commented May 4, 2024

Issue Creation Checklist

  • I understand that my issue will be automatically closed if I don't fill in the requested information
  • I have read the contribution guidelines

Feature Description

Describe the feature you'd like to see implemented

I'd like to have an in-class @scope decorator like the wonderful @Attribute decorator, because that's such a nice declarative, inline pattern.

I'd like to see something like this

export class User extends Model<InferAttributes<User>, InferCreationAttributes<User>> {
  @Attribute(DataTypes.BOOLEAN)
  @NotNull
  @Default(true)
  declare isActive: CreationOptional<boolean>

  @Scope(() => {
    return {
      where: {
        isActive: true,
      },
    }
  })
  static active: () => ModelStatic<User>
}

const activeUsers = await User.active().findAll()
// or
const activeUsers2 = await User.withScope('active').findAll()

This a of a style thing rather than a new feature, since the feature already exists in a couple different ways. Given the short example its not really obvious why I wouldn't want scopes at the top of the class definition.

Say I have this model (with a bunch more scopes expected in the near future)

import {
  CreationOptional,
  DataTypes,
  InferAttributes,
  InferCreationAttributes,
  Model,
  ModelStatic,
  NonAttribute,
  Op,
  WhereOptions,
  sql,
} from "@sequelize/core"
import {
  Attribute,
  AutoIncrement,
  Default,
  HasMany,
  Index,
  NotNull,
  PrimaryKey,
} from "@sequelize/core/decorators-legacy"

import { Scope } from "@/db/utils/scope-decorator"
import Notification from "@/models/notification"
import Position from "@/models/position"
import Preference from "@/models/preference"
import Role from "@/models/role"
import Team from "@/models/team"
import WorkflowPlayer from "@/models/workflow-player"

export class User extends Model<InferAttributes<User>, InferCreationAttributes<User>> {
  @Attribute(DataTypes.INTEGER)
  @PrimaryKey
  @AutoIncrement
  declare id: CreationOptional<number>

  @Attribute(DataTypes.STRING(100))
  @NotNull
  @Index({ unique: true })
  declare email: string

  @Attribute(DataTypes.STRING(100))
  @NotNull
  @Index({ unique: true })
  declare authSubject: string

  @Attribute(DataTypes.STRING(100))
  declare firstName: string | null

  @Attribute(DataTypes.STRING(100))
  declare lastName: string | null

  @Attribute(DataTypes.STRING(200))
  declare displayName: string | null

  @Attribute(DataTypes.STRING(100))
  declare title: string | null

  @Attribute(DataTypes.STRING(100))
  declare department: string | null

  @Attribute(DataTypes.STRING(100))
  declare division: string | null

  @Attribute(DataTypes.STRING(100))
  declare branch: string | null

  @Attribute(DataTypes.STRING(100))
  declare unit: string | null

  @Attribute(DataTypes.BOOLEAN)
  @NotNull
  @Default(true)
  declare isActive: CreationOptional<boolean>

  @Attribute(DataTypes.DATE(0))
  @NotNull
  @Default(sql.fn("getutcdate"))
  declare createdAt: CreationOptional<Date>

  @Attribute(DataTypes.DATE(0))
  @NotNull
  @Default(sql.fn("getutcdate"))
  declare updatedAt: CreationOptional<Date>

  @Attribute(DataTypes.DATE(0))
  declare deletedAt: Date | null

  // Associations
  @HasMany(() => WorkflowPlayer, {
    foreignKey: "addedByUserId",
    inverse: "addedByUser",
  })
  declare workflowPlayersAddedBy?: NonAttribute<WorkflowPlayer[]>

  @HasMany(() => WorkflowPlayer, {
    foreignKey: "userId",
    inverse: "user",
  })
  declare workflowPlayers?: NonAttribute<WorkflowPlayer[]>

  // TODO: define as Sequelize associations once other models are defined
  declare roles?: NonAttribute<Role[]>
  declare preferences?: NonAttribute<Preference[]>
  declare notifications?: NonAttribute<Notification[]>
  declare teams?: NonAttribute<Team[]>
  declare positions?: NonAttribute<Position[]>

  @Scope((terms: string[]) => {
    if (terms.length === 0) {
      return {}
    }
 
    const where: {
      [Op.and]?: WhereOptions<User>[]
    } = {}

    const whereConditions: WhereOptions<User>[] = terms.map((term: string) => {
      const termPattern = `%${term}%`
      return sql`LOWER(first_name) like ${termPattern} OR LOWER(last_name) like ${termPattern} OR LOWER(display_name) like ${termPattern}`
    })

    where[Op.and] = whereConditions

    return {
      where,
    }
  })
  static search: (terms: string[]) => ModelStatic<User>
}

export default User

If I add a bunch more scopes, they don't cover up the main content, i.e. attribute defintions.

The implementation could look something like this:

import { Attributes, FindOptions, Model, ModelStatic } from "@sequelize/core"
import { registerModelOptions } from "@sequelize/core/_non-semver-use-at-your-own-risk_/decorators/shared/model.js"

function Scope<M extends Model, ScopeArgs extends []>(
  scopeDefinition: (...scopeArgs: ScopeArgs[]) => FindOptions<Attributes<M>>
) {
  return function <M extends Model>(target: ModelStatic<M>, propertyKey: string): void {
    // Ensure the propertyKey is a string since it will be used as the scope name
    if (typeof propertyKey !== "string") {
      throw new Error(
        `@Scope can only be applied to methods and the key must be a string, got ${typeof propertyKey}`
      )
    }

    // Register the scope as model options
    registerModelOptions(target, {
      scopes: { [propertyKey]: scopeDefinition.bind(target) },
    })
    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    ;(target as any)[propertyKey] = function (...args: ScopeArgs[]) {
      return this.withScope({
        method: [propertyKey, ...args],
      })
    }
  }
}

export { Scope }

Describe why you would like this feature to be added to Sequelize

It seems like a nature extension of the "@Attribute" decorator pattern.

Is this feature dialect-specific?

  • No. This feature is relevant to Sequelize as a whole.
  • Yes. This feature only applies to the following dialect(s):

Would you be willing to resolve this issue by submitting a Pull Request?

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I will need guidance.
  • No, I don't have the time, but my company or I are supporting Sequelize through donations on OpenCollective.
  • No, I don't have the time, and I understand that I will need to wait until someone from the community or maintainers is interested in implementing my feature.

Indicate your interest in the addition of this feature by adding the 👍 reaction. Comments such as "+1" will be removed.

@klondikemarlen klondikemarlen added pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet type: feature For issues and PRs. For new features. Never breaking changes. labels May 4, 2024
@klondikemarlen
Copy link
Author

klondikemarlen commented May 4, 2024

The alternative to this pattern is the same as establishing associations used to be in Sequelize 6.
e.g.

import {
  CreationOptional,
  DataTypes,
  InferAttributes,
  InferCreationAttributes,
  Model,
  NonAttribute,
  Op,
  WhereOptions,
  sql,
} from "@sequelize/core"
import {
  Attribute,
  AutoIncrement,
  Default,
  HasMany,
  Index,
  NotNull,
  PrimaryKey,
} from "@sequelize/core/decorators-legacy"

export class User extends Model<InferAttributes<User>, InferCreationAttributes<User>> {
  @Attribute(DataTypes.INTEGER)
  @PrimaryKey
  @AutoIncrement
  declare id: CreationOptional<number>

  @Attribute(DataTypes.STRING(100))
  @NotNull
  @Index({ unique: true })
  declare email: string

  @Attribute(DataTypes.STRING(100))
  @NotNull
  @Index({ unique: true })
  declare authSubject: string

  @Attribute(DataTypes.STRING(100))
  declare firstName: string | null

  @Attribute(DataTypes.STRING(100))
  declare lastName: string | null

  @Attribute(DataTypes.STRING(200))
  declare displayName: string | null

  static establishScopes(): void {
    this.addScope("search", (terms: string[]) => {
      if (terms.length === 0) {
        return {}
      }

      // TODO: rebuild as successive scope calls once
      // https://github.com/sequelize/sequelize/issues/17304 is fixed
      // (we would no longer need the and operator in the where clause)
      const where: {
        [Op.and]?: WhereOptions<User>[]
      } = {}

      const whereConditions: WhereOptions<User>[] = terms.map((term: string) => {
        const termPattern = `%${term}%`
        return sql`LOWER(first_name) like ${termPattern} OR LOWER(last_name) like ${termPattern} OR LOWER(display_name) like ${termPattern}`
      })

      where[Op.and] = whereConditions

      return {
        where,
      }
    })
  }
}

export default User

See https://github.com/icefoganalytics/wrap/commit/cac7d62ac971ece06f39f1008e9f6df556b9e861

So its not really needed, just bit of syntactic sugar at relatively low cost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-approval Bug reports that have not been verified yet, or feature requests that have not been accepted yet type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests

1 participant