octabytes / FireO

Google Cloud Firestore modern and simplest convenient ORM package in Python. FireO is specifically designed for the Google's Firestore
https://fireo.octabyte.io
Apache License 2.0
250 stars 29 forks source link

Collection.get fetch by id without collection name as prefix #70

Closed sreeram-dev closed 4 years ago

sreeram-dev commented 4 years ago

When we are making a query - User.collection.get(user_key)

user_key has to be the form of 'collection_name/{id}'

Can we support the form of just querying by ID and appending the collection name?

if '/' not in self.collection_path:
            # check collection path is the same as collection name
            if self.collection_path != self.model_cls.collection_name:
                raise errors.InvalidKey(f'Invalid key is given, expected "{model_name}" type key, '
                                        f'got "{self.collection_path}" type key')

In the above code, without a prefix, it returns invalid_key

Version:1.3.3

sreeram-dev commented 4 years ago

@AxeemHaider If you feel this is a valid issue, I can work on it and submit a PR.

AxeemHaider commented 4 years ago

I was already thinking about it that's why I added the helper function to generate key from id. If you see utils there is a method named generateKeyFromId(model, id)

Passing a model and id will return you key e.g collection_name/id

I was thinking about to add functionality to accept id in get() method For example you can do like this User.collection.get(id='user-123') Something like this.

Yes you can send PR it will be acceptable

trungtin commented 1 year ago

Since you removed generateKeyFromId(model, id) in version 2.0.0, what is the recommended way to get the key without manually appending the collection_name? Or is there anyway to query using id? @AxeemHaider

ADR-007 commented 1 year ago

Hi there,

In my project, I use:

class MyManager(Manager):
    def get_by_id(self, id_: str, transaction=None):
        return self.get(self.get_key_by_id(id_), transaction)

    def get_key_by_id(self, id_: str) -> str:
        self.model_cls: Type[Model]
        key = f'{self.model_cls.collection_name}/{id_}'
        if self._parent_key:
            key = f'{self._parent_key}/{key}'

        return key

But I would like to have some implementation out of the box. I can make a PR, but we need to choose the right solution first 🤔

AxeemHaider commented 1 year ago

@ADR-007 I like your solution, it's ok we can with this what you think?

ADR-007 commented 1 year ago

Ok, I'll make a PR in a few following days :)