Checks fail on non-swappable ForeignKeys when the related model is swapped out

[Ben Davis]

The documentation states that setting swappable=False on a ForeignKey allows you to reference a swappable model even if that model is swapped out. Unfortunately this doesn't work because RelatedField's check function will look for that model in apps.get_models(), which doesn't include swapped models by default.

If a ForeignKey references a swapped out model and has swappable=False, shouldn't that model be loaded? It seems to me that swapped=False will never work because of this.

[Tim Graham]

Haven't completely verified this, but I don't see any tests for swappable=False in c9de1b4a55713ad1557f8c40621226253038f665.

[Ben Davis]

For reference, the swappable keyword was originally proposed here: ​https://groups.google.com/forum/#!msg/django-developers/arSS604xc6U/24j1vKtlESoJ

Although, it was not implemented as proposed. Russel suggested that the swappable keyword default to None, as we can safely assume that when the "to" model is not a string, it should point to the original. If to is a string that points to a swappable model, we would assume that it should point to the swapped model. Any other case would be an edge case, and can be controlled explicitly by setting the keyword to either True or False.

[Ben Davis]

This is my initial attempt to fix this. It passes current tests but it still needs tests for swappable=None/True/False (which I can do soon): ​https://github.com/bendavis78/django/commit/cc70dd04fcc2a36592a6896b253c6adda8651cf4

The current change fixes the issue for startup checks, however I'm not 100% sure what should be done with migrations, if anything. Migrations will still fail if a foreign key points to a swappable model that has been swapped out (regardless of whether or not its a string). It may be that it should stay this way but I'd probably need someone to chime in on that. Not sure if I'm on the right track here, or even if this bug would realistically effect anyone else (I do have a workaround for my use case).

[Andrew Godwin]

Your parsing of what swappable means is incorrect; it's not for a swappable model, it's for a model that might be swapped in in place of that model. It won't allow you to access models that are swapped out, and that _should_ fail the checks system immediately.

I'm bumping this down from release blocker as it's just a documentation clarification and a missing test for the false side IMO (we didn't implement Russ' plan exactly; from what I remember, the None option wasn't necessary in the end due to the limited options migrations had dealing with it).

[Ben Davis]

Ah, so if I understand correctly, the meaning of swappable=False is "System check should fail if the referenced model is swapped out", and swappable=True (the default) means "Allow the referenced model to be swapped out". Is that correct?

[Andrew Godwin]

Not quite; first of all, this is designed for FKs that point to models that might be the swap-in replacement - e.g. myapp.User, not django.contrib.auth.User.

swappable=True means "if the other end of this foreignkey is currently the value of AUTH_USER_MODEL then make this FK point to settings.AUTH_USER_MODEL

swappable=False means "even if the other end of this is the current value of AUTH_USER_MODEL, don't do a swappable link, do it directly, as I really want to always link to myapp.User, never to the generic user model". It's meant for companion models that are definitely only for a certain user model.

[Andrew Godwin <andrew@…>]

In 6d504562f5f27698a668759254101290b82e89b9:

Fixed #22534: Reinforce swappable documentation

[Andrew Godwin <andrew@…>]

In ffa9e60638bc1447cba9ddef4585b70267c435f7:

[1.7.x] Fixed #22534: Reinforce swappable documentation