Opened 5 years ago

Closed 5 years ago

#30302 closed Cleanup/optimization (fixed)

model_to_dict() should return an empty dict for an empty list of fields.

Reported by: Belegnar Owned by: nobody
Component: Forms Version: dev
Severity: Normal Keywords:
Cc: 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

Been called as model_to_dict(instance, fields=[]) function should return empty dict, because no fields were requested. But it returns all fields
The problem point is

if fields and f.name not in fields:

which should be
if fields is not None and f.name not in fields:

PR: https://github.com/django/django/pull/11150/files

Change History (7)

comment:1 by Mariusz Felisiak, 5 years ago

Type: BugCleanup/optimization
Version: 2.1master

model_to_dict() is a part of private API. Do you have any real use case for passing empty list to this method?

comment:2 by Mariusz Felisiak, 5 years ago

Needs tests: set
Summary: forms.models.model_to_dict fails when `fields` is empty listmodel_to_dict() should return an empty dict for an empty list of fields.
Triage Stage: UnreviewedAccepted

comment:3 by Belegnar, 5 years ago

This method is comfortable to fetch instance fields values without touching ForeignKey fields. List of fields to be fetched is an attr of the class, which can be overridden in subclasses and is empty list by default

Also, patch been proposed is in chime with docstring and common logic

comment:4 by Belegnar, 5 years ago

Needs tests: unset

comment:5 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 714cf468:

Refs #30302 -- Added more tests for forms.model_to_dict().

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: newclosed

In 1ffddfc:

Fixed #30302 -- Fixed forms.model_to_dict() result if empty list of fields is passed.

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