Merge pull request #1265 from pawl/refactor_get_column_names

SQLA - fix column names error for export and details view

flask_admin/contrib/sqla/view.py

@@ -494,43 +494,42 @@ class ModelView(BaseModelView):
 
             return result
 
-    def get_list_columns(self):
+    def get_column_names(self, only_columns, excluded_columns):
         """
             Returns a list of tuples with the model field name and formatted
-            field name. If `column_list` was set, returns it. Otherwise calls
-            `scaffold_list_columns` to generate the list from the model.
-        """
-        if self.column_list is None:
-            columns = self.scaffold_list_columns()
+            field name.
 
-            # Filter excluded columns
-            if self.column_exclude_list:
-                columns = [c for c in columns
-                           if c not in self.column_exclude_list]
+            Overridden to handle special columns like InstrumentedAttribute.
 
-            return [(c, self.get_column_name(c)) for c in columns]
-        else:
-            columns = []
+            :param only_columns:
+                List of columns to include in the results. If not set,
+                `scaffold_list_columns` will generate the list from the model.
+            :param excluded_columns:
+                List of columns to exclude from the results.
+        """
+        if excluded_columns:
+            only_columns = [c for c in only_columns if c not in excluded_columns]
 
-            for c in self.column_list:
-                column, path = tools.get_field_with_path(self.model, c)
+        formatted_columns = []
+        for c in only_columns:
+            column, path = tools.get_field_with_path(self.model, c)
 
-                if path:
-                    # column is in another table, use full path
-                    column_name = text_type(c)
+            if path:
+                # column is a relation (InstrumentedAttribute), use full path
+                column_name = text_type(c)
+            else:
+                # column is in same table, use only model attribute name
+                if getattr(column, 'key', None) is not None:
+                    column_name = column.key
                 else:
-                    # column is in same table, use only model attribute name
-                    if getattr(column, 'key', None) is not None:
-                        column_name = column.key
-                    else:
-                        column_name = text_type(c)
+                    column_name = text_type(c)
 
-                visible_name = self.get_column_name(column_name)
+            visible_name = self.get_column_name(column_name)
 
-                # column_name must match column_name in `get_sortable_columns`
-                columns.append((column_name, visible_name))
+            # column_name must match column_name in `get_sortable_columns`
+            formatted_columns.append((column_name, visible_name))
 
-            return columns
+        return formatted_columns
 
     def init_search(self):
         """

flask_admin/model/base.py

@@ -930,23 +930,6 @@ class BaseModelView(BaseView, ActionsMixin):
         else:
             return self._prettify_name(field)
 
-    def get_list_columns(self):
-        """
-            Returns a list of tuples with the model field name and formatted
-            field name. If `column_list` was set, returns it. Otherwise calls
-            `scaffold_list_columns` to generate the list from the model.
-        """
-        columns = self.column_list
-
-        if columns is None:
-            columns = self.scaffold_list_columns()
-
-            # Filter excluded columns
-            if self.column_exclude_list:
-                columns = [c for c in columns if c not in self.column_exclude_list]
-
-        return [(c, self.get_column_name(c)) for c in columns]
-
     def get_list_row_actions(self):
         """
             Return list of row action objects, each is instance of :class:`~flask_admin.model.template.BaseListRowAction`
@@ -970,45 +953,63 @@ class BaseModelView(BaseView, ActionsMixin):
 
         return actions + (self.column_extra_row_actions or [])
 
-    def get_details_columns(self):
+    def get_column_names(self, only_columns, excluded_columns):
         """
-            Returns a list of the model field names in the details view. If
-            `column_details_list` was set, returns it. Otherwise calls
-            `scaffold_list_columns` to generate the list from the model.
+            Returns a list of tuples with the model field name and formatted
+            field name.
+
+            :param only_columns:
+                List of columns to include in the results. If not set,
+                `scaffold_list_columns` will generate the list from the model.
+            :param excluded_columns:
+                List of columns to exclude from the results if `only_columns`
+                is not set.
         """
-        columns = self.column_details_list
+        if excluded_columns:
+            only_columns = [c for c in only_columns if c not in excluded_columns]
 
-        if columns is None:
-            columns = self.scaffold_list_columns()
+        return [(c, self.get_column_name(c)) for c in only_columns]
 
-            # Filter excluded columns
-            if self.column_details_exclude_list:
-                columns = [c for c in columns
-                           if c not in self.column_details_exclude_list]
+    def get_list_columns(self):
+        """
+            Uses `get_column_names` to get a list of tuples with the model
+            field name and formatted name for the columns in `column_list`
+            and not in `column_exclude_list`. If `column_list` is not set,
+            the columns from `scaffold_list_columns` will be used.
+        """
+        return self.get_column_names(
+            only_columns=self.column_list or self.scaffold_list_columns(),
+            excluded_columns=self.column_exclude_list,
+        )
 
-        return [(c, self.get_column_name(c)) for c in columns]
+    def get_details_columns(self):
+        """
+            Uses `get_column_names` to get a list of tuples with the model
+            field name and formatted name for the columns in `column_details_list`
+            and not in `column_details_exclude_list`. If `column_details_list`
+            is not set, the columns from `scaffold_list_columns` will be used.
+        """
+        only_columns = self.column_details_list or self.scaffold_list_columns()
+        return self.get_column_names(
+            only_columns=only_columns,
+            excluded_columns=self.column_details_exclude_list,
+        )
 
     def get_export_columns(self):
         """
-            Returns a list of the model field names in the export view. If
-            `column_export_list` was set, returns it. Otherwise, if
-            `column_list` was set, returns it. Otherwise calls
-            `scaffold_list_columns` to generate the list from the model.
+            Uses `get_column_names` to get a list of tuples with the model
+            field name and formatted name for the columns in `column_export_list`
+            and not in `column_export_exclude_list`. If `column_export_list` is
+            not set, it will attempt to use the columns from `column_list`
+            or finally the columns from `scaffold_list_columns` will be used.
         """
-        columns = self.column_export_list
-
-        if columns is None:
-            columns = self.column_list
-
-            if columns is None:
-                columns = self.scaffold_list_columns()
+        only_columns = (self.column_export_list or self.column_list or
+                        self.scaffold_list_columns())
 
-            # Filter excluded columns
-            if self.column_export_exclude_list:
-                columns = [c for c in columns
-                           if c not in self.column_export_exclude_list]
-
-        return [(c, self.get_column_name(c)) for c in columns]
+        return self.get_column_names(
+            only_columns=only_columns,
+            excluded_columns=self.column_export_exclude_list,
+        )
 
     def scaffold_sortable_columns(self):
         """