Skip to content

Conversation

@jsrodas-pdpaola
Copy link
Contributor

Summary

Packages:

"mongodb/laravel-mongodb": "^4.7",
"rebing/graphql-laravel": "^9.2",

This pull request addresses an issue encountered when using $query->select($fields->getSelect()). Without this fix, the result of the query is incorrect, as it fails to properly handle the selection of fields when working with the latest version of laravel-mongodb.

The issue arises due to the change in the namespace from Jenssegers\Mongodb\ to MongoDB\Laravel\ in the latest version 4 of laravel-mongodb. This change affects the way fields are selected and retrieved in queries, leading to discrepancies in the output.

Example Output Before the Fix

When executing the following code:

$query->select($fields->getSelect());

//  The output of dd($fields->getSelect()) is as follows:

array:15 [
  0 => "users._id"
  1 => "users.active"
  2 => "users.email"
  3 => "users.name"
  4 => "users.password"
  5 => "users.created_at"
  6 => "users.updated_at"
]

Example Output After the Fix

With the fix applied (also support MongoDB\Laravel\Eloquent\Model), the output is corrected to:

array:16 [ 
  0 => "_id"
  1 => "active"
  2 => "email"
  3 => "name"
  4 => "password"
  5 => "created_at"
  6 => "updated_at"
]

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Misc. change (internal, infrastructure, maintenance, etc.)

Checklist:

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

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

After digging a bit around, my understanding is that:

Therefore I think: it doesn't make sense to have a check for both namespaces and it should just be changed.

WDYT?

@jsrodas-pdpaola jsrodas-pdpaola force-pushed the fix-namespace-mongodb-packages branch 2 times, most recently from 86c74c9 to 8b87a50 Compare August 23, 2024 06:53
@jsrodas-pdpaola jsrodas-pdpaola force-pushed the fix-namespace-mongodb-packages branch from 8b87a50 to e0615ed Compare August 23, 2024 06:54
@jsrodas-pdpaola
Copy link
Contributor Author

Hi @mfn , I totally agree with what you said, I made the change only leaving MongoDB\Laravel\, thanks for your comments.

@jsrodas-pdpaola jsrodas-pdpaola requested a review from mfn August 23, 2024 07:03
@mfn mfn self-assigned this Aug 23, 2024
@mfn mfn merged commit 561a0c7 into rebing:master Aug 23, 2024
@mfn
Copy link
Collaborator

mfn commented Aug 23, 2024

Thanks for the PR!

@jsrodas-pdpaola
Copy link
Contributor Author

@mfn Thank you very much for your promptness! 👍

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