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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct MongoDB ID conversion in hasManyThrough relations #10469

Merged
merged 1 commit into from Apr 24, 2024

Conversation

enes-sahin
Copy link
Contributor

@enes-sahin enes-sahin commented Apr 10, 2024

This pull request addresses an issue where MongoDB ID was causing problems in hasManyThrough relations. The solution implemented attempts conversion to a string type using the toString() function for both related and entity IDs. If the toString() function does not exist, the ID is used as it is. This ensures consistency in ID types, allowing the intersection function to work correctly.

Fixes #10259

Checklist

  • DCO (Developer Certificate of Origin) signed in all commits
  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

馃憠 Check out how to submit a PR 馃憟

@dhmlau
Copy link
Member

dhmlau commented Apr 10, 2024

@enes-sahin, thanks for the PR. Your changes LGTM. Could you please add some test cases? Thanks.

@coveralls
Copy link

coveralls commented Apr 10, 2024

Pull Request Test Coverage Report for Build 8692020676

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 3 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 55.332%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/repository/src/relations/has-many/has-many-through.inclusion-resolver.ts 0 3 0.0%
Totals Coverage Status
Change from base Build 8646446166: -0.04%
Covered Lines: 9563
Relevant Lines: 12458

馃挍 - Coveralls

Signed-off-by: Enes Sahin <25066964+enes-sahin@users.noreply.github.com>
@enes-sahin enes-sahin force-pushed the fix/has-many-through-resolver branch from fb5b0c8 to 7a51d56 Compare April 15, 2024 15:39
@enes-sahin
Copy link
Contributor Author

@enes-sahin, thanks for the PR. Your changes LGTM. Could you please add some test cases? Thanks.

Thank you for the feedback! I've added the test cases.

@dhmlau dhmlau merged commit d61a21f into loopbackio:master Apr 24, 2024
13 checks passed
@dhmlau
Copy link
Member

dhmlau commented Apr 24, 2024

@enes-sahin, thanks for your contribution. Your PR has been merged! 馃帀

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.

@loopback/repository 6.1.4 broke hasManyThrough relations
3 participants