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

Auto-ID for arrays instead of index #38

Merged
merged 4 commits into from
Mar 10, 2020
Merged

Conversation

3zzy
Copy link
Contributor

@3zzy 3zzy commented Mar 7, 2020

Lets Firebase generate auto-id's if an array or objects is passed.

@dalenguyen
Copy link
Owner

Thanks for contributing. Can you plain more with an example of this case and update the test file cover this? Thanks,

@3zzy
Copy link
Contributor Author

3zzy commented Mar 9, 2020

When importing data, if you provide an array of objects, they would be imported with their numeric indexes as the key (eg. 0, 1, 2 ..). That's because the import script uses set() by default. To let Firebase generate the IDs, we should use the add() instead.

From the docs:

When you use set() to create a document, you must specify an ID for the document to create. In the case of arrays, all you have are numeric indexes and that is used as key currently.

But sometimes there isn't a meaningful ID for the document, and it's more convenient to let Cloud Firestore auto-generate an ID for you. You can do this by calling add()

Read: https://cloud.google.com/firestore/docs/manage-data/add-data#add_a_document

Also its not recommended to use numeric indexes.

This PR fixes that, but I'm not sure how to write tests for that.

@dalenguyen
Copy link
Owner

Hi @3zzy, the data structure already has an id for the document from export function. We don't have to regenerate it. Which JSON structure are you trying to import?

https://github.com/dalenguyen/firestore-backup-restore/blob/master/test/import-to-firestore.json

In the example, the document id is already defined: first-key & second-key.

@3zzy
Copy link
Contributor Author

3zzy commented Mar 9, 2020

@dalenguyen Right, but for when the id needs to be auto generated instead of manually defining it, you could pass an array of objects.

So from the above example JSON. Instead of:

{
    "test": {
        "first-key": {
        ...
        "subCollection": {
            "test/first-key/details": {
            ...
            }
        }
        },
        "second-key": {
        ...
        }
    }
}

We pass:

{
    "test": [
        {
            ...
            "subCollection": {
                "details": {
                    ...
                }
            }
        },
        {
            ...
        }
    ]
}

@dalenguyen
Copy link
Owner

@3zzy you're right. This is another case. I will add the sample file and test cases for supporting this case.

@dalenguyen dalenguyen changed the base branch from master to dev March 10, 2020 18:29
@dalenguyen dalenguyen merged commit b677872 into dalenguyen:dev Mar 10, 2020
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 this pull request may close these issues.

2 participants