to_field lost when adding via raw_id_fields

[Collin Anderson]

When using to_field with raw_id_fields, you get a popup to select an object to connect the ForeignKey to. This correctly handles to_field.

However, you also have the option of adding a new object from the popup. When you add the new object, the popup is closed the the primary key is sent back to the raw_id widget.

As far as I can tell, this has never worked correctly in django. (at least since 1.2)

I have a simple django app that can be used to reproduce the problem:
​https://github.com/collinanderson/to_field/commit/0b913d4df7c8ab699aca2886ee619313373f4c8c

[Collin Anderson]

Basically, TO_FIELD_VAR really needs to follow IS_POPUP_VAR everywhere it goes. Whenever it's a popup, there's always the possibility that there's a to_field.

pull request (against 1.6.x, sorry):
​https://github.com/django/django/pull/1417

[loic84]

I'm not sure I understand.

I tried your test project: clicked on the magnifying glass, clicked on "Add user", filled out the form, saved, then the popup closed and the PK was added to the raw_id field. Isn't that the expected behavior?

[Collin Anderson]

I should be more clear. my field looks like this: user = models.ForeignKey('auth.User', to_field='username').

The to_field is the field that is referenced on the other model. Normally it is the primary key, but Django provides the option to override it. Really, any unique=True field should work in theory.

So the expected behavior is that when the popup is closed I get back the username instead of the PK, because that's what to_field is.

Again, it works correctly when selecting an existing object (username is returned), but doesn't not work when adding. (pk is returned).

[loic84]

I think it's a reasonable request, marking it as accepted.

I didn't look into the rest of the patch into detail, but if this GET param is used on add_view and change_view we need to rename it (_tofield maybe?) otherwise it might shadow a field name.

#20288 and [7462a78c1b] may help on how to approach such rename.

[Collin Anderson]

Yeah, I was thinking about that when I was creating the pull request. I'm personally not that worried about it because this is such a corner case as it is. I'm apparently the first one to ever notice the raw_id + to_field + adding case, and we would need a raw_id + to_field + adding + having a field named "t" before there would be an issue.

Though, yes, it's possible and it will only be harder to change in the future. And deprecating/changing this during the same release as _popup would make a lot of sense.

Actually, here's a proposal. How about instead of having a separate "t" variable, we simply have _popup=username or _popup=pk (or rename _popup again before 1.6 comes out). I think that would actually simplify the code quite a bit because it's one less thing we need to keep passing around. Currently is_popup is a boolean, and the "t" var only makes sense when _popup is true, so if we're changing things anyway, why not just combine the two into one?

[Collin Anderson]

irc conversation:
<loic84> personally I'd keep the 2 variables separate as it's more explicit
<loic84> I think _tofield works well, it makes it easy to grep on the code to find out what it is

[Peter Sheats]

I added a selenium test for this issue and added a little to CollinAnderson's solution.

​https://github.com/django/django/pull/1585

[Julien Phalip <jphalip@…>]

In 55a11683f7b094ae4fd0b9fa030d18a12657ba98:

Fixed #20836 -- Ensure that the ForeignKey's to_field attribute is properly considered by the admin's interface when creating related objects.
Many thanks to Collin Anderson for the report and patch and to Peter Sheats for the test.