search

onespacemedia / cms

A collection of Django extensions that add content-management facilities to Django projects.
BSD 3-Clause "New" or "Revised" License
14 stars 7 forks source link

Remove the `cached_url` field from the Page model. #181

Closed lewiscollard closed 4 years ago

lewiscollard commented 4 years ago

First, it seems that this never actually gets used on our projects, or in the CMS internally. That is almost a good standalone reason to remove it.

Secondly, if it was being used, it would be unreliable, because a page's URL can change without the save method being called on it. For example, if the parent changes, or the parent's slug changes, or if something is changed with queryset.update, the URL changes but save is not called. I do not think it is a good idea to attempt to catch all of these cases.

Once it is incorrect, it is impossible to detect the case in which it is incorrect[1]. It would only be correct again once get_absolute_url had been called without cached=True (this will update cached_url - which is slightly surprising behaviour!). It so happens that get_absolute_url is likely to be called on it at some point, but that is not guaranteed - i.e. the breakage is quiet and unpredictable, the kind I like least.

[1] By checking to see if it was stale, you'd have to compare it with the result of get_absolute_url(cached=False), which negates having a cached URL at all.