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

fix: duplicatePromotion endpoint issue #6806

Merged
merged 1 commit into from
Feb 22, 2023

Conversation

vannguyenn
Copy link
Contributor

  • Use insertedId instead of insertedCount for success check
  • The response from insertOne doesn't include the insertedCount property

Signed-off-by: Chloe <pinkcloudvnn@gmail.com>
@changeset-bot
Copy link

changeset-bot bot commented Feb 21, 2023

⚠️ No Changeset found

Latest commit: 9aa30d9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Member

@vanpho93 vanpho93 left a comment

Choose a reason for hiding this comment

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

We are using many insertedCount in other places, this issue could relate to mongodb upgrade so I think I will work on it.

@vannguyenn vannguyenn closed this Feb 21, 2023
@zenweasel
Copy link
Collaborator

@vannguyenn @vanpho93 What was the outcome of this? If we think we have uncovered another issue can we create a ticket to track the work? I think Chloe's fix is still legit for this particular problem?

@vanpho93
Copy link
Member

@zenweasel
Copy link
Collaborator

zenweasel commented Feb 22, 2023

@vanpho93 Providing backwards compatibility is good, but shouldn't any new code use the updated API i.e. use insertedId?

@vanpho93
Copy link
Member

@vanpho93 Providing backwards compatibility is good, but shouldn't any new code use the updated API i.e. use insertedId?

Yes, you're right, I think this PR is good to go.

@zenweasel zenweasel reopened this Feb 22, 2023
Copy link
Collaborator

@zenweasel zenweasel left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @vannguyenn

@zenweasel zenweasel merged commit 0c41448 into feat/promotions Feb 22, 2023
@zenweasel zenweasel deleted the fix/duplicate-endpoint branch February 22, 2023 00:31
vanpho93 pushed a commit that referenced this pull request Apr 24, 2023
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.

None yet

3 participants