Opened 21 months ago

Closed 21 months ago

Last modified 21 months ago

#34025 closed Bug (fixed)

Autocomplete field fills all empty required fields references to the same model when added a choice in popup.

Reported by: Alexandre da Silva Owned by: David Sanders
Component: contrib.admin Version: 4.1
Severity: Release blocker Keywords: admin, autocomplete
Cc: Marcelo Galigniana Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description (last modified by Alexandre da Silva)

Example of how to reproduce:

  1. create a model Person.
  1. create a model Order with 2 fks pointing to person, for example: supplier and customer
from django.db import models

# Create your models here.
class Person(models.Model):
    name = models.CharField(max_length=255)

    def __str__(self):
        return self.name

class Order(models.Model):
    supplier = models.ForeignKey(Person, related_name='the_supplier', on_delete=models.PROTECT)
    customer = models.ForeignKey(Person, related_name='the_customer', on_delete=models.PROTECT)
  1. create an admin page for Order.
  1. place the two fields in autocomplete_fields:
from django.contrib import admin
from sales.models import *


class PersonAdmin(admin.ModelAdmin):
    search_fields = ['name']

class OrderAdmin(admin.ModelAdmin):
    autocomplete_fields = ['supplier', 'customer']


admin.site.register(Person, PersonAdmin)
admin.site.register(Order, OrderAdmin)
  1. do the migration stuff and run the app.

when create an supplier from + symbol near the supplier, when close the window both fields get updated with the ID
this also occurs when editing.

ps. I´m not an expert but the code that seems to introduce this bug is on commit c72f6f36c13a21f6db3d4f85d2d3cec87bad45e6
probably on data-model-ref tag, since in the page all autocompletes have the same data-model-ref value, in this example will be "person"

function updateRelatedSelectsOptions(currentSelect, win, objId, newRepr, newId) {
        // After create/edit a model from the options next to the current
        // select (+ or :pencil:) update ForeignKey PK of the rest of selects
        // in the page.

        const path = win.location.pathname;
        // Extract the model from the popup url '.../<model>/add/' or
        // '.../<model>/<id>/change/' depending the action (add or change).
        const modelName = path.split('/')[path.split('/').length - (objId ? 4 : 3)];
        const selectsRelated = document.querySelectorAll(`[data-model-ref="${modelName}"] select`);

        selectsRelated.forEach(function(select) {
            if (currentSelect === select) {
                return;
            }

            let option = select.querySelector(`option[value="${objId}"]`);

            if (!option) {
                option = new Option(newRepr, newId);
                select.options.add(option);
                return;
            }

            option.textContent = newRepr;
            option.value = newId;
        });
    }


Change History (14)

comment:1 by Alexandre da Silva, 21 months ago

Description: modified (diff)

comment:2 by David Sanders, 21 months ago

Component: Uncategorizedcontrib.admin

A couple of points here:

(note: I only tested on Chrome, behaviour could differ for other browsers)

  • Yep the commit referenced is intentionally updating the options for all selects referencing that model - this is so they all get the new option.
  • While I could reproduce the described issue for creates, I couldn't do so for edits. In the edit screen only the field for which I clicked "+" was updated.
  • This appears to be because both fields are required. If you make the fks null=True, blank=True then this behaviour isn't shown. I'm not sure this is something that's fixable as this is the behaviour shown for native selects. If you create a page with simply:
<select></select>

And then run:

document.querySelector('select').options.add(new Option('test'))

it populates the select with the newly created option - because it's the only option to select.

You could possibly solve this by adding an "empty" option?

Version 3, edited 21 months ago by David Sanders (previous) (next) (diff)

comment:3 by David Sanders, 21 months ago

After a bit of playing around with jQuery select2 I believe this is actually fixable - because select2 allows required selects to be deselectable.

Before the option is added, we can observe the value of the select is null for selects that haven't an option selected yet. After adding the option, if the previously observed value was null then we can "deselect" it again.

comment:5 by David Sanders, 21 months ago

Has patch: set

comment:6 by David Sanders, 21 months ago

Owner: changed from nobody to David Sanders
Status: newassigned

comment:7 by Alexandre da Silva, 21 months ago

While I could reproduce the described issue for creates, I couldn't do so for edits. In the edit screen only the field for which I clicked "+" was updated.

To reproduce on edit, the steps are:

  1. create a new record for order
  2. select an existing supplier on select combobox, then click on edit.
  3. save the supplier record, customer select also gets updated

Applied patch and tested locally, so the issue semms to be resolved for both cases, add and edit.

comment:8 by David Sanders, 21 months ago

Alexandre the PR I've submitted above looks like it fixes the issues you've described. Are you able to try it out and report back if it works?

If it does work for you then this will increase the chance of the ticket & patch being accepted.

comment:9 by Alexandre da Silva, 21 months ago

Hello David,
I already tested yesterday. It worked

comment:10 by David Sanders, 21 months ago

Thanks Alexandre, will wait for a member of the triage team… Djangocon is on atm so we may need to be patient :)

comment:11 by Mariusz Felisiak, 21 months ago

Cc: Marcelo Galigniana added
Needs documentation: set
Needs tests: set
Severity: NormalRelease blocker
Summary: Bug on autocomplete field when two fields from same reference model are added in same pageAutocomplete field fills all empty required fields references to the same model when added a choice in popup.
Triage Stage: UnreviewedAccepted

Thanks for the report!

Regression in c72f6f36c13a21f6db3d4f85d2d3cec87bad45e6.

comment:12 by Mariusz Felisiak, 21 months ago

Needs documentation: unset
Needs tests: unset
Triage Stage: AcceptedReady for checkin

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 21 months ago

Resolution: fixed
Status: assignedclosed

In 9976f3d4:

Fixed #34025 -- Fixed selecting ModelAdmin.autocomplete_fields after adding/changing related instances via popups.

Regression in c72f6f36c13a21f6db3d4f85d2d3cec87bad45e6.

Thanks Alexandre da Silva for the report.

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 21 months ago

In 33d9247:

[4.1.x] Fixed #34025 -- Fixed selecting ModelAdmin.autocomplete_fields after adding/changing related instances via popups.

Regression in c72f6f36c13a21f6db3d4f85d2d3cec87bad45e6.

Thanks Alexandre da Silva for the report.

Backport of 9976f3d4b80cfb2e6f4c998438622b78eb1ac53e from main

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