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

[BUG] FirestoreQuery.select with array as parameter results in 400. #349

Closed
tzrot-jed opened this issue Jul 20, 2023 · 2 comments
Closed
Labels
bug Something isn't working

Comments

@tzrot-jed
Copy link
Contributor

tzrot-jed commented Jul 20, 2023

Describe the bug
Passing an array to FirestoreQuery.select doesn't format the query request to Firestore properly and results in a 400

To Reproduce
Assuming a Firestore collection stage with fields name and rating:
Running:

var query: FirestoreQuery = FirestoreQuery.new().select(["name", "rating"]).from("stage", false)
var task: FirestoreTask = Firebase.Firestore.query(query)
var result: Array = yield(task, "result_query")

Results in a 400 response by Firestore

Expected behavior

var query: FirestoreQuery = FirestoreQuery.new().select(["name", "rating"]).from("stage", false)
var task: FirestoreTask = Firebase.Firestore.query(query)
var result: Array = yield(task, "result_query")

should request Firestore with {fields:[{fieldPath:name}, {fieldPath:rating}]} instead of {fields:[name, rating]}

Screenshots
Not applicable

Environment:

  • OS: Windows 11
  • Godot 3.5.2

Additional context
I will be submitting a PR with a fix

@tzrot-jed tzrot-jed added the bug Something isn't working label Jul 20, 2023
tzrot-jed added a commit to tzrot-jed/GodotFirebase that referenced this issue Jul 20, 2023
String values seem to be passed by value rather than reference even though arrays are passed by reference.
Added a local array and re-populated it to fix
@WolfgangSenff
Copy link
Collaborator

That is super weird. I will have to look into this closer. Very interesting. Thanks for submitting it! Will probably merge it soon, just haven't had time lately to validate it or anything.

@WolfgangSenff
Copy link
Collaborator

Closing this per my comment on #350 - 4.x should be fixed permanently now, and I'm going to attempt the same refactor for 3.x, which should fix this as well I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants