8000 fix: Remove DocumentReference from cursor by milaGGL · Pull Request #1882 · googleapis/nodejs-firestore · GitHub
[go: up one dir, main page]
Skip to content

fix: Remove DocumentReference from cursor #1882

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 3, 2023
Merged

fix: Remove DocumentReference from cursor #1882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e4caa15
remove DocumentReference from cursor
milaGGL Aug 15, 2023
2e2ea21
Merge branch 'main' into mila/fix-DocumentReference-in-cursor
milaGGL Oct 2, 2023
ce75bd3
remove the warning
milaGGL Oct 2, 2023
f71e0c3
Merge branch 'main' into mila/fix-DocumentReference-in-cursor
MarkDuckworth Oct 3, 2023
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
19 changes: 12 additions & 7 deletions dev/system-test/firestore.ts
View file
Open in desktop
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
View file
Open in desktop
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
0