Provide .url_for_self for the Base model
|Reported by:||spaetz||Owned by:|
# We have to pass in an "urlgen" object to MediaEntry().url_for_self() to actually get the url. Being attached to the request, this means we have to carry around the request object all over the place rather than having that encapsulated nicely. Compare to django's "get_absolute_url()" which does not need such complexities. (I also think we should implement a get_absolute_url template in the Base model class, to enforce uniformity by the way).
# in contrast to werkzeug tools, this always returns relative URLs, so the URL generation in the atom feed that requires absolute urls looks for example:
werkzeug.urls.Href(self.request.host_url ( media_entry.url_for_self(self.request.urlgen)))
when what I want is:
(or even better a more consistent media_entry.get_absolute_url())
It should be possible to enforce absolute external urls easily without having to manually parse and fudge the request in every caller.
# Nitpick: MediaEntry.url_for_next and url_for_prev are defined in models.py, while url_for_self is defined in mixin.py. This should be made consistent.
Things would be easier if the url generation did not depend on the request-specific MapAdapter (request.urlgen), which it currently does. Not sure, how this could be improved.
To sum up:
- At the very least, this bug should be about adding an external=True parameter that will return absolute, external URLs.
- The url generation should be defined in the same places (mixin or models)
- Optionally, we would implement a "get_absolute_url" in the Base model that all subclasses can override, so we can use the same consistent interface for all.This could be the same as "url_for_self" here for now.
- Ideally, we don't have to pass in the request into the url_for_self function so we don't always have to carry that around and to simplify things. However, I don't see an easy way to fix that, so this could be a step 2 of this ticket.
Change History (6)
comment:4 by , 10 years ago
|Priority:||major → minor|
|Summary:||MediaEntry.url_for_self is horrible, horrible API → Provide .url_for_self for the Base model|