--
Ticket URL: <https://code.djangoproject.com/ticket/26142>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Comment (by Tim Graham <timograham@…>):
In [changeset:"204d31cd6033bfcd7ede1a6eedcc5f351d2cfa11" 204d31c]:
{{{
#!CommitTicketReference repository=""
revision="204d31cd6033bfcd7ede1a6eedcc5f351d2cfa11"
[1.9.x] Refs #26142 -- Documented that Formset's extra=0 doesn't prevent
creating objects.
Backport of 8e6a08e937272f088902cdbec65a9f2e919783bf from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:2>
Comment (by Tim Graham <timograham@…>):
In [changeset:"8e6a08e937272f088902cdbec65a9f2e919783bf" 8e6a08e]:
{{{
#!CommitTicketReference repository=""
revision="8e6a08e937272f088902cdbec65a9f2e919783bf"
Refs #26142 -- Documented that Formset's extra=0 doesn't prevent creating
objects.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:1>
* owner: nobody => Mortal
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:3>
Comment (by dsanders11):
Doesn't `max_num` already allow this? Testing on master, if you have
`max_num` set to 0 and `validate_max` to `True`, then the formset is
effectively edit-only for existing objects. From the
[https://docs.djangoproject.com/en/1.9/topics/forms/modelforms/#limiting-
the-number-of-editable-objects documentation]:
> `max_num` does not prevent existing objects from being displayed
Is there a corner case of functionality that's not covered by these two
settings?
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:4>
Comment (by timgraham):
Yes, I think it's that sending data that references pks that don't appear
in the queryset creates new objects for that data.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:5>
Comment (by dsanders11):
Can you expand on that a little? Looking at the formset code, if the form
number is less than the initial form count, it uses
`save_existing_object`. Since `validate_max` prevents any forms above
initial form count from being a valid formset, the only way a new object
could be created is if `save_existing_object` could create new objects,
which would be a bug in itself probably.
Playing with it in Chrome, providing a blank value for the PK of one of
the objects in the formset will cause an error when
[https://github.com/django/django/blob/master/django/forms/models.py#L605
trying to use to_python on the PK value] since the value is blank and an
integer is expected. Even if that didn't fail, it would fail to find the
object by its PK in the queryset.
Providing a bogus value for the PK will also
[https://github.com/django/django/blob/master/django/forms/models.py#L588
fail to look up in the queryset] on the formset. This also occurs if you
use a PK for an existing object that isn't in the queryset, which is good,
otherwise you'd be able to modify objects outside the queryset which would
be very bad.
So, I'm not sure I see the path where new objects can be created if
`validate_max` is set and `max_num` is 0. Doesn't mean it's not there, but
it's not immediately obvious how that could occur.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:6>
Comment (by timgraham):
I don't think `max_num=0` allows editing existing instances. Such a
formset will fail with "Please submit 0 or fewer forms." won't it?
The idea I had to try to accomplish this is setting
`max_num=len(queryset)` but I'm attaching a regression test demonstrating
that an extra instance is created if the user edits the `'form-
INITIAL_FORMS'` form value.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:7>
* Attachment "26142-test.diff" added.
* owner: Mathias Rav => Parth Patil
Comment:
Hey Mathias, I have started working on this ticket, hope it's not a
problem that I'm assigning this to myself. Found no other to contact you
so am directly assigning it.
I will try to make an `edit_only` mode for the `ModelFormSet` which will
fix this issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:8>
Comment (by Parth Patil):
Here is a link to the PR for this ticket, please have a look
https://github.com/django/django/pull/11580
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:9>
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:10>
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:11>
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:12>
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:13>
* owner: Parth Patil => (none)
* status: assigned => new
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:14>
Comment (by Vlad):
I added a PR (https://github.com/django/django/pull/14725) for this
ticket. It's based on previous patch that Parth Patil made. I cleaned up
git artifacts and changed tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:15>
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:16>
* owner: (none) => Vlad
* needs_better_patch: 0 => 1
* status: new => assigned
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:17>
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:18>
* needs_tests: 0 => 1
Comment:
Patch still lacks a regression test that fails on main (upon removing the
keyword introduced by the PR). Luckily, there is one we can use in the
previous PR. Vlad, can you update your patch with additional tests from
the previous PR?
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:19>
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:20>
* needs_better_patch: 0 => 1
Comment:
I agree with [https://github.com/django/django/pull/14725/files#r690721475
David's suggestion] to include the new argument in
`modelformset_factory()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:21>
* needs_better_patch: 1 => 0
Comment:
I added ***edit_only*** argument for the {{{modelfomset_factory}}} and
fixed tests as
[https://github.com/django/django/pull/14725/#issuecomment-913259753
discussed with Jacob]. Also, I documented changes for the
{{{modelformset_factory}}} in release notes and topic about modelformsets.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:22>
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:23>
* needs_better_patch: 1 => 0
Comment:
Added `edit_only` argument to the `formset_factory`.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:24>
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:25>
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0
Comment:
Added argument for `inlineformset_factory`, tests for
`inlineformset_factory` and `formset_factory`, changed docs and release
notes
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:26>
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:27>
* needs_docs: 1 => 0
Comment:
Added topic about `edit_only` mode in Formsets and fixed other remarks.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:28>
* needs_docs: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:29>
* needs_docs: 1 => 0
Comment:
Fixed docs, where Sphinx raised warnings
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:30>
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:31>
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e87f57fdb8dcdabc452bd15abd015bf6c9b1f7a8" e87f57f]:
{{{
#!CommitTicketReference repository=""
revision="e87f57fdb8dcdabc452bd15abd015bf6c9b1f7a8"
Fixed #26142 -- Allowed model formsets to prevent new object creation.
Thanks Jacob Walls, David Smith, and Mariusz Felisiak for reviews.
Co-authored-by: parth <part...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:32>
Comment (by Matthias Kestenholz):
Hi all
Great new feature! Thanks. I wanted to comment on the naming of the new
parameter; maybe `can_add=True` would be a better default than
`edit_only=False`? There's a precedent for using `can_*` in Django's
codebase and it would be a bit more consistent.
--
Ticket URL: <https://code.djangoproject.com/ticket/26142#comment:33>