id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
24328	The new Options._get_fields() method needs to be cleaned up	Anssi Kääriäinen	nobody	"At least the method _get_fields() has a lot of places where the code isn't as clean as it should be.

Examples from master:
  - The method has a flag export_ordered_set, but that flag is never used, and the method definitely doesn't export an ordered set.
  - The comment in the first try-catch states that the method must always return a shallow copy. However, the comment doesn't make any sense in that location (the code doesn't make a shallow copy in that location)
  - The method constructs an OrderedDict of field -> boolean. The boolean flag is always True and never used. It would be possible to use an ordinary list instead.
  - The comment in the beginning of ""if not export_ordered_set"" isn't accurate, it states that
   {{{
    # By default, fields contains field instances as keys and all
               # possible names if the field instance as values. When
               # _get_fields() is called, we only want to return field instances,
               # so we just preserve the keys.
   }}}
   this comment doesn't make much sense.

Other parts of the code seem to be lot better. In addition, L592 contains f.rel.field.rel.is_hidden(). But f.rel.field.rel is always the same as f.rel. L476 states that the code calls _get_fields() with export_ordered_set=True, but that isn't actually happening.

The functionality itself seems to be fine, but there is just a lot of left-overs from internal refactorings."	Cleanup/optimization	closed	Database layer (models, ORM)	1.8alpha1	Release blocker	fixed	1.8-beta	pirosb3	Ready for checkin	1	0	0	0	0	0