Closed luisza closed 7 years ago
Hey @luisza
First of all, thank you for the PR and for your time. It's nice to find someone proposing and implementing solutions.
I've read the patch and it looks ok to me, but I've a couple of comments/questions:
crud.py
now doing all the job instead of views.py
or does we need views.py
for something?. I mean, imho it's better to remove or merge them in only one.demo
app in the "new_crud_system" branch and the auth stuff seems to work ok (it gave me a 403 when I was not logged, worked like a charm once I logged in admin/
) but the form is not being rendered properly (no tabs, neither widgets). Screenshot attached:django admin
, what do you think? My mind thinks in a crud system for users, groups and perms too but probably it's too much.I think this PR would be perfectly accepted if we're able to fix first two points. What do you think?.
Thanks again for all the efforts!
Ok, got it. The form is ok, in the demo
app you included both examples, with namespace
(no modelforms) and with modelforms
(no namespace):
ns = crud_for_app('testapp', check_perms=True, namespace="ns")
urlpatterns += crud_for_app('testapp', check_perms=True, modelforms=customerforms)
So that point is ok with me.
Is crud.py now doing all the job instead of views.py or does we need views.py for something?. I mean, imho it's better to remove or merge them in only one.
Yes, I think curd.py do all the job, I got all I need of views.py (CRUDMixin) and I think you can remove safely views.py code.
it gave me a 403 when I was not logged
Yea, but you will redirect to login view if set login_required=True, this is because 403 is an error when you haven't permissions to access a view (as django admin permission system).
urlpatterns += crud_for_app('testapp', login_required=True,
check_perms=True, modelforms=customerforms)
My mind thinks in a crud system for users, groups and perms too but probably it's too much.
I don't understand what you want, do you want crud for users and groups, something like
urlpatterns += crud_for_app('auth')
I also update demo branch.
urlpatterns += crud_for_app('auth')
That was just what I was looking for. Really nice. Do you want to be a collaborator and write changes directly in this repository? I'm sure it would be faster.
@luisza OTOH - if it's ok with you - would be nice to keep in touch via email or similar to talk about the goals of the project, features, priorities...
Do you want to be a collaborator and write changes directly in this repository?
Yes, and No, thats because I think PR are good approach, and provide second review.
Would be nice to keep in touch via email or similar to talk about the goals of the project, features, priorities.
Yea, I will send you a private email.
Hi, I create new CRUD system that works like your CRUD but with different approach.
Sorry if this PR change a lot of things I try to maintain all your features without change but including a lot of new features like permission system, namespace based urls, decorator per view system, UserCRUDView for saving user in model field user, possibility to overwrite generic view easy.
I also update urls function adding new parameters
CRUDView is a generic way to provide create, list, detail, update, delete views in one class, you can inherit for it and manage login_required, model perms, pagination, update and add forms how to use:
In views
In urls.py
The default behavior is check_login = True and check_perms=True but you can turn off with
You also can defined extra perms with
If checkperms = True we will add default django model perms (.[add|change|delete|view] )
You can also overwrite add and update forms
And of course overwrite base template name
Remember basename is generated like app_label/modelname if template_name_base is set as None and 'cruds' by default so template loader search this structure
basename + '/create.html' basename + '/detail.html' basename + '/update.html' basename + '/list.html' basename + '/delete.html'
Using namespace
In views
In urls.py
To see in action watch a demo brach in my fork. And sorry there are a lot of test that needs to be done.