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

[🐛] Firestore orderBy treats FieldPath differently than where #6602

Closed
3 of 5 tasks
Nestoro opened this issue Oct 12, 2022 · 6 comments · Fixed by #6814
Closed
3 of 5 tasks

[🐛] Firestore orderBy treats FieldPath differently than where #6602

Nestoro opened this issue Oct 12, 2022 · 6 comments · Fixed by #6814
Assignees
Labels
help: needs-triage Issue needs additional investigation/triaging. type: bug New bug report Workflow: Needs Review Pending feedback or review from a maintainer.

Comments

@Nestoro
Copy link

Nestoro commented Oct 12, 2022

Issue

If an inequality is used for a where query with a FirebaseFirestoreTypes.FieldPath field name, you trigger an exception 'Invalid query'. Seemingly because orderBy handles FirebaseFirestoreTypes.FieldPath field names differently

                    firestore().collection('table')
                        .where(
                            new firestore.FieldPath('address.zipCode'),
                            '>=',
                            options.partialMatch.value,
                        )
                        .where(
                            new firestore.FieldPath('address.zipCode'),
                            '<=',
                            options.partialMatch.value + '\uf8ff',
                        )
                        .orderBy(new firestore.FieldPath('address.zipCode'));

produces:

Invalid query. You have an inequality where filter (whereLessThan(), whereGreaterThan(), etc.) on field '`address.zipCode`' and so you must also have '`address.zipCode`' as your first orderBy() field, but your first orderBy() is currently on field 'address.zipCode' instead.

(it is with and without backticks.)

On web, if you escape both, a query like this works.

System:
    OS: macOS 12.4
    CPU: (8) x64 Intel(R) Core(TM) i7-7820HQ CPU @ 2.90GHz
    Memory: 232.03 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.10.0 - /usr/local/bin/node
    Yarn: 1.22.17 - /usr/local/bin/yarn
    npm: 8.19.2 - /usr/local/bin/npm
    Watchman: 2022.09.26.00 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.11.3 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.4, iOS 15.5, macOS 12.3, tvOS 15.4, watchOS 8.5
    Android SDK: Not Found
  IDEs:
    Android Studio: 2021.3 AI-213.7172.25.2113.9014738
    Xcode: 13.4.1/13F100 - /usr/bin/xcodebuild
  Languages:
    Java: javac 19 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.0.0 => 18.0.0 
    react-native: 0.69.1 => 0.69.1 
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found
  • Platform that you're experiencing the issue on:
    • iOS
    • Android
    • iOS but have not tested behavior on Android
    • Android but have not tested behavior on iOS
    • Both
  • react-native-firebase version you're using that has this issue:
    • 14.11.1
  • Firebase module(s) you're using that has the issue:
    • firestore
  • Are you using TypeScript?
    • Y & 4.5.4


@Nestoro Nestoro added help: needs-triage Issue needs additional investigation/triaging. type: bug New bug report labels Oct 12, 2022
@mikehardy
Copy link
Collaborator

Apologies this has sat so long - I saw it when it first came in but had no immediate answer so kept silent, and it's been pretty busy for me in this repo and a couple others so I haven't had a chance to do any triage.

With apologies for my response here being not just a lack-of-answer but requesting effort, the quickest way to move this forward is probably to add an e2e test that reproduces the issue in the firebase e2e set: https://github.com/invertase/react-native-firebase/blob/main/packages/firestore/e2e/issues.e2e.js

We put issue-specific cases in there, and it is usually not too hard to get the test rig up and running - it's how all development is done in the repo (https://github.com/invertase/react-native-firebase/blob/main/tests/README.md) and the test app there (along with all the e2e tests) are exactly what qualifies all the builds for release / how we probe native APIs while making new features etc

@Ehesp
Copy link
Member

Ehesp commented Nov 2, 2022

Yeah looks as though on Android at least, we just get the string value:

https://github.com/invertase/react-native-firebase/blob/main/packages/firestore/android/src/reactnative/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreQuery.java#L132

 String fieldPath = order.get("fieldPath");

whereas on a where query, we apply a fieldvalue:

FieldPath fieldPath = FieldPath.of(Objects.requireNonNull(segmentArray));

So we'll need to modify the underlying JS code to send the fieldpath segment list and apply similar changes here.

@Ehesp
Copy link
Member

Ehesp commented Nov 2, 2022

Interestingly we already send a FieldPath over to native, so looks like we'll need to add a test to handle this properly.

@mikehardy mikehardy changed the title [🐛] Bug Report Title - Firestore orderBy treats FieldPath differently than where [🐛] Firestore orderBy treats FieldPath differently than where Nov 2, 2022
@Salakar
Copy link
Member

Salakar commented Dec 5, 2022

Hello 👋, to help manage issues we automatically close stale issues.\n\nThis issue has been automatically marked as stale because it has not had activity for quite some time.\nHas this issue been fixed, or does it still require attention?\n\n> This issue will be closed in 15 days if no further activity occurs.\n\nThank you for your contributions.

@Salakar Salakar added the Type: Stale Issue has become stale - automatically added by Stale bot label Dec 5, 2022
@github-actions github-actions bot removed the Type: Stale Issue has become stale - automatically added by Stale bot label Dec 5, 2022
@mikehardy mikehardy added the Workflow: Needs Review Pending feedback or review from a maintainer. label Dec 5, 2022
@github-actions
Copy link

github-actions bot commented Jan 2, 2023

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

@github-actions github-actions bot added the Type: Stale Issue has become stale - automatically added by Stale bot label Jan 2, 2023
@Nestoro
Copy link
Author

Nestoro commented Jan 4, 2023

To my knowledge, it still needs attention

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help: needs-triage Issue needs additional investigation/triaging. type: bug New bug report Workflow: Needs Review Pending feedback or review from a maintainer.
Projects
None yet
6 participants