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: describe findByPk's option bag #15115

Closed
wants to merge 610 commits into from

Conversation

evanrittenhouse
Copy link
Member

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

Todos

Closes sequelize/website#202. Not sure if we want to add more documentation, but I didn't want to duplicate work when FindOptions already exists.

Duplicate of #15111

papandreou and others added 30 commits May 14, 2022 12:53
This adds supports for sending JS BigInt values to databases. For retrieving SQL BigInts as JS BigInts, see sequelize#14296
Co-authored-by: Renovate Bot <bot@renovateapp.com>
)

Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: SequelizeJS <mail@sequelizejs.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
* refactor: migrate associations to ts (wip)

* refactor: migrate has-one to ts

* refactor: migrate belongs-to association to TS

* refactor: fix typing errors in has-one, belongs-to

* refactor: migrate has-many to ts

* refactor: migrate has-many to ts

* refactor: finish migrating has-many.ts

* refactor: migrate association/index.ts

* refactor: continue association migration to ts

* build: accept @typeparam, @internal typedoc tags

* feat: improve typings of hasOne.set

* fix: make HasOneGetAssociationMixin's output nullable

* fix: improve typing of BelongsTo set method

* feat: update hasMany

* refactor: complete typing associations

* fix: fix failing tests

* refactor: migrate sequelize export to esm

* fix: fix associations based on unit tests

* fix: fix associations based on intergration tests

* test: add test to prevent duplicate associations aliases

* fix: fix Sequelize typings

* test: unify groupedLimit m-n test with the integration one

* feat: forbid instanciating Association/ManyAssociation

* docs: revert accidental comment removal

* fix: normalize includes before merging them

* fix: replace query generator trying to create a new association

* refactor: revert changes to reduce noise

* test: fix mssql query generator test

* feat: forbid users from instantiating association Classes

* fix: dedupe BelongsToMany association

* fix: use isSameModel to compare models

* feat: improve error message for 'this association was already created'

* fix: continue work to fix tests

* feat: fix associations based on unit tests

* refactor: migrate mixin.js to typescript

* refactor: rename 'mixin.ts' -> 'define-association-methods.ts'

* fix: fix accidental usage of _.eq instead of _.isEqual

* test: fix m:n tests

* refactor: move association definitions methods inside their classes

* refactor: continue refactoring associations

* feat: improve association deduplication

* fix: remove accidental export

* test: support 'cause' property when checking error message

* fix: normalize belongs to many opts, improve error msg

* feat: print error cause on systems without support

* test: fix belongs to many tests

* fix: continue fixing based on tests

* fix: normalize the right timestamps option

* feat: add Model.{getInitialModel,withoutScope}, rename schema+(un)?scope

* fix: add deprecation IDs

* feat: improve 'invalid scope' error message

* fix: new batch of fixes

* fix: make association.get return Map for multiple

* docs: remove todos

* fix: fix failing tests

* fix: rollback test update

* test: fix mariadb/mysql test

* test: fix test

* test: fix mysql8 test

* feat(types): strongly type association mixins

* test: migrate association.test to ts

* test: migrate belongs-to test

* feat(types): strongly type belongsToMany FKs

* test: test deep through options version

* test: test sourceKey/targetKey typing

* test: migrate has-one unit test to typescript

* test: migrate has-many unit tests to ts

* test: migrate belongs-to-many unit test to ts

* test: fix test

* test: fix support.js

* test: test two associations sharing the same pair

* docs: update tsdoc of associations

* feat: refactor associations to support 'inverse' option

* feat: remove onUpdate, onDelete, inverse.{onUpdate,onDelete}

They already exist as foreignKey.{onDelete,onUpdate}, otherkey.{onDelete,onUpdate}

* fix: make CASCADE default onDelete for non-null FKs

* refactor: normalize otherKey option

* feat: rename constraints, foreignKeyConstraint to foreignKeyConstraints

* fix: fix wrong scope option being passed to belongsTo

* feat: support 'inverse' in belongs-to

* feat: force all self-associations to be named

* fix: simplify the name of foreignKeys on through models

* fix: special case through models in order by

* test: fix tests

* test: fix broken test

* fix: fix sqlite treating allowNull undefined as false

* test: improve sql minifier

* feat: use through model name in groupedLimit through include

* refactor: remove unnecessary changes

* test: mute type error due to missing typing

* refactor: update comments

* refactor: update comment

* test: update unit tests

Co-authored-by: Fauzan "Evan Edrok" 0x <fncolon@pm.me>
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Zoé <zoe@ephys.dev>
)

Co-authored-by: Renovate Bot <bot@renovateapp.com>
Move my email to Gmail for security reports and accept specific mail only.
…equelize#14560)

Explanation:
  - User.sync(), which references the schema, was executed before the schema exists
  - add an `await` before the schema is created to enforce sequencing/synchronocity
