Skip to content

Commit

Permalink
FIX: ModelViews can have Resources whose models have unique fields.
Browse files Browse the repository at this point in the history
ReadModelMixin and UpdateModelMixin store model instance as a property. This allows ModelResource to bind the ModelForm using the model instance making the form validate the input data against the model instance and not a brand new instance. When the latter happened and the model used unique fields, the form validation failed whenever a PUT was maintaining the previuos value of the unique field.
  • Loading branch information
fzunino committed Jul 1, 2011
1 parent 8bafa01 commit 60cd536
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 31 deletions.
20 changes: 10 additions & 10 deletions djangorestframework/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,17 +491,17 @@ def get(self, request, *args, **kwargs):
try:
if args:
# If we have any none kwargs then assume the last represents the primrary key
instance = model.objects.get(pk=args[-1], **kwargs)
self.model_instance = model.objects.get(pk=args[-1], **kwargs)
else:
# Otherwise assume the kwargs uniquely identify the model
filtered_keywords = kwargs.copy()
if BaseRenderer._FORMAT_QUERY_PARAM in filtered_keywords:
del filtered_keywords[BaseRenderer._FORMAT_QUERY_PARAM]
instance = model.objects.get(**filtered_keywords)
self.model_instance = model.objects.get(**filtered_keywords)
except model.DoesNotExist:
raise ErrorResponse(status.HTTP_404_NOT_FOUND)

return instance
return self.model_instance


class CreateModelMixin(object):
Expand Down Expand Up @@ -540,19 +540,19 @@ def put(self, request, *args, **kwargs):
try:
if args:
# If we have any none kwargs then assume the last represents the primrary key
instance = model.objects.get(pk=args[-1], **kwargs)
self.model_instance = model.objects.get(pk=args[-1], **kwargs)
else:
# Otherwise assume the kwargs uniquely identify the model
instance = model.objects.get(**kwargs)
self.model_instance = model.objects.get(**kwargs)

for (key, val) in self.CONTENT.items():
setattr(instance, key, val)
setattr(self.model_instance, key, val)
except model.DoesNotExist:
instance = model(**self.CONTENT)
instance.save()
self.model_instance = model(**self.CONTENT)
self.model_instance.save()

instance.save()
return instance
self.model_instance.save()
return self.model_instance


class DeleteModelMixin(object):
Expand Down
50 changes: 29 additions & 21 deletions djangorestframework/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,12 @@ def _validate(self, data, files, allowed_extra_fields=(), fake_data=None):

# Return HTTP 400 response (BAD REQUEST)
raise ErrorResponse(400, detail)


def get_bound_form(self, data=None, files=None, method=None):

def get_form_class(self, method=None):
"""
Given some content return a Django form bound to that content.
If form validation is turned off (:attr:`form` class attribute is :const:`None`) then returns :const:`None`.
Returns the form class used to validate this resource.
"""

# A form on the view overrides a form on the resource.
form = getattr(self.view, 'form', None) or self.form

Expand All @@ -200,6 +198,16 @@ def get_bound_form(self, data=None, files=None, method=None):
form = getattr(self, '%s_form' % method.lower(), form)
form = getattr(self.view, '%s_form' % method.lower(), form)

return form


def get_bound_form(self, data=None, files=None, method=None):
"""
Given some content return a Django form bound to that content.
If form validation is turned off (:attr:`form` class attribute is :const:`None`) then returns :const:`None`.
"""
form = self.get_form_class(method)

if not form:
return None

Expand Down Expand Up @@ -306,31 +314,31 @@ def get_bound_form(self, data=None, files=None, method=None):
If the :attr:`form` class attribute has been explicitly set then that class will be used
to create the Form, otherwise the model will be used to create a ModelForm.
"""
form = self.get_form_class(method)

form = super(ModelResource, self).get_bound_form(data, files, method=method)

# Use an explict Form if it exists
if form:
return form

elif self.model:
if not form and self.model:
# Fall back to ModelForm which we create on the fly
class OnTheFlyModelForm(forms.ModelForm):
class Meta:
model = self.model
#fields = tuple(self._model_fields_set)

# Instantiate the ModelForm as appropriate
if data and isinstance(data, models.Model):
# Bound to an existing model instance
return OnTheFlyModelForm(instance=content)
elif data is not None:
return OnTheFlyModelForm(data, files)
return OnTheFlyModelForm()
form = OnTheFlyModelForm

# Both form and model not set? Okay bruv, whatevs...
return None

if not form:
return None

# Instantiate the ModelForm as appropriate
if data is not None or files is not None:
if issubclass(form, forms.ModelForm) and hasattr(self.view, 'model_instance'):
# Bound to an existing model instance
return form(data, files, instance=self.view.model_instance)
else:
return form(data, files)

return form()


def url(self, instance):
"""
Expand Down

0 comments on commit 60cd536

Please sign in to comment.