How to avoid this redundancy when using Django model inheritance?

Question

All pages in my Django website have a footer link "Feedback/Questions". If the new person comes to the site and clicks that link, they should be directed to a form with a pulldown to indicate if they have feedback versus a question and fields for their email address and their feedback or question. The page will have a simple header all non-authenticated users will see. On the other hand, if a site member signs in and is authenticated, they should see the same form but without the email field (since I already know their email address) and a different authenticated header containing the site's internal navbar, buttons, etc.

My initial thought was to create an abstract class FeedbackQuestion:

class FeedbackQuestion(models.Model):
  submission_type = ...  (type, i.e. feedback or question)
  submission_text = ...  (actual feedback or question)
  ...
  class Meta:
    abstract = True

Then I'd create two separate concrete child classes:

class AnonFeedbackQuestion(FeedbackQuestion):
  email = models.EmailField(...)
  class Meta:
    db_table = anon_feedback_question

class AuthFeedbackQuestion(FeedbackQuestion):
  user = models.ForeignKey(User, related_name="user")
  class Meta:
    db_table = auth_feedback_question

These two classes would have their own model forms:

class AnonFeedbackQuestionForm(ModelForm):
  class Meta:
    model = AnonFeedbackQuestion
    fields = ['submission_type', 'submission_text', 'email']

class AuthFeedbackQuestionForm(ModelForm):
  class Meta:
    model = AuthFeedbackQuestion
    fields = ['submission_type', 'submission_text']

The problem I forsee is that I will have to do the following in my view that displays the feedback form:

def get_feedback_questions(request, template):
  if request.method == 'POST':
    ...
    if request.user.is_authenticated():
      form = AuthFeedbackQuestionForm(request.POST)
    else:
      form = AnonFeedbackQuestionForm(request.POST)
    if form.is_valid():
      (process form)
      ...
  else:
    if request.user.is_authenticated():
      form = AuthFeedbackQuestionForm(request.POST)
    else:
      form = AnonFeedbackQuestionForm(request.POST)
    ...
  context = {'form': form}
  return render(request, template, context)

Having to repeat these if/then/else blocks to identify which form to use seems rather inelegant. Is there a better, cleaner "Django" way to do this?

Thanks!

score 1 · Accepted Answer · edited May 23 '17 at 11:58

1

I wouldn't subclass your models - if it's an anonymous question you could just include a user attribute as well as an email attribute on one model with blank=True and null=True:

class FeedbackQuestion(models.Model):
    submission_type = ...  (type, i.e. feedback or question)
    submission_text = ...  (actual feedback or question)
    email = models.EmailField(..., blank=True, null=True)
    user = models.ForeignKey(User, related_name="user", blank=True, null=True)
    ...
    class Meta:
        abstract = True

This way you can add either the email for an anonymous user's feedback/question or the user if they're authenticated.

Then I'd combine your forms into one including the email field, but remove the email field depending on if the user is authenticated (see this answer):

def __init__(self, *args, **kwargs):
    self.user = kwargs.pop('user', None)
    super(UserForm, self).__init__(*args, **kwargs)
    if self.user:
        # For logged-in users, email field not necessary
        self.fields.pop('email')
    else:
        # Otherwise, the field needs to be required
        self.fields['email'].required = True

Then you just need to make sure you create the user appropriately as you clean the form's data (e.g., make sure the email address isn't already taken, etc.)

edited May 23 '17 at 11:58

Community

1
1

answered Jul 12 '16 at 18:43

YPCrumble

26,610
23
107
172

Thank you but this solution doesn't work well for two reasons. #1 If the email field can be blank/null, no error is displayed for the email field if an anonymous users submits the form without an email. #2 If you fix #1 by making the email field required and an authenticated user submits the form with errors in other fields, the email field reappears and is flagged as an error since it doesn't have a value. I'm continuing to look for other solutions. – Jim Jul 13 '16 at 04:21
@Robert setting the field as required or not should be at the form level also, not the model level. That should solve your problem number one. Number two shouldn't be an issue either. No matter what, you should only have two possibilities: either the field exists and is required, or it does not exist. See my updated answer, does that make sense? – YPCrumble Jul 13 '16 at 14:48
@YPCrumble could you expand upon why you wouldn't subclass your models? – Bill Brower Jul 13 '16 at 15:46
@Robert Fundamentally because [simple is better than complex](https://www.python.org/dev/peps/pep-0020/). Your model should be for one type of data - in your case it's a `FeedbackQuestion`. (In fact, diving deeper I might split feedback and question into two distinct models, but haven't seen context of your app). What it is not is a `FeedbackQuestionUsertypeModel`. My suggestion is to make the `is_anonymous` parameter based on the presence or not of a user somehow. In fact, another option is to move the `email` straight to the `user` model rather than have it on `FeedbackQuestion`... – YPCrumble Jul 13 '16 at 15:50
...and set a placeholder password so that the `User` saves. You'd have to have an `is_anonymous` flag or something depending on whether you set a default password for the user who just gives an email address (`is_anonymous=True`), or (`is_anonymous=False`) if the user actually created the password/logs in. At the core though, I was more focused on recommending that you not subclass the models because this is breaking apart the `FeedbackQuestion` and subclassing models adds a much higher level of complexity (e.g., needing 3 models & 2 forms) than does the `if...else` statement in the form. – YPCrumble Jul 13 '16 at 15:55
@YPCrumble I'm crediting you with the answer since it prompted me to think about how to solve this problem without using multiple forms. I replaced the user field with a submitter CharField since anonymous users won't have a record in the user table. I added an email field to the form with a hidden submitter field. For anonymous users, I put "Anonymous" in the hidden field and they fill in their email. For authenticated users,I put their username in the hidden field and populate the email field with their email. All form error handling works and I can stick with one model and modelform. – Jim Jul 14 '16 at 17:32
@Robert a hidden field like that can be a security issue...for instance I could adjust the field's value in my HTML and submit as anonymous even though I'm authenticated. Better to check `request.user.is_authenticated()` in your view to add the authenticated or anonymous information. Otherwise makes sense. – YPCrumble Jul 17 '16 at 14:19

How to avoid this redundancy when using Django model inheritance?

1 Answers1