{"action":"created","issue":{"url":"https://api.github.com/repos/tomchristie/django-rest-framework/issues/3253","labels_url":"https://api.github.com/repos/tomchristie/django-rest-framework/issues/3253/labels{/name}","comments_url":"https://api.github.com/repos/tomchristie/django-rest-framework/issues/3253/comments","events_url":"https://api.github.com/repos/tomchristie/django-rest-framework/issues/3253/events","html_url":"https://github.com/tomchristie/django-rest-framework/issues/3253","id":99960757,"number":3253,"title":"perform_*_args / perform_*_kwargs instead of overriding perform_* method","user":{"login":"thenewguy","id":77731,"avatar_url":"https://avatars.githubusercontent.com/u/77731?v=3","gravatar_id":"","url":"https://api.github.com/users/thenewguy","html_url":"https://github.com/thenewguy","followers_url":"https://api.github.com/users/thenewguy/followers","following_url":"https://api.github.com/users/thenewguy/following{/other_user}","gists_url":"https://api.github.com/users/thenewguy/gists{/gist_id}","starred_url":"https://api.github.com/users/thenewguy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thenewguy/subscriptions","organizations_url":"https://api.github.com/users/thenewguy/orgs","repos_url":"https://api.github.com/users/thenewguy/repos","events_url":"https://api.github.com/users/thenewguy/events{/privacy}","received_events_url":"https://api.github.com/users/thenewguy/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-08-10T03:15:17Z","updated_at":"2015-08-10T16:22:08Z","closed_at":"2015-08-10T06:46:00Z","body":"It would be much more convenient for mixin based view construction if the perform_create/perform_update/etc methods allowed subclasses to build up the args/kwargs passed to save()\r\n\r\nSomething like the following instead of https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/mixins.py#L25:\r\n```\r\nclass CreateModelMixin(object):\r\n    ...\r\n    def perform_create(self, serializer):\r\n        args = self.perform_create_args(serializer)\r\n        kwargs = self.perform_create_kwargs(serializer)\r\n        serializer.save(*args, **kwargs)\r\n\r\n    def perform_create_args(self, serializer):\r\n        return []\r\n\r\n    def perform_create_kwargs(self, serializer):\r\n        return {}\r\n    ...\r\n```\r\nfor each of the relevant methods"},"comment":{"url":"https://api.github.com/repos/tomchristie/django-rest-framework/issues/comments/129516411","html_url":"https://github.com/tomchristie/django-rest-framework/issues/3253#issuecomment-129516411","issue_url":"https://api.github.com/repos/tomchristie/django-rest-framework/issues/3253","id":129516411,"user":{"login":"thenewguy","id":77731,"avatar_url":"https://avatars.githubusercontent.com/u/77731?v=3","gravatar_id":"","url":"https://api.github.com/users/thenewguy","html_url":"https://github.com/thenewguy","followers_url":"https://api.github.com/users/thenewguy/followers","following_url":"https://api.github.com/users/thenewguy/following{/other_user}","gists_url":"https://api.github.com/users/thenewguy/gists{/gist_id}","starred_url":"https://api.github.com/users/thenewguy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thenewguy/subscriptions","organizations_url":"https://api.github.com/users/thenewguy/orgs","repos_url":"https://api.github.com/users/thenewguy/repos","events_url":"https://api.github.com/users/thenewguy/events{/privacy}","received_events_url":"https://api.github.com/users/thenewguy/received_events","type":"User","site_admin":false},"created_at":"2015-08-10T16:22:08Z","updated_at":"2015-08-10T16:22:08Z","body":"@tomchristie  I think it would be generally useful so I wanted to put together a use case before the issue is closed for good.\r\n\r\nSay you have the following two abstract model mixins (or more in actual practice to facilitate ease of testing and separation of concerns):\r\n```\r\nclass ClientMetadataMixin(APIAbstractBaseModel):\r\n    class Meta:\r\n        abstract = True\r\n    ...\r\n    raw_user_agent = models.TextField(blank=True)\r\n    ...\r\n\r\nclass OwnedModelMixin(APIAbstractBaseModel):\r\n    class Meta:\r\n        abstract = True\r\n    ...\r\n    owner = models.ForeignKey(settings.AUTH_USER_MODEL, null=True, blank=True)\r\n    ...\r\n```\r\n\r\nThen say these abstract models are inherited by 10 models which are controlled by viewsets and each of the associated viewsets also have conditional logic for `perform_*` methods.\r\n\r\nNow each viewset needs to write the following:\r\n```\r\nclass FooModelViewSet(viewsets.ModelViewSet):\r\n   def perform_create(self, serializer):\r\n       # viewset specific logic\r\n       ....\r\n       foo_field = \"bar\"\r\n       ....\r\n\r\n       # duplicate inherited logic from simple abstract class #1\r\n       if self.request.user.is_anonymous():\r\n           user = None\r\n       else:\r\n           user = self.request.user\r\n\r\n       # duplicate inherited logic from simple abstract class #2\r\n       raw_user_agent = request.META.get(\"HTTP_USER_AGENT\", \"\")\r\n\r\n       # etc\r\n\r\n       serializer.save(foo_field=foo_field,owner=user, raw_user_agent=raw_user_agent)\r\n\r\n   def perform_update(self, serializer):\r\n       # viewset specific logic\r\n       ....\r\n       foo_field = \"bar\"\r\n       ....\r\n\r\n       # duplicate inherited logic from simple abstract class #1\r\n       if self.request.user.is_anonymous():\r\n           user = None\r\n       else:\r\n           user = self.request.user\r\n\r\n       # duplicate inherited logic from simple abstract class #2\r\n       raw_user_agent = request.META.get(\"HTTP_USER_AGENT\", \"\")\r\n\r\n       # etc\r\n\r\n\r\n       serializer.save(foo_field=foo_field,owner=user, raw_user_agent=raw_user_agent)\r\n```\r\n\r\nThis makes the view prone to copy/paste or omission errors when the logic changes.  It also isn't very DRY.  The DRY part can be mitigated by helper functions but it seems more complicated than the following:\r\n\r\n```\r\nclass FooModelViewSet(mixin1, mixin2, viewsets.ModelViewSet):\r\n     def perform_create_kwargs(self, serializer):\r\n          kwargs = super(FooModelViewSet, self).perform_create_kwargs(serializer)\r\n          kwargs['foo_field'] = \"bar\"\r\n          return kwargs\r\n\r\n     def perform_update_kwargs(self, serializer):\r\n          kwargs = super(FooModelViewSet, self).perform_update_kwargs(serializer)\r\n          kwargs['foo_field'] = \"bar\"\r\n          return kwargs\r\n\r\n```\r\n\r\nThe perform_*_kwarg methods could be condensed into one method by passing along the operation type.\r\n\r\nThen something like the following could be done:\r\n```\r\nclass FooModelViewSet(mixin1, mixin2, viewsets.ModelViewSet):\r\n     def perform_operation_kwargs(self, serializer, operation):\r\n          kwargs = super(FooModelViewSet, self).perform_operation_kwargs(serializer, operation)\r\n          if operation in (\"create\", \"update\"):\r\n                kwargs['foo_field'] = \"bar\"\r\n          return kwargs\r\n```\r\n\r\n\r\nThe benefits offered by this construction seem worthy of the minimal complications that are introduced.\r\n\r\n"}}