Re: [Django] #36128: Document the necessity of adding unique constraints to related models in intermediary m2m model.

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama
Type: Bug | Status: assigned
Component: Documentation | Version: 5.1
Severity: Normal | 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 Clifford Gama):

* has_patch: 0 => 1

Comment:

https://github.com/django/django/pull/19158
--
Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama
Type: Bug | Status: assigned
Component: Documentation | Version: 5.1
Severity: Normal | 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 Jacob Walls):

* stage: Accepted => Ready for checkin

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

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama

Type: Bug | Status: closed
Component: Documentation | Version: 5.1
Severity: Normal | 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:"ae2736ca3bf4c6a27e23ee95530ad965b550d4cc" ae2736c]:
{{{#!CommitTicketReference repository=""
revision="ae2736ca3bf4c6a27e23ee95530ad965b550d4cc"
Fixed #36128 -- Clarified auto-generated unique constraint on m2m through
models.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:7>

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama
Type: Bug | Status: closed
Component: Documentation | Version: 5.1
Severity: Normal | 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
-------------------------------------+-------------------------------------

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

In [changeset:"4406ce15ff487eb0ac5f384dd2ed84a8d7e40971" 4406ce15]:
{{{#!CommitTicketReference repository=""
revision="4406ce15ff487eb0ac5f384dd2ed84a8d7e40971"
[5.2.x] Fixed #36128 -- Clarified auto-generated unique constraint on m2m
through models.

Backport of ae2736ca3bf4c6a27e23ee95530ad965b550d4cc from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:8>

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama
Type: Bug | Status: closed
Component: Documentation | Version: 5.1
Severity: Normal | 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
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"cc405e154649af37e1741aeb4438391943133c29" cc405e15]:
{{{#!CommitTicketReference repository=""
revision="cc405e154649af37e1741aeb4438391943133c29"
[5.1.x] Fixed #36128 -- Clarified auto-generated unique constraint on m2m

through models.

Backport of ae2736ca3bf4c6a27e23ee95530ad965b550d4cc from main.
}}}
--

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

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama

Type: Bug | Status: new

Component: Documentation | Version: 5.1
Severity: Normal | 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 Guillaume LEBRETON):

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

Comment:

I'm sorry to not have taken a look while it was still open, but my point
was not so about the lack of integrity constraint on the example, but the
lack of proper validation when there is an integrity constraint.
I've looked at the patch and it just add a mention about the unique
constraint, but nothing about the validation.

If i add this uniqueness constraint, the result of adding a duplicate will
be an integrity error rather than a validation error, and the hassle is
writing a complex validation logic like this for a proper validation:

`admins.py`
{{{
class MembersFormset(forms.models.BaseInlineFormSet):

def clean(self):
groups = []

for form in self.forms:
if form.cleaned_data:
groups.append((form.cleaned_data['group'],
form.cleaned_data['person']))

duplicated_groups = [x for x in groups if groups.count(x) > 1]
if duplicated_groups:
raise ValidationError(
'Duplicated values: %(duplicates)s',
params={'duplicates': ", ".join(group.__str__() for group
in set(duplicated_groups))}
)


class MembershipInline(admin.TabularInline):
model = Membership
extra = 1
formset = MembersFormset

}}}

Maybe i missed something, but i find the updated doc even more misleading
now, before the example was quite incomplete because there is almost
always a need for unique constraint in m2m models, but now the provided
example is not working properly because of failing validation.
--
Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:10>

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama

Type: Bug | Status: closed
Component: Documentation | Version: 5.1
Severity: Normal | 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 Jacob Walls):

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

Comment:

Isn't the situation captured by comment:4? It would help if you could test
the linked PR for #35676 and verify it solves the issue you present.
--
Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:11>

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama
Type: Bug | Status: closed
Component: Documentation | Version: 5.1
Severity: Normal | 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
-------------------------------------+-------------------------------------

Comment (by Guillaume LEBRETON):

Replying to [comment:11 Jacob Walls]:

> Isn't the situation captured by comment:4? It would help if you could
test the linked PR for #35676 and verify it solves the issue you present.

My bad, i didn't see it i only checked the patch for this PR. I will check
properly this evening, sorry.
--
Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:12>

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama
Type: Bug | Status: closed
Component: Documentation | Version: 5.1
Severity: Normal | 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
-------------------------------------+-------------------------------------

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

In [changeset:"daba609a9bdc7a97bcf327c7ba0a5f7b3540b46e" daba609a]:
{{{#!CommitTicketReference repository=""
revision="daba609a9bdc7a97bcf327c7ba0a5f7b3540b46e"
Fixed #35877, Refs #36128 -- Documented unique constraint when migrating a
m2m field to use a through model.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:13>

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama
Type: Bug | Status: closed
Component: Documentation | Version: 5.1
Severity: Normal | 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
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"956455dc8a8654bad1264c39d02bd5ffafc7733d" 956455dc]:
{{{#!CommitTicketReference repository=""
revision="956455dc8a8654bad1264c39d02bd5ffafc7733d"
[6.0.x] Fixed #35877, Refs #36128 -- Documented unique constraint when

migrating a m2m field to use a through model.

Backport of daba609a9bdc7a97bcf327c7ba0a5f7b3540b46e from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:14>

[Django]

#36128: Document the necessity of adding unique constraints to related models in
intermediary m2m model.
-------------------------------------+-------------------------------------
Reporter: Guillaume LEBRETON | Owner: Clifford
| Gama
Type: Bug | Status: closed
Component: Documentation | Version: 5.1
Severity: Normal | 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
-------------------------------------+-------------------------------------
Comment (by Sarah Boyce <42296566+sarahboyce@…>):

In [changeset:"6f3813e4b6e3db7fa3cc612c78f898142468bca9" 6f3813e4]:
{{{#!CommitTicketReference repository=""
revision="6f3813e4b6e3db7fa3cc612c78f898142468bca9"
[5.2.x] Fixed #35877, Refs #36128 -- Documented unique constraint when

migrating a m2m field to use a through model.

Backport of daba609a9bdc7a97bcf327c7ba0a5f7b3540b46e from main.
}}}
--

Ticket URL: <https://code.djangoproject.com/ticket/36128#comment:15>