1

I have created a form to delete objects but I need to check that the user that want to delete the object is the user who created that object. I would like to check it in the form (as well as in the view) as it is a business constraint. Where is the best place to check that, in the init, delete or clean method?

class DeleteFooForm(forms.ModelForm):
    class Meta:
        model = Foo
        fields = []

        def __init__(self, user, *args, **kwargs):
            super(DeleteFooForm, self).__init__(*args, **kwargs)
            self.user = user

        def delete(self):
            if self.user is not self.instance.user:
                raise PermissionDenied("Wrong user")        

            self.instance.delete()

            # more actions, send email, etc.
Ivan
  • 1,477
  • 3
  • 18
  • 36
  • At the form you don't have access to the request object, so it is the wrong place to validate permissions. Do it in the view and use the messages framework to flash the error to the user. – Paulo Scardine Sep 11 '14 at 13:02
  • Personally I would recommend doing it in the `clean()` method since it's a form constraint which makes the form invalid (in a way). – Wolph Sep 11 '14 at 13:04
  • @PauloScardine, I totally disagree. The form is *absolutely* the right place to do validation: that's what it's mainly for. There's no reason to do validation in two separate places. – Daniel Roseman Sep 11 '14 at 13:22
  • @DanielRoseman: May be you are right, but unfortunately you just can't do that. The reason is that inside the form methods, you don't have the information about who is logged in. Without that information it is impossible to validate ownership. – Paulo Scardine Sep 11 '14 at 13:45
  • 2
    @PauloScardine I pass the user information in the form constructor. – Ivan Sep 11 '14 at 14:34
  • @Ivan: you do, but IMHO it is not the idiomatic way to use forms. Why the django developers chose not to pass the whole request object in the forms constructor is beyond me, but it is clear they want to keep concerns separated. – Paulo Scardine Sep 11 '14 at 17:30

1 Answers1

1

Really this should happen in the clean method: that's the place for validation. The main reason to do it there is you can then follow the normal method for validation, which is to raise a ValidationError which will be caught by the form API and presented as an error.

You certainly don't want to do it in __init__, as that will then raise an error even when the form is originally displayed, and delete is too late.

Daniel Roseman
  • 588,541
  • 66
  • 880
  • 895
  • Note that the form doesn't have any data associated with it, so the `clean` method never will be executed (unless I execute it directly). I don't display anything I just use a form for validation issues (csrf and user) and because I could want to add some extra actions when a object is deleted. – Ivan Sep 11 '14 at 14:49
  • @Ivan: the way I do it is having a delete URL that takes the object ID from the path (like `/foo/123/delete`), at the GET request I show a simple yes/no confirm form (not a ModelForm) and on the POST I call foo.delete() if the the user chooses "yes". If the user is not the object owner, I show a message and a link to go back instead of the form. – Paulo Scardine Sep 11 '14 at 17:39
  • If this was the intended way to delete objects, there would be a ModelDeleteForm in Django. – Paulo Scardine Sep 11 '14 at 17:57
  • @PauloScardine Then you are deleting the object and executing any other action related with that command (validate user, write a log message, ...) in the view (API), coupling business logic with API. I think that **delete** (as **save** or **update**), is a command that shouldn't be implemented in the view and a form is the best place to implement it as indicated [here](http://stackoverflow.com/questions/12578908/separation-of-business-logic-and-data-access-in-django). Of course this is a matter of opinion. – Ivan Sep 11 '14 at 22:33
  • @Ivan: your preference is matter of opinion, it being not idiomatic in Django is a verifiable fact: there is not a single example of your method on Django documentation and it is not used in the Django `admin` app. :-) – Paulo Scardine Sep 11 '14 at 23:36