Confusing exception message when using values_list with relations

[Jonas Obrist]

Assuming following models:

from django.db import models


class Reporter(models.Model):
    first_name = models.CharField(max_length=30)
    last_name = models.CharField(max_length=30)
    email = models.EmailField()

    def __unicode__(self):
        return u"%s %s" % (self.first_name, self.last_name)

class Article(models.Model):
    headline = models.CharField(max_length=100)
    pub_date = models.DateField()
    reporter = models.ForeignKey(Reporter)

    def __unicode__(self):
        return self.headline

    class Meta:
        ordering = ('headline',)

Running this query:

Article.objects.values_list('reporter__notafield')

Will report this error message:

"Cannot resolve keyword 'reporter__notafield' into field. Choices are: headline, id, pub_date, reporter"

This might look as if values_list only allows fields on a model and not relations. I propose the error message to change to something like "Cannot resolve keyword 'notafield' into field. Choices are 'article', 'email', 'first_name', 'id', 'last_name'".

I've attached a simple patch with a test case.

[Jonas Obrist]

test case

[antoviaque]

Cf attached file for a first attempt at fixing this. Since I'm new to django development, please be gentle in the review : )

One thing that is unsatisfactory about this patch is that it doesn't properly handle the case where the QuerySet has an extra(select=...) and a lookup error on the first name of a field_names containing a LOOKUP_SEP. In such a case, it should correctly show the list of available field names on the right model, but won't show the available extra() parameters, which could in turn be confusing.

However, the only way I could see to solve this was to mingle with the FieldError() exception, to include the model & field name on which the exception occurs. But I wasn't too sure about this, and it seems a little overkill.

Anyway - if you see a better way to solve this, let me know, I'll be happy to implement it differently.

[Jonas Obrist]

I would say better error messages for extra could be a separate ticket. The supplied patch applied cleanly to trunk and tests passed. Also manually checked that the output is what I would expect. LGTM.

[Anssi Kääriäinen]

I am looking into committing this one. Let me see if I understand what is happening currently:

1. The part 2 of the lookup raises a FieldError
2. That FieldError is caught at level 1
3. A level 1 error message is generated, so you see the reporter__nonexisting part for the original model, not the level 2 part for the reporter model.

The patch changes the logic in a way that the level 1 message is let through. Is this correct?

The only question I have is if there is some easy way to show the lookup path, that is how we ended up to the nonexisting lookup. Something like 'the whole lookup was reporter__nonexisting', or "the lookup prefix was "reporter__". If there is no easy way I can live with the above patch, if there is an easy way it would be a nice addition.

[Anssi Kääriäinen <akaariai@…>]

In [fcad6c48f07bdc6346c065849a87f0f02afb6f94]:

Fixed #17497 -- Corrected FieldError message in add_fields()

The erroneous message was user visible in values_list() calls.

Thanks to ojii for report and review, and to antoviaque for the patch.