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.

Sign up for GitHub

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

Jump to bottom

fix: adds unique index on note and user id of teams table #286

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: adds unique index on note and user id of teams table #286

wants to merge 1 commit into from

Conversation

Tushar504
Copy link

@Tushar504 Tushar504 commented Jan 25, 2025 •
edited
Loading

@Tushar504 Tushar504 linked an issue Jan 25, 2025 that may be closed by this pull request
fix: Fix join route user duplication #285
Open
@Tushar504 Tushar504 requested review from e11sy and neSpecc January 25, 2025 14:10
e11sy
e11sy requested changes Jan 25, 2025
View reviewed changes
src/repository/storage/postgres/orm/sequelize/teams.ts
// Create a unique index on noteId and userId
{
unique: true,
fields: ['noteId', 'userId'],
Copy link
Contributor

@e11sy e11sy Jan 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

we use underscore column naming in db, either write with underscore here, or add underscored: true to the model arguments

src/repository/storage/postgres/orm/sequelize/teams.ts
@@ -103,6 +103,13 @@ export default class TeamsSequelizeStorage {
tableName: this.tableName,
sequelize: this.database,
timestamps: false,
indexes: [
Copy link
Contributor

Choose a reason for hiding this comment

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

make sure, that we have this unique index in migration, if we don't have this index, add it in a new migration

neSpecc
neSpecc requested changes Jan 25, 2025
View reviewed changes
Copy link
Member

@neSpecc neSpecc left a comment

Choose a reason for hiding this comment

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

Have you been able to reproduce the problem?
Please provide a detailed explanation of the issue, its root cause, and your solution in the PR description

@Tushar504
Copy link
Author

Have you been able to reproduce the problem? Please provide a detailed explanation of the issue, its root cause, and your solution in the PR description

  • I am not able reproduce the problem
  • if we add the unique key constraints then it won't allow duplicate entry into db.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@e11sy e11sy e11sy requested changes

@neSpecc neSpecc neSpecc requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fix: Fix join route user duplication
3 participants
@Tushar504 @neSpecc @e11sy