ForeignKey.validate() should validate using the base manager.

[Jon Dufresne]

ForeignKey.validate() should validate using the base manager instead of the default manager.

Consider the models:

class ArticleManager(models.Manage):
    def get_queryset(self):
        qs = super().get_queryset()
        return qs.filter(archived=False)

class Article(models.Model):
    title = models.CharField(max_length=100)
    archived = models.BooleanField(default=False)

    # Don't include archived articles by default.
    objects = ArticleManager()

class FavoriteAricles(models.Model):
    article = models.ForeignKey(Article, on_delete=models.CASCADE)

In the example, now consider a form that allows users to pick a favorite article including archived articles.

class FavoriteAriclesForm(forms.ModelForm):
    class Meta:
        model = FavoriteArticle
        fields = '__all__'

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        # Use the base manager instead of the default manager to allow archived articles.
        self.fields['article'].queryset = Article._base_manager.all()

The above form will never validate as True when a user selects an archived article. This is because the ForeignKey validation always uses _default_manager instead of _base_manager. The user facing error message is "article instance with id 123 does not exist." (quite confusing to typical users). The code for this validation is here:

​https://github.com/django/django/blob/94f63b926fd32d7a7b6e2591ef72aa8f040f25cc/django/db/models/fields/related.py#L917-L919

The FavoriteAriclesForm is specifically designed to use a different manager, but the ForeignKey validation makes this difficult.

In this example scenario, it is not acceptable to change the model's default manager as the default should avoid archived articles in other typical scenarios.

Suggested solution: the ForeignKey validation should use the _base_manager instead which does not include the default filters.

[Jon Dufresne]

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

[Carlton Gibson]

OK, yes, let's provisionally Accept this: I think it's probably correct.
(At this level we're avoiding DB errors, not business logic errors, like "was this archived", which properly belong to the form layer...)

[Carlton Gibson]

I take it the change here is ORM rather than forms per se.

[Jon Dufresne]

I take it the change here is ORM rather than forms per se.

Makes sense. This is a change to model validation to allow the form validation to work.

[Carlton Gibson <carlton@…>]

In e13cfc6d:

Fixed #31596 -- Changed ForeignKey.validate() to use the base manager.

[Damian Nardelli]

Hello. The documentation for v2.2 (​https://docs.djangoproject.com/en/2.2/topics/db/managers/#using-managers-for-related-object-access) is stating that the base manager will be used in that version too instead of the default manager. We just saw that this fix has been released 12 days ago for v3.2.5, we were wondering when it'll be released for v2.2? Thanks!

[Mariusz Felisiak]

Replying to Damian Nardelli:

Hello. The documentation for v2.2 (​https://docs.djangoproject.com/en/2.2/topics/db/managers/#using-managers-for-related-object-access) is stating that the base manager will be used in that version too instead of the default manager. We just saw that this fix has been released 12 days ago for v3.2.5, we were wondering when it'll be released for v2.2? Thanks!

It is not a regression, per our backporting policy this means it doesn't qualify for a backport to 2.2.x (see ​Django’s release process for more details). Moreover Django 2.2 is in extended support so it doesn't get bugfixes (except security issues) anymore.

[Iván Brizuela]

This change results in a breaking change or a “gotcha” for validations that depended on proxy model classes.

See this pseudocode setup:

class AccountDocument(models.Model):
    INVOICE_TYPE = 'I'
    PAYMENT_TYPE = 'P'
    DOCUMENT_TYPES = [
        (INVOICE_TYPE, 'Invoice'),
        (PAYMENT_TYPE, 'Payment'),
    ]
    document_type = models.CharField(
        max_length=1,
        choices=DOCUMENT_TYPES,
        default=INVOICE_TYPE)
    account: str
    total: Decimal

class Invoice(AccountDocument):
    # On insert / update guarantee that document_type == INVOICE_TYPE

    objects: Custom manager filters AccountDocument by document_type == INVOICE_TYPE

    class Meta:
        proxy = True

class Payment(AccountDocument):
    # On insert / update guarantee that document_type == PAYMENT_TYPE

    objects: Custom manager filters AccountDocument by document_type == PAYMENT_TYPE

    class Meta:
        proxy = True

class Receipt(models.Model):
    invoice: Invoice
    payment: Payment
    amount: Decimal

When using this setup:

invoice:Invoice = Invoice(document_type=INVOICE_TYPE, account='xyz', total=Decimal('100'))
payment:Payment = Payment(document_type=PAYMENT_TYPE, account='xyz', total=Decimal('50'))

invoice.save()
payment.save()

# Setting error condition to be tested: invalid account documents are assigned as receipt properties
receipt = Receipt(
    invoice=payment, # invalid!
    payment=invoice, # invalid!
    amount=Decimal('50'))

receipt.full_clean()

Expected (Before 3.2):
ValidationError is raised, as neither invoice:Payment nor payment:Invoice exist when looked up by their default manager.

Actual (Since 3.2):
No exception is raised, which is unexpected given the field definitions.

Both the example given to request this change and the one I am providing, stretch the base idea behind proxy models: “only change the Python behavior of a model”.

Semantically, I don't think an ArticleManager should filter by 'archived' status. It would be preferred to have three different managers or proxy models:

• Article with ArticleManager (not filtered by archived status): Use this when any article can be picked from a list.
• ActiveArticle(proxy) with ActiveArticleManager (filter archived==False): Enforce custom rules, like editing an article.
• ArchivedArticle(proxy) with ArchivedArticleManager (filter archived==True): Enforce other rules, like read-only status.

Keeping a proxy model self-contained by using its default manager to validate a ForeignKey seems a more valuable use case to me.

Consider adding the following note to the release notes in 3.2:
​https://docs.djangoproject.com/en/4.0/releases/3.2/

(existent) ForeignKey.validate() now uses _base_manager rather than _default_manager to check that related instances exist.
(addition) This might break validations enforced by using proxy model classes in ForeignKey field definitions.