-
Notifications
You must be signed in to change notification settings - Fork 59
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
Enhancement: ModelView.find_all and ModelView.count still don't take into account the Request for the stmt - needed for multitenant support #489
Comments
How about we wrap the calls to |
I once was in a talk by a famous software developer and he was covering a DO/DONT list. In the DONT list, was the usage of exceptions as a control flow mechanism. So, I never do. To be honest, I prefer my solution because there is no magic involved, it is bread&butter OO, and backwards-compatible. And very K.I.S.S. You can put the deprecation warnings in the 2 new methods in views.py.ModelView But either way, I am looking forward to having this, thank you SO MUCH for listening! Super happy here. |
I agree with you, but adding those two methods may cause confusion. Instead, we can introduce a breaking change in release 0.14. |
Thank you for your kind words. I'm glad to know that you are enjoying being here. |
I agree with the OP, the first suggestion seems better, and very much needed for this kind of filter. |
Describe the bug
As already suggested by me in #274 , views.py.ModelView.find_all and views.py.ModelView.count still do not take into account the Request when building the stmt. This forces people to subclass this class, and copy/paste code every time a startlette-admin version upgrade comes out.
To Reproduce
Try to filter records based on the logged user, properly supporting multi tenancy.
The problem is in the line stmt = self.get_list_query().offset(skip) , in both methods.
Environment (please complete the following information):
Suggested fix
with this line:
with this line:
This will make things work, and it will give people a hook to properly filter based on the logged user (their id). And, the best of all: it is backwards-compatible.
Additional context
Without the support above, I am force to do this every time I upgrade starlet-admin:
Copy both methods into my own custom subclass called WorakaroundModelView, fixing imports and hoping things will still work
Find the spots and apply the changes again
The text was updated successfully, but these errors were encountered: