[admin] submit_row template tag reusability issue

[Federico Capoano]

Hi all,

I came across the need of modifying the submit row functionality in the django admin and I found out a small issue which prevents me from modifying the template without rewriting (duplicating) also the templatetag entirely.

If you take a look at the code of submit_row templatetag, you will notice the original view context is not passed entirely to the return statement. A new ctx dictionary is created and filled only with a smaller subset of key/value pairs.

I took a quick look at other template tags and I noticed they return the entire context and not a stripped version of it.

I would prefer submit_row to behave in the same way as the other templatetags so I wouldn't have to duplicate its code to add new context values to it.

If there's consensus on this issue I can create a ticket and send a pull request, let me know.

Federico

[Tim Graham]

Here is an accepted ticket: https://code.djangoproject.com/ticket/13875

Feel free to update and/or improve the patch, add tests, and send a pull request. Thanks!

[Federico Capoano]

Hi Tim,

thank you for letting me know of the open ticket.

The "patch needs improvement" flag is set to "no". Is that correct or not?

Federico

--
You received this message because you are subscribed to a topic in the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/django-developers/yrngp53RvxY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@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/bbb82460-8a55-47ff-8e7f-acfc063f6cf0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Tim Graham]

The patch is 5 years old and doesn't apply cleanly, so it needs improvement in that sense but that shouldn't be difficult. The main blocker to getting it merged looks like "Needs tests."

[Federico Capoano]

Updated the ticket and sent a PR, I report the comment here too for convenience:

Just opened PR https://github.com/django/django/pull/6078.

A more paranoid test could be written, like creating a custom app with an admin that extends the context, loads a modified version of the subimt_row template and check the extended context is available in the template. But I think all the complexity needed wouldn't add much value.

Federico

[Federico Capoano]

Ticket closed: https://code.djangoproject.com/ticket/13875

I guess there might other 5-6 year old tickets lying around.. this one in particular did not have "patch needs improvement" or "easy picking", these two flags would help wanna be contributors to find this type of tickets, especially during events like sprints.

Thanks to Tim for the review.

Federico