fix: Remove DocumentReference from cursor (#1882)

* remove DocumentReference from cursor * remove the warning --------- Co-authored-by: Mark Duckworth <[email protected]>
googleapis · Oct 3, 2023 · da4f8f8 · da4f8f8
1 parent 1f5abb8
commit da4f8f8
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 23 deletions.
diff --git a/dev/src/reference.ts b/dev/src/reference.ts
@@ -1900,27 +1900,14 @@ export class Query<
       DocumentSnapshot<AppModelType, DbModelType> | unknown
     >
   ): FieldOrder[] {
-    // Add an implicit orderBy if the only cursor value is a DocumentSnapshot
-    // or a DocumentReference.
+    // Add an implicit orderBy if the only cursor value is a DocumentSnapshot.
     if (
       cursorValuesOrDocumentSnapshot.length !== 1 ||
-      !(
-        cursorValuesOrDocumentSnapshot[0] instanceof DocumentSnapshot ||
-        cursorValuesOrDocumentSnapshot[0] instanceof DocumentReference
-      )
+      !(cursorValuesOrDocumentSnapshot[0] instanceof DocumentSnapshot)
     ) {
       return this._queryOptions.fieldOrders;
     }
 
-    // TODO(b/296435819): Remove this warning message.
-    if (cursorValuesOrDocumentSnapshot[0] instanceof DocumentReference) {
-      // eslint-disable-next-line no-console
-      console.warn(
-        `Warning: Passing DocumentReference into a cursor without orderBy clause is not an intended
-        behavior. Please use DocumentSnapshot or add an explicit orderBy on document key field.`
-      );
-    }
-
     const fieldOrders = this._queryOptions.fieldOrders.slice();
     const fieldsNormalized = new Set([
       ...fieldOrders.map(item => item.field.toString()),

diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts
@@ -309,7 +309,9 @@ describe('Firestore class', () => {
 
       const documents: QueryDocumentSnapshot<DocumentData>[] = [];
       for (const partition of partitions) {
-        let partitionedQuery: Query = collectionGroup;
+        let partitionedQuery: Query = collectionGroup.orderBy(
+          FieldPath.documentId()
+        );
         if (partition.startAt) {
           partitionedQuery = partitionedQuery.startAt(...partition.startAt);
         }
@@ -1740,9 +1742,10 @@ describe('Query class', () => {
     expectDocs(res, {foo: 'b'});
   });
 
-  it('startAt() adds implicit order by for DocumentReference', async () => {
+  it('startAt() adds implicit order by for DocumentSnapshot', async () => {
     const references = await addDocs({foo: 'a'}, {foo: 'b'});
-    const res = await randomCol.startAt(references[1]).get();
+    const docSnap = await references[1].get();
+    const res = await randomCol.startAt(docSnap).get();
     expectDocs(res, {foo: 'b'});
   });
 
@@ -3046,16 +3049,18 @@ describe('Aggregates', () => {
       await randomCol.doc('doc6').set({foo: 'bar'});
       await randomCol.doc('doc7').set({foo: 'bar'});
 
-      const count1 = randomCol.startAfter(randomCol.doc('doc3')).count();
+      const docSnap = await randomCol.doc('doc3').get();
+
+      const count1 = randomCol.startAfter(docSnap).count();
       await runQueryAndExpectCount(count1, 4);
 
-      const count2 = randomCol.startAt(randomCol.doc('doc3')).count();
+      const count2 = randomCol.startAt(docSnap).count();
       await runQueryAndExpectCount(count2, 5);
 
-      const count3 = randomCol.endAt(randomCol.doc('doc3')).count();
+      const count3 = randomCol.endAt(docSnap).count();
       await runQueryAndExpectCount(count3, 3);
 
-      const count4 = randomCol.endBefore(randomCol.doc('doc3')).count();
+      const count4 = randomCol.endBefore(docSnap).count();
       await runQueryAndExpectCount(count4, 2);
 
       const count5 = randomCol.offset(6).count();

diff --git a/dev/test/query.ts b/dev/test/query.ts
@@ -2302,7 +2302,7 @@ describe('startAt() interface', () => {
       firestore = firestoreInstance;
       return snapshot('collectionId/doc', {foo: 'bar'}).then(doc => {
         let query: Query = firestore.collection('collectionId');
-        query = query.startAt(doc.ref);
+        query = query.startAt(doc);
         return query.get();
       });
     });