Re: ModelChoiceField's clean() uses default manager instead of query set
> Sorry, perhaps I don't quite understand how this is different... > unless you are rendering the field widget manually and somehow adding > extra choices that aren't in the query set passed to the > Model*ChoiceField? There would be one extra choice - the previously selected, and no longer valid, related field! Perhaps I can be clearer with the following example. Please ignore the details of getting/displaying the form - this is only to show you the issue with related fields and the choice validation: class Employee(models.Model): first_name ... last_name ... active ... class Activity(models.Model): employee = models.ForeingnKey(Employee, limit_choices_to={'active__exact':True}) type = models.ChoiceField(...) duration = models.PositiveSmallIntegerField() notes = models.TextField() # again, ignore the details of getting/displaying the form. Just using the admin will show the potential problem 1) Create new active employee, employee1 2) Create new activity object, activity1 linked to employee1 3) Find employee in intimate relationship with office equipment, fire employee 4) Edit employee1 so that active=False 5) Edit activity1 (the original employee1 item is still displayed & selected) to add some notes ("Remember to buy new furniture") & save. Blam! Can't save because employee is not active, if using the "active=False" manager. (Currently however, admin would be using the default manager so it would allow you to save.) So hopefully you can see why I think it is a good idea to restrict the choices, for convenience, but not the validation part of the ModelChoiceField. This doesn't solve all problems - like adding "activity2" after employee1 is fired - but at least it keeps the original objects usable. Now if you really wanted to prevent somebody from cheating the selection, add the extra validation - but again, this would be easier as a method (e.g. if superuser, use default manager for validation: else if editing & the same, return: else use custom manager for validation). > I guess my take on it is that if you wanted the user to be able to > choose from any object you would just pass the default manager as the > query set instead of passing in a filtered query set in the first > place. Because the number of unfiltered items might be unreasonable large, or at least annoyingly large, in a select field. > As I said before I'm not too fussed if it's decided that this should > not apply to the standard Model*ChoiceField classes, but I felt that > it made sense. I'll shut up now :-) Well it is a judgement call, not black and white. I've just been in the situation I've mentioned (except maybe for the office furniture part!) and I know it would be a major pain to deal with validation that prevents legitimate use. -rob --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: ModelChoiceField's clean() uses default manager instead of query set
On Aug 9, 5:49 am, oggie rob <[EMAIL PROTECTED]> wrote: > > That could work. The main point of this thread was to see if there are > > use cases for using the default manager for validation instead of the > > query set because I couldn't really think of any - but I'm sure there > > could well be some. > > Like the one I explained? :) Sorry, perhaps I don't quite understand how this is different... unless you are rendering the field widget manually and somehow adding extra choices that aren't in the query set passed to the Model*ChoiceField? > I think the strongest argument against using adding an extra > "validation queryset" argument is that it doesn't give the flexibility > of programmatically validating data. A function works much better in > this respect. And as for the default behaviour, I've put forth my > thoughts on it and I think it is more commonly a problem than the > security issue that was suggested (since you probably won't usually > construct your forms to allow a user to have any say in their own > access). I guess my take on it is that if you wanted the user to be able to choose from any object you would just pass the default manager as the query set instead of passing in a filtered query set in the first place. If you are passing in a query set that has some objects excluded, the user won't be able to choose them anyway since they won't be presented in the list of choices for the field[1]. I figured the validation should work in the same way and restrict the submitted choices to the choices displayed in the form field. > Not incidentally, this is what the "validator_list" has been used for. > However I'm not sure if this will remain in place since oldforms is > going away... but you should be able to transition it easily if > necessary. I could add a clean_FIELDNAME method to the form to validate that the choice is actually one contained in the query set and not just the default manager, but this means an extra lookup since it will be done after the field's clean() method is called (which does the original lookup using the default manager). As I said before I'm not too fussed if it's decided that this should not apply to the standard Model*ChoiceField classes, but I felt that it made sense. I'll shut up now :-) Cheers, Nick [1] Unless a clever user hacked up a POST request which included the excluded items. Validation would pass, which could be considered a security issue. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: ModelChoiceField's clean() uses default manager instead of query set
> That could work. The main point of this thread was to see if there are > use cases for using the default manager for validation instead of the > query set because I couldn't really think of any - but I'm sure there > could well be some. Like the one I explained? :) I think the strongest argument against using adding an extra "validation queryset" argument is that it doesn't give the flexibility of programmatically validating data. A function works much better in this respect. And as for the default behaviour, I've put forth my thoughts on it and I think it is more commonly a problem than the security issue that was suggested (since you probably won't usually construct your forms to allow a user to have any say in their own access). Not incidentally, this is what the "validator_list" has been used for. However I'm not sure if this will remain in place since oldforms is going away... but you should be able to transition it easily if necessary. -rob --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: ModelChoiceField's clean() uses default manager instead of query set
On Aug 7, 6:55 pm, David Danier <[EMAIL PROTECTED]> wrote: > > However, I'm a little confused now. I > > thought the problem was only validation, and not the actual data that > > was added to the choice list. > > The problem is, that validation does not use the actual data of the > choice list. > > I think doing so may even become a security issue, as people may guess > valid IDs that they should not use (for example if you use > user_profile.some_relation as the queryset). Well that was the point I was trying to make, but I probably didn't word myself as clearly as I should have :-) Validation is done using a (possibly) different set of data than the choices displayed to the user. > Perhaps it could be changed to only allow choices in the queryset, but > an option is added to ModelChoiceField to use the default manager? That could work. The main point of this thread was to see if there are use cases for using the default manager for validation instead of the query set because I couldn't really think of any - but I'm sure there could well be some. Cheers, Nick --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: ModelChoiceField's clean() uses default manager instead of query set
> However, I'm a little confused now. I > thought the problem was only validation, and not the actual data that > was added to the choice list. The problem is, that validation does not use the actual data of the choice list. I think doing so may even become a security issue, as people may guess valid IDs that they should not use (for example if you use user_profile.some_relation as the queryset). Perhaps it could be changed to only allow choices in the queryset, but an option is added to ModelChoiceField to use the default manager? Greetings, David Danier --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: ModelChoiceField's clean() uses default manager instead of query set
> Hmm, I see what you're saying but (using your example) if the employee > is no longer active, wouldn't that mean the field *should* raise a > validation error? If the choices are meant to be limited to only > employees which are active shouldn't the validation prevent the user > from choosing an option which has been invalidated since the form was > rendered? Not in my case. Among other things, I'm tracking employee's time. So just because they're no longer active doesn't mean the work they did previously is invalid. Although making changes to these records is not common, sometimes some details need to be updated and I don't want to be prevented. I see the choices interface as a convenience / performance tool, but not as a validation tool. > At least that's the reason I came across this in the first place. For > some background: The user needs to choose from a list of objects, and > these objects expire over time. I wanted validation to catch the case > of a selected choice expiring and no longer being valid. I have > created a custom ModelChoiceField which works using the query set > instead of the default manager and that suits my purposes fine, I just > thought I would raise it here to see if this should apply in general > or not. So, this would be a problem if you were using the form for updating records, but reading between the lines it looks like you are using the form only for adding records. However, I'm a little confused now. I thought the problem was only validation, and not the actual data that was added to the choice list. Perhaps its too late for me to be commenting (like usual)... I'll let others speak now :) -rob --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: ModelChoiceField's clean() uses default manager instead of query set
On Aug 7, 4:14 pm, oggie rob <[EMAIL PROTECTED]> wrote: > I'm not sure of the intent, but something to keep in mind is that if > you have previously-valid information and wish to save it again, it > may become invalid. > For example, I have a list of "active" employees for selection in the > filtered choice field. If I open up some object and hit "Save" after > the employee is no longer active, this would incorrectly raise an > error. I would imagine this avoids it. Hmm, I see what you're saying but (using your example) if the employee is no longer active, wouldn't that mean the field *should* raise a validation error? If the choices are meant to be limited to only employees which are active shouldn't the validation prevent the user from choosing an option which has been invalidated since the form was rendered? At least that's the reason I came across this in the first place. For some background: The user needs to choose from a list of objects, and these objects expire over time. I wanted validation to catch the case of a selected choice expiring and no longer being valid. I have created a custom ModelChoiceField which works using the query set instead of the default manager and that suits my purposes fine, I just thought I would raise it here to see if this should apply in general or not. Cheers, Nick > -rob > > On Aug 6, 10:00 pm, Nick <[EMAIL PROTECTED]> wrote: > > > The clean() methods in both ModelChoiceField and > > ModelMultipleChoiceField use a block similar to the following in order > > to validate the selected choice: > > > try: > > value = self.queryset.model._default_manager.get(pk=value) > > except self.queryset.model.DoesNotExist: > > raise ValidationError(ugettext(u'Select a valid choice. That > > choice is not one of the available choices.')) > > > Is there any reason that shouldn't simply be > > self.queryset.get(pk=value) ? > > > I came across this in a project when I was trying to work out why it > > was allowing choices that I had explicitly filtered out of the > > queryset - and of course this explains it. Is there a reason that the > > default manager should be used instead of just the original queryset? --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: ModelChoiceField's clean() uses default manager instead of query set
I'm not sure of the intent, but something to keep in mind is that if you have previously-valid information and wish to save it again, it may become invalid. For example, I have a list of "active" employees for selection in the filtered choice field. If I open up some object and hit "Save" after the employee is no longer active, this would incorrectly raise an error. I would imagine this avoids it. -rob On Aug 6, 10:00 pm, Nick <[EMAIL PROTECTED]> wrote: > The clean() methods in both ModelChoiceField and > ModelMultipleChoiceField use a block similar to the following in order > to validate the selected choice: > > try: > value = self.queryset.model._default_manager.get(pk=value) > except self.queryset.model.DoesNotExist: > raise ValidationError(ugettext(u'Select a valid choice. That > choice is not one of the available choices.')) > > Is there any reason that shouldn't simply be > self.queryset.get(pk=value) ? > > I came across this in a project when I was trying to work out why it > was allowing choices that I had explicitly filtered out of the > queryset - and of course this explains it. Is there a reason that the > default manager should be used instead of just the original queryset? --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
ModelChoiceField's clean() uses default manager instead of query set
The clean() methods in both ModelChoiceField and ModelMultipleChoiceField use a block similar to the following in order to validate the selected choice: try: value = self.queryset.model._default_manager.get(pk=value) except self.queryset.model.DoesNotExist: raise ValidationError(ugettext(u'Select a valid choice. That choice is not one of the available choices.')) Is there any reason that shouldn't simply be self.queryset.get(pk=value) ? I came across this in a project when I was trying to work out why it was allowing choices that I had explicitly filtered out of the queryset - and of course this explains it. Is there a reason that the default manager should be used instead of just the original queryset? --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---