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

docs: Added upsertKeys property to BulkCreateOptions interface #15505

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

Conversation

lpizzinidev
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 Change

Added documentation for the upsertKeys option in the BulkCreateOptions interface.
fix #15226

@lpizzinidev lpizzinidev changed the title docs(model): Added upsertKeys property to BulkCreateOptions interface docs: Added upsertKeys property to BulkCreateOptions interface Dec 27, 2022
@WikiRik WikiRik requested a review from ephys December 30, 2022 10:33
src/model.d.ts Outdated
*/
include?: AllowArray<Includeable>;
upsertKeys?: string[];
Copy link
Member

Choose a reason for hiding this comment

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

Can you undo the changes to the order so only upsertKeys is added?

Copy link
Author

Choose a reason for hiding this comment

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

👍 sure!

Copy link
Author

Choose a reason for hiding this comment

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

yarn lint changed the orders of the ModelHooks imports but the pipeline is not passing the lint checks that pass locally.
Am I missing some steps in the PR submission process?

Copy link
Member

Choose a reason for hiding this comment

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

yarn lint should not do this (the CI is running yarn lint-no-fix with the same rules), but you are able to undo these changes manually

/**
* Primary key names used as conflict target columns during insertion.
*/
upsertKeys?: string[];
Copy link
Member

Choose a reason for hiding this comment

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

Looking at bulkCreate it seems that it sets this by itself;

sequelize/src/model.js

Lines 2251 to 2253 in f6045f8

options.upsertKeys = upsertKeys.length > 0
? upsertKeys
: Object.values(model.primaryKeys).map(x => x.field);

Can you add a test using this option that shows otherwise?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @WikiRik
I'll work on this 💪

Anyway, I've encountered problems after cloning the repo and running yarn install.
I keep getting this error message:
Screenshot 2023-02-11 at 14 44 13

I searched online but I didn't find anything helpful.
Do you know how I could solve the problem? Should I open a new issue?

Thanks

Copy link
Member

Choose a reason for hiding this comment

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

Can you delete the node_modules folder and try to install it again? If that does not work, you can try opening an issue for the ibm_db project. It's not a sequelize issue

@WikiRik WikiRik removed the request for review from ephys January 3, 2023 12:52
@@ -1,6 +1,6 @@
import type { ModelHooks } from '@sequelize/core/_non-semver-use-at-your-own-risk_/model-hooks.js';
Copy link
Member

Choose a reason for hiding this comment

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

You have lint issues

@@ -1196,6 +1196,11 @@ export interface BulkCreateOptions<TAttributes = any> extends Logging, Transacti
* Return all columns or only the specified columns for the affected rows (only for postgres)
*/
returning?: boolean | Array<keyof TAttributes | Literal | Col>;

/**
* Primary key names used as conflict target columns during insertion.
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
* Primary key names used as conflict target columns during insertion.
* Column names used as conflict target columns during insertion.
* Defaults to the primary keys of the model.

@lpizzinidev lpizzinidev requested a review from a team as a code owner March 18, 2024 23:04
@lpizzinidev lpizzinidev requested review from ephys and WikiRik and removed request for a team March 18, 2024 23:04
@sequelize-bot sequelize-bot bot added the conflicted This PR has merge conflicts and will not be present in the list of PRs to review label Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicted This PR has merge conflicts and will not be present in the list of PRs to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing upsertKeys property from bulkCreate query interface documentation
3 participants