Add a mixin class for dashboard views on models with project relations #2353
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As we overhaul the project admin dashboard, a good amount of code can be cleaned
up by using a CBV rather than repeating the view code like is currently used. I
had a need for this outside this code base, but implemented it here instead.
Views would look like:
class DomainMixin(ProjectRelationMixin):
model = Domain
lookup_url_kwarg = 'pk'
class ListDomainView(DomainMixin, ListView):
pass
class DetailDomainView(DomainMixin, DetailView):
pass
Views would then have access to a `project` and `domains` in template context
data.
agjohnson
force-pushed
the
project-relation-mixin
branch
from
028e88e to
9efcdf1
Compare
| context = {} | ||
| try: | ||
| context = super(ProjectRelationMixin, self).get_context_data(**kwargs) | ||
| except AttributeError: |
There was a problem hiding this comment.
Is this check really needed? get_context_data() comes from ContentMixin and it's defined in almost all class based views except View and RedirectView.
There was a problem hiding this comment.
Only because this was written as a mixin class independent of any base class. But I think perhaps basing this class on ContextMixin is the most correct thing to do.
There was a problem hiding this comment.
Nevermind, MRO here means the ContextMixin.get_context_data returns context data, but this should instead be able to fallback to vanilla view's get_context_data. I altered the call based on this feedback to instead expect get_context_data to always be implemented on a super class.
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As we overhaul the project admin dashboard, a good amount of code can be cleaned
up by using a CBV rather than repeating the view code like is currently used. I
had a need for this outside this code base, but implemented it here instead.
Views would look like:
Views would then have access to a
projectanddomainsin template contextdata.