About #8500 and #27728

134 views
Skip to first unread message

Raffaele Salmaso

unread,
Sep 5, 2017, 4:58:50 AM9/5/17
to django-d...@googlegroups.com
Hi all,
I'm working to ticket #8500 and #27728 to improve a bit the admin, and being asked to ask to django-developers about some decision.

In ticket #8500 I've used a LazyObject to defer the the real django.contrib.admin.sites.site instantiation, so admin.site is always the custom one.
The first approach was simpler, with a setting. But as Tim pointed out it doesn't need to be a setting, so I rewrote to be a custom field for (Simple)AdminConfig (and django itself doesn't need to know it existence)

Currently I'm monkey patching in my custom admin setup (https://bitbucket.org/rsalmaso/django-fluo is my own utility library)

> class FluoAdminConfig(AdminConfig):
>     default_site = "fluo.admin.sites.AdminSite"
>
>     def ready(self):
>         self.override_admin_site()
>         super().ready()
>
>     def override_admin_site(self):
>         from django.contrib import admin as django_admin
>         from django.contrib.admin import sites as django_sites
>         from fluo import admin as fluo_admin
>         from fluo.admin.sites import DefaultAdminSite
>
>         site = DefaultAdminSite()

where the DefaultAdminSite is the same as in PR
> class DefaultAdminSite(LazyObject):
>     def _setup(self):
>         AdminSiteClass = import_string(apps.get_app_config('admin').default_site)
>         self._wrapped = AdminSiteClass()


I think it is a better approach than the Jannis' one (https://code.djangoproject.com/attachment/ticket/8500/8500.1.diff), as Adam claims it is simpler, because
- it doesn't require to rewrite your admin.py to add an helper function in admin.py
> def register(site):
>     site.register(...)
- it works with all current third party modules (so don't have to add a custom adapter in my own project)
- can use the @admin.register() decorator as is

Currently all my projects use this approach, and I'm really happy because I find no problem with 3rd apps, they always use my custom instance, and don't have two different site.

In ticket #27728 I've rewrite all include template tags (submit_row, pagination, admin_actions, search_form, date_hierarchy, result_list, prepopulated_fields_js, object_tools) to be overridable by the "standard" admin pattern (["admin/app_label/app_model/template_name", "admin/app_label/template_name", "admin/template_name"]).
This patch solves even the tickets #11974, #12566, #18957, #19235, #26763.
Choose to patch all include tags because:
- I have a project where I override all of them (!)
- they are all the same implementation, so easy to replicate

The approach is to create a custom InclusionAdminNode, which handle all common task to split the include parameters, select the correct template and wrapping the original tag function (which are never changed).

Hope this clears all intentions and have them in django 2.0 :)

--

Raffaele Salmaso

unread,
Jan 19, 2018, 4:41:22 PM1/19/18
to django-d...@googlegroups.com
Hi all,
as these PR have merge conflicts that need to be resolved, and it takes time to keep that ready to merge, I would be glad to know if continue to work on them (and how to get them merged) or just drop them and carry on.

Thanks

Carlton Gibson

unread,
Jan 20, 2018, 3:13:13 AM1/20/18
to Django developers (Contributions to Django itself)
Hi Raffaele, 

Thanks for both your effort and your patience. 

There are around 60 patches needing review (or just reviewed or being improved or...) at the moment, so it can take a while to get to. 

I will look at this issue on Monday. 

Please don't loose heart. 🙂

Kind Regards,

Carlton

Raffaele Salmaso

unread,
Jan 21, 2018, 4:26:32 PM1/21/18
to django-d...@googlegroups.com
Hi Carlton,

On Sat, Jan 20, 2018 at 9:13 AM, Carlton Gibson <carlton...@gmail.com> wrote:
Hi Raffaele, 

Thanks for both your effort and your patience. 

There are around 60 patches needing review (or just reviewed or being improved or...) at the moment, so it can take a while to get to. 

I will look at this issue on Monday. 
Thanks!
 
Please don't loose heart. 🙂
Don't worry😁 

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscribe@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/9b5418c5-5828-40b0-8f3e-dac9d50fd26c%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Carlton Gibson

unread,
Jan 22, 2018, 10:53:27 AM1/22/18
to Django developers (Contributions to Django itself)
Hi Raffaele,

I took a look at both these PRs — they both seem like good wins to me. 
I've commented briefly on the issues. 

Here I would just ask if people using the Admin have capacity to review that 
would be great. (If not I'm inclined to push these forwards.)


Kind Regards,

Carlton

Reply all
Reply to author
Forward
0 new messages