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

Keep ID in document.data() #68

Closed
HauptmannEck opened this issue Feb 25, 2021 · 4 comments · Fixed by #83
Closed

Keep ID in document.data() #68

HauptmannEck opened this issue Feb 25, 2021 · 4 comments · Fixed by #83

Comments

@HauptmannEck
Copy link

HauptmannEck commented Feb 25, 2021

Summary

The mocking system strips the id from the mocked doc.data() response. I would like it to keep id in the resulting document.

Basic example

    mockFirebase( {
        database: {
            root: [
                {
                    id: 'root-id',
                    _collections: {
                        subCollection: [
                            {
                                id: 'id-1',
                                test: true
                            },
                        ],
                    },
                },
            ],
        },
    } );
           
   firestore.collectionGroup( 'subCollection' ) .where( 'id', '==', 'id-1' ).get()
           .then((snap) => {
               const resp = snap.docs[0].data();
               // resp is {test: true} needs to be {id: 'id-1', test: true}
            });

Motivation

To allow us to search for a specific document within a collectionGroup we keep the id of our document stored as an id field in the document. So we need to have the mock not strip that out when returning doc.data().
Not sure if it should be an option on mockFirebase, automatic, or another field:
ex. {keepIds: true} or {id: 'id-1', _id: 'id-1'},

Side Question

It seems from my testing that each test file can only truly have 1 mockFirebase as they override each other, is it true that if I want to test say, both getting a Full QuerySnapshot and an Enpty one, that I will need 2 test files? As you cannot set the database field per test.

@sbatson5
Copy link
Owner

sbatson5 commented Mar 5, 2021

Hmm. So you have an additional field in your documents that are a copy of the id itself? That feels tricky, as the default behavior of a document is to return the id separate from the data() -- but if you have a field on the document called id as well, that's trickier and feels like an edge case.

For your side question, you should be able to do it in the same file, but not within the same describe block. Jest will hoist mocks to the top of the function, regardless of where you declare them. So if you wanted to test those two cases, you would need two distinct describe blocks.

@HauptmannEck
Copy link
Author

HauptmannEck commented Mar 14, 2021

Yeah it's a workaround, as the collectionGroup cannot get by id. To allow us to use similar context for doc(id).get() and collectionGroup(collection).where('id', '==', id). This workaround is so that we can find a record directly without having to carry around the multiple ids to taverse the whole tree to get to that record.
It is a common issue for firestore queries, and this is a popular solution for it.

@sbatson5
Copy link
Owner

sbatson5 commented Apr 2, 2021

Hey @HauptmannEck, can you take a look at this PR and see if it works for your use-case?
#83

@HauptmannEck
Copy link
Author

@sbatson5 Looks like a great solution that will solve for my usecase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants