Opened 4 years ago

Closed 3 years ago

#19082 closed Cleanup/optimization (fixed)

Improve the prepopulated_fields autocompletion logic

Reported by: msaelices Owned by: micaelwidell
Component: contrib.admin Version: 1.4
Severity: Normal Keywords:
Cc: d1fffuz0r@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no


I think the logic behind prepopulated fields in admin sould be improved.

The current condition to decide whether to autocomplete the field is: I autocomplete if we are adding the object and it is not already changed by the user.

The problem I found is the we are adding the object part of the previous condition. I think is useful not worry about the add/edit context. I prefer the condition I autocomplete if the field is not already changed by the user and it is empty when the page is loaded

I suppose the current logic is valid because most times the prepopulated fields are SlugField instances, which are usually required. This implies they cannot be empty when we are editing the object (because it was already added with a non empty value). But this is not a required precondition, because you can use the following model and model admin:

class Person(models.Model):
   name = models.CharField(max_length=200, null=True, blank=True)
   surname = models.CharField(...)
   nickname = models.SlugField(null=True, blank=True)

class PersonAdmin(admin.ModelAdmin):
   prepopulated_fields = {"nickname": ("name",)}

Attachments (2)

better_prepopulated_fields_logic.diff (1.6 KB) - added by msaelices 4 years ago.
Better autocompletion logic
better_prepopulated_fields_logic_with_tests.diff (3.1 KB) - added by d1ffuz0r 4 years ago.
upgrade the patch. added test

Download all attachments as: .zip

Change History (7)

Changed 4 years ago by msaelices

Better autocompletion logic

comment:1 Changed 4 years ago by julien

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Agreed. Could you write some Selenium tests for this? You can find an example in django.tests.regressiontests.admin_views.tests.SeleniumPrePopulatedFirefoxTests.

comment:2 Changed 4 years ago by micaelwidell

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

comment:3 Changed 4 years ago by julien

  • Needs tests set

Changed 4 years ago by d1ffuz0r

upgrade the patch. added test

comment:4 Changed 4 years ago by d1ffuz0r

  • Cc d1fffuz0r@… added
  • Needs tests unset

comment:5 Changed 3 years ago by Julien Phalip <jphalip@…>

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

In e351dbf6ee5d344a0fe73a6d05c7dcad7b1d2e9c:

Fixed #19082 -- Enabled admin field pre-population for existing objects.
Thanks to msaelices and d1ffuz0r for the initial patch and tests.

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