Refactor footer_html view to class #6125
Conversation
This is going to help us to extend this view more easily from the corporate site.
| if not project: | ||
| project_slug = self.request.GET.get('project', None) | ||
| project = get_object_or_404(Project, slug=project_slug) | ||
| setattr(self, cache_key, project) |
There was a problem hiding this comment.
Should we actually be using the real Django cache, and caching these across lookups? That would make it so we never have to hit the DB in the footer in normal cases.
Probably doesn't make sense, but worth a thought.
There was a problem hiding this comment.
Do you mean cache the view or the model? I'm opening a new issue to follow up.
| renderer_classes = [JSONRenderer, JSONPRenderer] | ||
|
|
||
| def _get_project(self): | ||
| cache_key = '_cached_project' |
There was a problem hiding this comment.
These cache keys should have a more specific prefix, like: footerhtml_cached_project. Otherwise, it's easy to override by mistake.
There was a problem hiding this comment.
It's an attribute on the class, so it's naturally scoped I think.
There was a problem hiding this comment.
Oh, right. We will need to change it if we move into a shared cache as you suggested, though
This is going to help us to extend this view more easily
from the corporate site.
Logic is the same, just code was moved.