Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#32348 closed Bug (fixed)

Deleting "extra" inlines in admin should not be possible.

Reported by: Guan Owned by: Carlton Gibson
Component: Documentation Version: 3.1
Severity: Normal Keywords:
Cc: Carlton Gibson Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Guan)

writing-your-first-django-app-part-7

There is a description: Note that you can’t remove the original three slots.

But I find that the original three slots is able to be removed in Django 3.1.5. The code is as follows:

# polls.models.py

class Question(models.Model):
    question_text = models.CharField(max_length=200)
    pub_date = models.DateTimeField(verbose_name="date published")

    def __str__(self):
        return self.question_text

    def was_published_recently(self):
        now = timezone.now()
        return now >= self.pub_date >= now - datetime.timedelta(days=1)


class Choice(models.Model):
    question = models.ForeignKey(Question, on_delete=models.CASCADE)
    choice_text = models.CharField(max_length=200)
    votes = models.IntegerField(default=0)

    def __str__(self):
        return self.choice_text
# polls.admin.py

class ChoiceInline(admin.StackedInline):
    model = Choice
    extra = 3


class QuestionAdmin(admin.ModelAdmin):
    # fields = ["pub_date", "question_text"]
    fieldsets = [
        (None, {"fields": ["question_text"]}),
        ("Date Information", {"fields": ["pub_date"]})
    ]
    inlines = [ChoiceInline]


admin.site.register(Question, QuestionAdmin)

The image is:

Attachments (1)

Snipaste_2021-01-13_12-48-24.png (35.1 KB ) - added by Guan 4 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 by Guan, 4 years ago

Component: UncategorizedDocumentation

comment:2 by Guan, 4 years ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 4 years ago

Cc: Carlton Gibson added
Component: Documentationcontrib.admin
Severity: NormalRelease blocker
Summary: Error description of Django document in "tutorial07"Deleting "extra" inlines in admin should not be possible.
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Thanks for this report. This is not an issue in docs by a regression in 24e540fbd71bd2b0843e751bde61ad0052a811b3 (see #29087). Deleting inlines added with extra should not be possible.

comment:4 by Hasan Ramezani, 4 years ago

Owner: changed from nobody to Hasan Ramezani
Status: newassigned

comment:5 by Hasan Ramezani, 4 years ago

I just created an initial draft patch which is not working. I will complete it soon.

comment:6 by Hasan Ramezani, 4 years ago

Has patch: set

comment:7 by Hasan Ramezani, 4 years ago

Now it is ready for review. I am not sure about the logic in toggleDeleteButtonVisibility function

in reply to:  7 comment:8 by Guan, 4 years ago

Replying to Hasan Ramezani:

Now it is ready for review. I am not sure about the logic in toggleDeleteButtonVisibility function

Hi! Thanks for your work. But there are probably two slight flaws I think:

  1. When ValidationError is raised, it's no 'X' at the top right of form. That the UX is not comfortable is ticket #29087 talked about.
  2. For toggleDeleteButtonVisibility function, my opinion comes from **extra** form could not be removed and if a formset contains no data, then extra + min_num empty forms will be displayed. So the calculation I think is:
    const isFormOverload = function() {                                                                                                                                                                                        
        const totalFormNum = ~~totalForms.val();                                                                                                                                                                               
        const initialFormNum = ~~initialForms.val();                                                                                                                                                                           
        const minFormNum = ~~minForms.val();                                                                                                                                                                                   
        const extraFormNum = ~~extraForms.val();                                                                                                                                                                               
    
        const emptyFormNum = totalFormNum - initialFormNum;                                                                                                                                                                    
        if (initialFormNum >= emptyFormNum) {                                                                                                                                                                                  
            return emptyFormNum > extraFormNum;                                                                                                                                                                                
        }                                                                                                                                                                                                                      
        return emptyFormNum > extraFormNum + minFormNum - initialFormNum;                                                                                                                                                      
    } 
    

When isFormOverload returns true, all forms show 'X'.

I created patch PR and I'd like to know your opinion.

Last edited 4 years ago by Guan (previous) (diff)

comment:9 by Hasan Ramezani, 4 years ago

Thanks Guan for the new calculation. as you created the ticket I think you have a better overview of It. so, I added your patch to my PR as a separate commit

Last edited 4 years ago by Hasan Ramezani (previous) (diff)

comment:10 by Carlton Gibson, 4 years ago

Reviewing, I think we just missed a doc change with 24e540fbd71bd2b0843e751bde61ad0052a811b3. I've suggest a new PR making that change.

comment:11 by Carlton Gibson, 4 years ago

Component: contrib.adminDocumentation
Owner: changed from Hasan Ramezani to Carlton Gibson
Severity: Release blockerNormal
Triage Stage: AcceptedReady for checkin

Having conferred with Mariusz, we will go with the documentation change. Thanks all.

comment:12 by Carlton Gibson <carlton@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In f4272d00:

Fixed #32348, Refs #29087 -- Corrected tutorial for updated deleting inlines UI.

Updated tutorial to match change in 24e540fbd71bd2b0843e751bde61ad0052a811b3
allowing deletion of original extra inlines.

comment:13 by Carlton Gibson <carlton.gibson@…>, 4 years ago

In 4dbbe37:

[3.2.x] Fixed #32348, Refs #29087 -- Corrected tutorial for updated deleting inlines UI.

Updated tutorial to match change in 24e540fbd71bd2b0843e751bde61ad0052a811b3
allowing deletion of original extra inlines.

Backport of f4272d000af598018247fe9687dac0fd02a29a7c from master

comment:14 by Carlton Gibson <carlton.gibson@…>, 4 years ago

In fa203f17:

[3.1.x] Fixed #32348, Refs #29087 -- Corrected tutorial for updated deleting inlines UI.

Updated tutorial to match change in 24e540fbd71bd2b0843e751bde61ad0052a811b3
allowing deletion of original extra inlines.

Backport of f4272d000af598018247fe9687dac0fd02a29a7c from master

Note: See TracTickets for help on using tickets.
Back to Top