BeanieODM / beanie

Asynchronous Python ODM for MongoDB
http://beanie-odm.dev/
Apache License 2.0
1.99k stars 211 forks source link

Added a feature that lets user create a Read Only Document #704

Closed ahmarhafeez1 closed 9 months ago

ahmarhafeez1 commented 12 months ago

I have added a feature that lets you create a document that is only able to read from DB and not modify it.

You can set read_only = True in Settings Object to make use of this feature by default it does not alter any other functionality of the ODM

roman-right commented 11 months ago

Hi @ahmarhafeez1,

Thank you for the PR.

I have a few comments. Please take a look there.

Also, I'd like to discuss the feature in general.

In this implementation, readonly documents can be changed with other methods, like class-level update:

await ReadOnlySample.all().update(...)

This can be confusing, as users might think that all the readonly documents are protected.

This can be avoided if you put a service field for such documents, like _read_only=True, in the database. Then, when you make updates or deletes, in the filter query part, you can add this condition to avoid such documents. And, probably, if a user needs to change this, you can add a special parameter to the changing operations. With this parameter set to true, it will change read-only docs.

What do you think?

ahmarhafeez1 commented 11 months ago

Hi @ahmarhafeez1,

Thank you for the PR.

I have a few comments. Please take a look there.

Also, I'd like to discuss the feature in general.

In this implementation, readonly documents can be changed with other methods, like class-level update:

await ReadOnlySample.all().update(...)

This can be confusing, as users might think that all the readonly documents are protected.

This can be avoided if you put a service field for such documents, like _read_only=True, in the database. Then, when you make updates or deletes, in the filter query part, you can add this condition to avoid such documents. And, probably, if a user needs to change this, you can add a special parameter to the changing operations. With this parameter set to true, it will change read-only docs.

What do you think?

Yeah We can do that but I was thinking more along the lines of creating Read Only Documents for certain instances in your app where you need to read data if we added readonly property to DB it would permanently change those documents to readonly

roman-right commented 11 months ago

Yeah We can do that but I was thinking more along the lines of creating Read Only Documents for certain instances in your app where you need to read data if we added readonly property to DB it would permanently change those documents to readonly

I see. I still think it's better to make docs fully read-only, with the possibility of a force update using a special parameter. Based on my experience, users don't distinguish much between the logic of the instances and the logic of the classes. In this particular case, it can lead to serious mistakes.

roman-right commented 9 months ago

It looks like this PR is not in work more. If no, feel free to open a new one.