[Django] #36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type:
| Cleanup/optimization
Status: new | Component: Database
| layer (models, ORM)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Passing `exclude=["pk"]` to `full_clean()` / `clean_fields()` has no
effect: the underlying fields have to be provided instead.

This "no effect" might be silent (coercing a value when you didn't expect)
or loud (raising ValidationError) given that we don't raise `ValueError`
for providing an extraneous field name.

I'm suggesting we either implement `exclude=["pk"]` to exclude all the
composed fields or raise `ValueError`. Raising an error might be wiser?

Failing test:
{{{#!diff
diff --git a/tests/composite_pk/test_models.py
b/tests/composite_pk/test_models.py
index ca6ad8b5dc..ed54d6c352 100644
--- a/tests/composite_pk/test_models.py
+++ b/tests/composite_pk/test_models.py
@@ -118,6 +118,10 @@ class CompositePKModelsTests(TestCase):

self.assertSequenceEqual(ctx.exception.messages,
messages)

+ def test_clean_fields_exclude_pk(self):
+ post = Token()
+ post.clean_fields(exclude=["pk"])
+
def test_field_conflicts(self):
test_cases = (
({"pk": (1, 1), "id": 2}, (1, 1)),
}}}
{{{
======================================================================
ERROR: test_clean_fields_exclude_pk
(composite_pk.test_models.CompositePKModelsTests.test_clean_fields_exclude_pk)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/.../django/tests/composite_pk/test_models.py", line 123, in
test_clean_fields_exclude_pk
post.clean_fields(exclude=["pk"])
~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^
File "/Users/.../django/django/db/models/base.py", line 1708, in
clean_fields
raise ValidationError(errors)
django.core.exceptions.ValidationError: {'tenant': ['This field cannot be
null.'], 'id': ['This field cannot be null.']}

----------------------------------------------------------------------
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36070>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: needsinfo

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* resolution: => needsinfo
* status: new => closed

Comment:

I might be misunderstanding but I feel like the current behavior is
correct
I think we tried to hint at this in the docs in the
[https://docs.djangoproject.com/en/dev/topics/composite-primary-key
/#composite-primary-keys-in-forms Composite Primary Keys in forms]
section. Do you think we need to expand on this or have I misunderstood
what you're expecting?
--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:1>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

This might be a nothing, but the doc explains that a "virtual field" is
"excluded from model forms" and "does not have a form field", but it
doesn't tell me what impact there is on model validation, forms aside.
"Virtual field" isn't described elsewhere in the docs, so for me I didn't
follow that it couldn't be passed to `exclude=`.

A model with composite pk's looks like it has a model field called `pk`,
so the idea is a user might think you can pass it to `exclude_fields` and
get every part of the `pk` excluded. From the linked doc I can't really
tell that I can't.

(As I'm sure you've gathered I'm just kicking the tires here. Fine to wait
to see if this comes up in practice.)
--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:2>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: (none)

Type: | Status: new

Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |

Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* resolution: needsinfo =>
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
* status: closed => new

Comment:

True, a "virtual field" is not really a defined term and in my head things
like a
[https://docs.djangoproject.com/en/5.1/ref/contrib/contenttypes/#django.contrib.contenttypes.fields.GenericForeignKey
GenericForeignKey] or a
[https://docs.djangoproject.com/en/5.1/ref/models/fields/#generatedfield
GeneratedField] might be in a similar category.
I will accept as happy to review a docs update
--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:3>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* owner: (none) => Jacob Walls
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:4>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/19022 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:5>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:6>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:7>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:8>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Ready for checkin => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:9>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:10>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls

Cleanup/optimization | Status: closed

Component: Database layer | Version: dev
(models, ORM) |

Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"de9f46d7074a948d781b31476bb12a3ed017c8c0" de9f46d]:
{{{#!CommitTicketReference repository=""
revision="de9f46d7074a948d781b31476bb12a3ed017c8c0"
Fixed #36070 -- Clarified model validation behavior for composite pks.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:12>

[Django]

#36070: clean_fields(exclude=["pk"]) not implemented for composite primary keys
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
Type: | Walls

Cleanup/optimization | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"f054045973ea767ee4e3d60723de4a2f13bf0c49" f0540459]:
{{{#!CommitTicketReference repository=""
revision="f054045973ea767ee4e3d60723de4a2f13bf0c49"
Refs #36070 -- Referred to pk as an attribute when a composite primary key
is defined.

This is to avoid confusion that a field is often associated with having
a single associated database column.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36070#comment:11>