Subtle style change:
  - remove word 'even' from test description

Addresses only PostgreSQL-portion of issue sequelize#14161
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: Zoé <zoe@ephys.dev>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: SequelizeJS <mail@sequelizejs.com>
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
WikiRik and others added 10 commits October 1, 2022 19:12
* get-options documentation improved

The documentation of instance.get was lacking information when it simply returns instance.dataValues, or when it returns a copy of instance.dataValues. Searching a reason for this phenomenon, I also found the great 'clone'option, which was also lacking a description.
Added both things.

* close===true

* clone===true (the second...)

* line moved up

* get-options as an interface

* Lint mistakes solved

(hopefully solved)

* lint mistake fixed 2

Co-authored-by: X <fncolon@pm.me>
Co-authored-by: Zoé <zoe@ephys.dev>
Co-authored-by: SequelizeJS <mail@sequelizejs.com>
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…15079)

* docs: correct instructions for running databases in Docker

* docs: correct, add versioning info for test databases

Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
* @param {Model<any, any>} [options.instance=null] A sequelize instance used to build the return instance.
* @param {number|Literal} [options.limit=null] Limits how many items will be returned by the operation.
* @param {boolean|LOCK|{level: LOCK; of: ModelStatic<Model<any, any>>}} [options.lock=null] Lock the selected rows. Possible options are transaction.LOCK.UPDATE and transaction.LOCK.SHARE. Postgres also supports transaction.LOCK.KEY_SHARE, transaction.LOCK.NO_KEY_UPDATE and specific model locks with joins
* @param {boolean} [options.logging=false] A function that gets executed while running the query to log the sql.
Copy link
Member Author

Choose a reason for hiding this comment

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

This line gave me syntax errors when trying to copy the callback signature from v7, so I left it out.

@evanrittenhouse evanrittenhouse requested review from ephys and a team October 11, 2022 14:48
@ephys ephys changed the title Findbypk jsdoc docs: describe findByPk's option bag Oct 11, 2022
@ephys
Copy link
Member

ephys commented Oct 11, 2022

Did you mean to target the v6 branch? This will unfortunately not resolve sequelize/website#202 since it's a v6 only issue 😅

@evanrittenhouse
Copy link
Member Author

evanrittenhouse commented Oct 11, 2022

Yeah I did 😬 I had some issues with the git remote on my local. Should I just do a new PR?

E: @ephys changed the base branch to v6.

@evanrittenhouse evanrittenhouse changed the base branch from main to v6 October 11, 2022 18:22
@fzn0x
Copy link
Member

fzn0x commented Oct 11, 2022

This is a lot of conflicts when we target it directly in the same PR, can you replicate this PR to a new PR that targetting v6 branch?

@fzn0x
Copy link
Member

fzn0x commented Oct 11, 2022

Since v6 branch is different source of truth

@ephys
Copy link
Member

ephys commented Oct 12, 2022

You can open a new PR if you want but you can also repurpose this one :)

Here is how:

  • copy your changes in a temporary file somewhere (or you will lose them)
  • git checkout v6
  • delete this branch git branch -D findbypk_jsdoc
  • recreate it from v6 git checkout -B findbypk_jsdoc
  • paste the changes you copied
  • push & replace the branch on the remote git push origin findbypk_jsdoc --force

@evanrittenhouse
Copy link
Member Author

evanrittenhouse commented Oct 14, 2022

@ephys just getting to this, did we remove the commit linting on v6 commits? Seems like I'm getting 107 (!) errors when trying to commit on the new branch as you mentioned. Though these are actually in the code (i.e. unexpected trailing comma, etc.), so I'm not sure if v6 follows a different linting strategy...?

@WikiRik
Copy link
Member

WikiRik commented Oct 17, 2022

@ephys just getting to this, did we remove the commit linting on v6 commits? Seems like I'm getting 107 (!) errors when trying to commit on the new branch as you mentioned. Though these are actually in the code (i.e. unexpected trailing comma, etc.), so I'm not sure if v6 follows a different linting strategy...?

v6 does indeed follow a different config for the linter

@WikiRik WikiRik added the v6 label Nov 6, 2022
@WikiRik WikiRik marked this pull request as draft November 16, 2022 12:20
@ephys
Copy link
Member

ephys commented Nov 28, 2022

This PR has been inactive for a while (says the person with 4 PRs in draft since January). Is there still interest in pursuing it, is it blocked, or should we close it?

I'll remove review requests until the branch issue has been resolved, but feel free to re-request a review when ready

@ephys ephys removed request for a team and ephys November 28, 2022 17:31
@WikiRik
Copy link
Member

WikiRik commented Jan 3, 2023

I'm closing this, we can always review a recreation on this PR that works for v6

@WikiRik WikiRik closed this Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there a way of querying only specific attributes using findByPK method ?