Code

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#12692 closed (fixed)

Wrong interpretation of extra clause in ModelAdmin with new admin UI

Reported by: apollo13 Owned by: jezdez
Component: Contrib apps Version: master
Severity: Keywords: admin, js, inlines
Cc: jezdez, apollo13 Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by russellm)

In the current admin you can specify max_num and extra for inlines. Before r12297 this meant that you would get the current data from the database + extra as long as it stays below max_num. If you were still below max_num you had to save the page to get extra new fields (this could be repeated until you reached the limit). This behaviour is currently implemented with the js too; but I think (as js is a usability enchancement) you should be able to add as many rows until you reach max_num not TOTAL-FORMS (which is rows+extra while <= max_num). To get this working I guess we would need to pass max_num to the js so it can decide when to stop.


An alternate description from #12704:

The new javascript-enabled admin UI appears have an unusual interpretation of the 'extra' clause on a ModelAdmin?.

The current code interprets 'extra = N' as "allow the user to add no more than N extra rows. The user must press (+ add another XXX) to add the first inline.

I would have thought the interpretation would have been "instantiate the form with with N empty extra rows". The task of constraining the number of allowed inlines is the job of the 'max_num' clause.

The admin UI should always show N blank inputs (for extra = N), as long as that wouldn't exceed the max_num. The user should be able to fill in up to N inlines before needing to press the (+ add another) button.

If I'm incorrect, then the second failing test reported by #12703 needs to be fixed by accounting for the 5 fields (and missing default date) that are no longer on the form.

Attachments (3)

12692.1.diff (5.5 KB) - added by jezdez 4 years ago.
Updated inline code to be more useful wrt the maximal number of items. A max_num of 0 means infinite additions.
12692.2.diff (5.3 KB) - added by jezdez 4 years ago.
A patch without console.log calls
12692.3.diff (47.5 KB) - added by jezdez 4 years ago.
Patch with Closure compiled min file and modified tests.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 4 years ago by russellm

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Summary changed from shortcoming in the new admin inlines to Wrong interpretation of extra clause in ModelAdmin with new admin UI
  • Triage Stage changed from Unreviewed to Design decision needed

#12704 was a duplicate. Updated ticket title to be more explicit.

comment:2 Changed 4 years ago by jezdez

  • Triage Stage changed from Design decision needed to Accepted

comment:3 Changed 4 years ago by jezdez

  • Owner changed from nobody to jezdez
  • Status changed from new to assigned

Changed 4 years ago by jezdez

Updated inline code to be more useful wrt the maximal number of items. A max_num of 0 means infinite additions.

Changed 4 years ago by jezdez

A patch without console.log calls

Changed 4 years ago by jezdez

Patch with Closure compiled min file and modified tests.

comment:4 Changed 4 years ago by jezdez

I believe the last patch fixes all of what was wrong with the "extra" implementation, now showing initial and extra forms by default, allowing to add new objects till the total number of inline forms equals max_num (or indefinitely adding new). I'm passing the max_num to the javascript as a separate form field.

comment:5 Changed 4 years ago by jezdez

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [12369]) Fixed #12692 - Properly handle the extra clause of admin inline formsets. Also fixes #12703, second error.

comment:6 Changed 4 years ago by gregplaysguitar

There are times when one doesn't want the users to be able to add new inlines at all - for example if they are generated on model save or at some other time, but you do want the user to be able to edit them. extra=0 used to achieve this, but it doesn't anymore.

comment:7 Changed 3 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.