You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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):
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:
How about we wrap the calls to get_list_query and get_count functions inside a try-catch block? In the try block, we can pass the request and to maintain backward compatibility, we will catch the Too Many Arguments error and call the functions without the request, but with a deprecation warning.
jowilf
changed the title
Bug: ModelView.find_all and ModelView.count still don't take into account the Request for the stmt - needed for multitenant support
Enhancement: ModelView.find_all and ModelView.count still don't take into account the Request for the stmt - needed for multitenant support
Jan 24, 2024
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.
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: