Handling CreateView form data precondition errors

Using Django
1

Hey, all! I hit an interesting design choice on my Twitch stream this evening related to Class Based Views, and I wanted to check if anyone has alternative patterns to the solution I arrived at.

I have a CreateView that creates records for a Course model. The form class that I’m using needs a QuerySet that is dynamically set when the form is created. Here’s the whole form for context:

class CourseForm(DaysOfWeekModelForm):
    class Meta:
        model = Course
        fields = ["name", "grade_levels"] + DaysOfWeekModelForm.days_of_week_fields

    grade_levels = forms.ModelMultipleChoiceField(queryset=GradeLevel.objects.none())

    def __init__(self, school_year, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.fields["grade_levels"].queryset = GradeLevel.objects.filter(
            school_year=school_year
        )

In my view, I need to provide an appropriate school_year record, which I do in get_form_kwargs.

    def get_form_kwargs(self):
        kwargs = super().get_form_kwargs()
        kwargs["school_year"] = SchoolYear.get_current_year_for(self.request.user)
        return kwargs

My challenge is this: SchoolYear.get_current_year_for may return None. When the value is None, I don’t want the page to 404. Ideally, it would redirect to a page where a user can create a school year.

The problem is that, to my knowledge, there isn’t a great way to trigger a redirect from the get_form_kwargs method.

This is an exceptional case for my app so I’ve considered using an exception. I’ve considered raising a new exception like NoSchoolYearError in get_form_kwargs and handling that exception in dispatch to do the redirect.

Generalized, I think my question is this: is there an appropriate hook in CBVs to use to check on form data preconditions before a form is instantiated?

If you’ve got other ideas or I’m missing something obvious about CBVs, I’d love to read your thoughts. Thanks.

2

That’s an interesting topic.

My first reaction would be to override the get method on my view to make this check before calling super. If I need to call the other view, then I call that view directly returning its response rather than my own.

I’m envisioning something along the lines of:

def get(...):
    if (preconditions satisfied):
        return super().get(...)
    else:
        return OtherCreateView().get(...)

(Note: I’ve never tried anything like this - this is a real WAG.)

But in our situation, we took a slightly different approach. (We had a similar, but not identical situation.) Our template includes a conditional that selects between one of two forms when the page is rendered. So the context determines which half of the template is seen. (Also, in this case we used a FormView, not a CreateView)

Ken

3

As I understand, @mblayman only wants to return a redirect, not the response generated by another view. I think your approach would work for that though Ken.

@mblayman Since you’ll need to call get_current_year_for() in both the GET and POST paths, I think it would work to call it in dispatch(), store the SchoolYear on self or redirect appropriately, and fetch it in get_form_kwargs().

Or you could write a function based view…

4

As others have said, you can solve this issue by extracting the retrieval of the user’s current school year out of get_form_kwargs().

As Adam said, dispatch() could be a good place for it. There’s also setup() which is not that different. Here’s how I’d do it:

class CourseCreateView(CreateView):
    form_class = CourseForm

    # If you find yourself doing this a lot, this `setup` method could
    # be extracted in a mixin and reused
    def setup(self, request, *args, **kwargs):
        super().setup(request, *args, **kwargs)
        self.school_year = SchoolYear.get_current_year_for(request.user)

    def get(self, request, *args, **kwargs):
        if self.school_year is None:
            return redirect(...)
        return super().get(request, *args, **kwargs)

    def get_form_kwargs(self):
        kwargs = super().get_form_kwargs()
        kwargs['school_year'] = self.school_year
        return kwargs

Note that with this implementation, you have a potential problem fs the user’s current school year becomes None between the time you display your form (get) and when you submit it (post).

5

I totally forgot about setup, @bmispelon! Those are all great suggestions everyone. I appreciate you thinking about it.

Storing school_year on self certainly sounds like a very reasonable way to handle this without introducing a special exception class.

6

An update on this because I thought it was interesting.

I tried storing school_year on self in setup and in dispatch. The approach didn’t work for me. The approach failed because setup happens before dispatch. Since I’m using the LoginRequiredMixin (which works by overriding dispatch), it’s possible to call the model method of get_current_school_year_for with an AnonymousUser. This fails in my method implementation when it needs to use the user as a filter to make sure that the data only pulls from the user’s school years and not any other users.

Because all of the interactions with this SchoolYear model are for authenticated users, I don’t want to starting writing guard code to check if the user is anonymous or not.

I still think this approach could have worked if it wasn’t dependent on an authenticated user. I think this is an interested case when using a CBV works less well than a function based view. In a function, @login_required would be guaranteed to run before the view function body so it wouldn’t be possible for an AnonymousUser to be a possibility.

7

I hadn’t thought of this unfortunate interaction with LoginRequiredMixin.dispatch(), that does make the CBV approach less practical.

I’ve been using a “homemade” LoginRequiredMixin that wraps as_view() with login_required so I hadn’t run into the problem you’re describing (which I implemented before the official LoginRequiredMixin was a thing).

You could probably fix your issue with some clever ordering of mixins in your inheritance tree (to make sure LoginRequiredMixin.dispatch() runs first) but at this point it might be wise to give up on CBV and implement a FBV instead.