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.

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

Jump to bottom

Adding FieldMask support to GetAll() #443

Merged
merged 3 commits into from
Nov 12, 2018
Merged

Adding FieldMask support to GetAll() #443

merged 3 commits into from
Nov 12, 2018

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Nov 9, 2018 •
edited
Loading

This adds a ReadOptions argument to getAll() to allow users to specify a field mask.

I tried to keep the API useable and declared the first argument of the varargs method so that it is known the DocumentReferences have to be passed first.

Fixes: #42

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 9, 2018
gsoltis
gsoltis suggested changes Nov 10, 2018
View reviewed changes
dev/src/index.ts
* @param options.fieldMask - The subset of fields to return from a read
* operation.
*/
export function validateReadOptions(options: ReadOptions): boolean {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dev/src/transaction.ts Outdated
@@ -117,7 +118,7 @@ export class Transaction {

if (refOrQuery instanceof DocumentReference) {
return this._firestore
.getAll_([refOrQuery], this._requestTag, this._transactionId)
.getAll_([refOrQuery], null, this._requestTag, this._transactionId)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dev/src/util.ts Outdated
let documents: DocumentReference[];
let readOptions: ReadOptions|undefined = undefined;

const usesVarags = !Array.isArray(documentRefsOrReadOptions[0]);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dev/src/util.ts Outdated
documents = documentRefsOrReadOptions as DocumentReference[];
}
} else {
// Support an array of document references as the first argument for

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dev/test/index.ts
@@ -883,7 +879,8 @@ describe('getAll() method', () => {
const overrides = {batchGetDocuments: () => stream(found('documentId'))};

return createInstance(overrides).then(firestore => {
return firestore.getAll(firestore.doc('collectionId/documentId'))
return (firestore as InvalidApiUsage)
.getAll([firestore.doc('collectionId/documentId')])

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@gsoltis gsoltis assigned schmidt-sebastian and unassigned gsoltis Nov 10, 2018
schmidt-sebastian
schmidt-sebastian commented Nov 10, 2018
View reviewed changes
Copy link
Contributor Author

@schmidt-sebastian schmidt-sebastian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments addressed. Thanks for the review.

dev/src/index.ts
* @param options.fieldMask - The subset of fields to return from a read
* operation.
*/
export function validateReadOptions(options: ReadOptions): boolean {

This comment was marked as spam.

Sign in to view

dev/src/transaction.ts Outdated
@@ -117,7 +118,7 @@ export class Transaction {

if (refOrQuery instanceof DocumentReference) {
return this._firestore
.getAll_([refOrQuery], this._requestTag, this._transactionId)
.getAll_([refOrQuery], null, this._requestTag, this._transactionId)

This comment was marked as spam.

Sign in to view

dev/src/util.ts Outdated
let documents: DocumentReference[];
let readOptions: ReadOptions|undefined = undefined;

const usesVarags = !Array.isArray(documentRefsOrReadOptions[0]);

This comment was marked as spam.

Sign in to view

dev/src/util.ts Outdated
documents = documentRefsOrReadOptions as DocumentReference[];
}
} else {
// Support an array of document references as the first argument for

This comment was marked as spam.

Sign in to view

dev/test/index.ts
@@ -883,7 +879,8 @@ describe('getAll() method', () => {
const overrides = {batchGetDocuments: () => stream(found('documentId'))};

return createInstance(overrides).then(firestore => {
return firestore.getAll(firestore.doc('collectionId/documentId'))
return (firestore as InvalidApiUsage)
.getAll([firestore.doc('collectionId/documentId')])

This comment was marked as spam.

Sign in to view

@schmidt-sebastian schmidt-sebastian merged commit 791eacc into master Nov 12, 2018
@schmidt-sebastian schmidt-sebastian mentioned this pull request Nov 19, 2018
Merged
@jmdobry jmdobry deleted the mrschmidt-getall branch December 5, 2018 07:07
@ushu ushu mentioned this pull request Dec 28, 2018
Closed
@snyk-bot snyk-bot mentioned this pull request Dec 5, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsoltis gsoltis gsoltis approved these changes

Assignees

@gsoltis gsoltis

@schmidt-sebastian schmidt-sebastian

Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@schmidt-sebastian @gsoltis @googlebot @JustinBeckwith