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

Use current models transaction client #37

Open
evoactivity opened this issue Feb 27, 2024 · 3 comments
Open

Use current models transaction client #37

evoactivity opened this issue Feb 27, 2024 · 3 comments

Comments

@evoactivity
Copy link

I am running into problems where .assignRole is creating a new transaction and using up my transaction pool when used in a loop (think importing users from a CSV).

Patching the package so it uses the current models transaction fixes my issue.

public async assignRole(
  this: HasRolesModelContract,
  ...roles: Array<RoleContract | string>
): Promise<void> {
  const roleModels = await Promise.all(
    roles.map(async (r) =>
      typeof r === 'string' ? await Role.firstOrCreate({ name: r }, { name: r }, { client: this.modelTrx }) : r
    )
  )

  await this.related('roles').attach(roleModels.map((r) => r.id))
}

I guess this should be done for syncRoles and revokeRole as well.

@arthur-er
Copy link
Member

We should patch it to do creation of strings in one go using fetchOrCreateMany then merging the array.

PR welcome

@mr-feek
Copy link

mr-feek commented Mar 4, 2024

We should patch it to do creation of strings in one go using fetchOrCreateMany then merging the array.

FWIW, I view that as two separate issues.

Issue 1: It's currently not possible to assign roles inside of an ongoing transaction.

Issue 2: Assigning multiple roles results in querying the roles table multiple times.

@arthur-er
Copy link
Member

Agreed

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

No branches or pull requests

3 participants