search

barseghyanartur / django-admin-timeline

A Facebook-like timeline app for Django admin. It's very similar to built-in feature Daily progress, but has nicer templates and infinite scroll. Actions are broken up by day, then by action. It's possible to filter actions by user (multiple select) and content type (multiple select).
https://pypi.python.org/pypi/django-admin-timeline
53 stars 10 forks source link

Link in the timeline are wrong. #2

Closed mick-t closed 8 years ago

mick-t commented 8 years ago

Hi,

I updated django-admin-timeline from 1.4 to 1.5.3 and now links to models in the timeline are incorrect.

The should be like this:

/admin/app/model/pk

Instead they have timeline as part of the link:

/admin/timeline/app/model/pk

See the screenshot for an example, the link is shown in the bottom of the screenshot (in the browser statusbar).

screen shot 2015-09-29 at 18 03 35

This is using Django 1.5.12 and Python 2.7.3, Django Grappelli 2.4.6.

Thanks.

mick-t commented 8 years ago

The links work correctly in 1.5.0, but not in 1.5.1. I'll look at the source later this week to see if can figure out where it changed/broke.

Cheers Mick

barseghyanartur commented 8 years ago

@mick-t:

Thanks for reporting this. Could you be more specific? Which Django version do you use? I can't reproduce it on Django==1.8a1/django-admin-timeline==1.5.3.

mick-t commented 8 years ago

Django 1.5.12. Versions are at the bottom of my original report :)

mick-t commented 8 years ago

I did some digging.

get_admin_url() in 1.5.12 returns a URI like this:

app/model/pk

In the timeline_ajax.h line 37 the URL is generated like this:

<a href="{{ entry_admin_url }}">{{ entry_object_repr }}<span class="change">{{ entry_change_message }}</span></a>

Which produces a relative URL that looks like this:

<a href="app/model/pk/">/This is a Model.<span class="change">Changed pages.</span></a>

Replacing it with this creates the correct URL:

<a href="/admin/{{ entry_admin_url }}">{{ entry_object_repr }}<span class="change">{{ entry_change_message }}</span></a>

Can you verify and test with 1.8 and if this works for you I'll create a pull-request.

Edit: Diff here: https://github.com/mick-t/django-admin-timeline/commit/e380621424bdb330a928d3afa42fd345853bc959

barseghyanartur commented 8 years ago

@mick-t:

Hey, thanks for looking it up. I checked the sources of Django 1.4.x (https://github.com/django/django/blob/stable/1.4.x/django/contrib/admin/models.py) and 1.5.x (https://github.com/django/django/blob/stable/1.5.x/django/contrib/admin/models.py). The get_admin_url method is indeed not well implemented there. Starting from version 1.6.x (https://github.com/django/django/blob/stable/1.6.x/django/contrib/admin/models.py) it works well. I have moved that part into a separate template tag.

Latest version (1.5.4) contains the fix.