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

fix: Remove DocumentReference from cursor #1882

Merged
merged 4 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions dev/src/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1900,27 +1900,14 @@
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()),
Expand Down Expand Up @@ -2109,7 +2096,7 @@
*/
startAt(
...fieldValuesOrDocumentSnapshot: Array<
firestore.DocumentSnapshot<any, any> | unknown

Check warning on line 2099 in dev/src/reference.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 2099 in dev/src/reference.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
>
): Query<AppModelType, DbModelType> {
validateMinNumberOfArguments(
Expand Down
19 changes: 12 additions & 7 deletions dev/system-test/firestore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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'});
});

Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion dev/test/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand Down
Loading