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 information on extra attributes on join table in N:M relati… #494

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

Conversation

garretaserra
Copy link

Added documentation for new functionality in sequelize/sequelize#16101

@ephys
Copy link
Member

ephys commented Jun 17, 2023

sequelize/sequelize#16101 targets the v7 branch but this documentation is written for v6

@garretaserra
Copy link
Author

Moved documentation to v7

@ephys
Copy link
Member

ephys commented Jun 26, 2023

Thanks :)

Unfortunately your branch was outdated and the changes were made against the old version of the v7 docs, which were recently completely rewritten. Could you look into fixing the merge conflict?

I assume this could be documented as part of this section https://sequelize.org/docs/v7/associations/belongs-to-many/#association-setter-setx

@garretaserra
Copy link
Author

I'm not sure where to put it exactly as it affects both the set and the add mixins.

@ephys
Copy link
Member

ephys commented Jul 1, 2023

That's a good point. I think it could go as a sub-section of this instead then: https://sequelize.org/docs/v7/associations/belongs-to-many/#customizing-the-junction-table

With mentions of set & add that links to the sections about set and add

About the content: While what you wrote still works in Sequelize 7, the new documentation uses the typescript + decorator approach first (with optional Tabs if we want to provide a typescript-free example)

@garretaserra
Copy link
Author

It took a while because I had to learn how to correctly use the typescript definitions.

Also while working on the sscce for the documentation example I found I bug on the code for this update which I will be looking into.

Finally I think that its probably for the best to merge the documentation from Customizing the Junction Table nad what I did in the subsection Associations with extra attributes on through table by basing from the same example of person and toot.
What do you think?

On another note: The example of Person and Toot might not be the best. As a non native speaker I had barely heard the expression Toot, and a quick google search gives the following meaning:

toot
noun

1 a short, sharp sound made by a horn, trumpet, or similar instrument.
2 INFORMAL a snort of a drug, especially cocaine.
3 INFORMAL•NORTH AMERICAN a spell of drinking and lively enjoyment; a spree.

verb

1 sound (a horn or similar) with a short, sharp sound.
2 INFORMAL snort (cocaine).

@garretaserra
Copy link
Author

Let me know if there is something more to do in this PR

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.

2 participants