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

feat: add missing createRoles association method for HasMany relationship #17163

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sakupan102
Copy link

Pull Request Checklist

  • Have you added new tests to prevent regressions?
  • If a documentation update is necessary, have you opened a PR to the documentation repository?
  • Did you update the typescript typings accordingly (if applicable)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Does the name of your PR follow our conventions?

Description of Changes

  • Added new method createMultiple to create multiple associated instances.
    Example use case:
class Article extends Model:
  declare: id: number;
  @HasMany(() => Label, 'articleId')
  declare createLabels: HasManyCreateAssociationMixin<Label>
class Label extends Model:
  declare id: number;
  declare articleId: number

const article = await Article.create();
const labels = await article.createLabels([{id: 100}, {id: 200}])
// created two instances

this PR closes #11372

@sakupan102 sakupan102 requested a review from a team as a code owner March 13, 2024 17:17
Copy link
Member

@ephys ephys left a comment

Choose a reason for hiding this comment

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

Thank you for your PR. This would be a nice addition


Don't forget to add the following:

  • Typing test
  • Integration test

options:
| HasManyCreateAssociationMixinOptions<T>
| HasManyCreateAssociationMixinOptions<T>['fields'] = {},
): Promise<T[]> {
Copy link
Member

Choose a reason for hiding this comment

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

instead of copying the code, create should call createMultiple

@@ -368,6 +368,8 @@ export class BelongsToManyAssociation<
addMultiple: `add${plural}`,
add: `add${singular}`,
create: `create${singular}`,
createMultiple: `create${plural}`,
// TODO: add createMultiple association method for BelongsTo relationship
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// TODO: add createMultiple association method for BelongsTo relationship
// TODO: add createMultiple association method for BelongsToMany relationship

Ideally this should be done in the same PR, as it now declares it as being available

Copy link
Member

Choose a reason for hiding this comment

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

Don't forget to add the HasManyCreateAssociationsMixin type

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.

Adding missing createRoles association method for HasMany relationship
2 participants