pallets-eco / flask-admin

Simple and extensible administrative interface framework for Flask
https://flask-admin.readthedocs.io
BSD 3-Clause "New" or "Revised" License
5.76k stars 1.57k forks source link

type object ... has no attribute ... when using formatters #1299

Closed ugomeda closed 8 years ago

ugomeda commented 8 years ago

Hi, i'm using formatters for a model :

    column_formatters = {
        'name': macro('name_formatter')
    }

With its template :

{% macro name_formatter(model, column) %}
  {{ model.firstname }} {{ model.lastname }}
{% endmacro %}

Since flask-admin 1.4.1, i get this exception when running my application :

Traceback (most recent call last):
  File "run.py", line 2, in <module>
    from backend import app
  File "/path/to/project/backend/__init__.py", line 68, in <module>
    admin.create_admin(app)
  File "/path/to/project/backend/views/admin.py", line 204, in create_admin
    admin.add_view(OrderModelView(Order, db.session))
  File "/path/to/project/lib/python2.7/site-packages/flask_admin/contrib/sqla/view.py", line 318, in __init__
    menu_icon_value=menu_icon_value)
  File "/path/to/project/lib/python2.7/site-packages/flask_admin/model/base.py", line 771, in __init__
    self._refresh_cache()
  File "/path/to/project/lib/python2.7/site-packages/flask_admin/model/base.py", line 847, in _refresh_cache
    self._list_columns = self.get_list_columns()
  File "/path/to/project/lib/python2.7/site-packages/flask_admin/model/base.py", line 980, in get_list_columns
    excluded_columns=self.column_exclude_list,
  File "/path/to/project/lib/python2.7/site-packages/flask_admin/contrib/sqla/view.py", line 517, in get_column_names
    column, path = tools.get_field_with_path(self.model, c)
  File "/path/to/project/lib/python2.7/site-packages/flask_admin/contrib/sqla/tools.py", line 144, in get_field_with_path
    value = getattr(current_model, attribute)
AttributeError: type object 'Order' has no attribute 'name'

Version 1.4.0 works well.

lnielsen commented 8 years ago

We've seen this error as well @inveniosoftware (on v1.4.1).

mrjoes commented 8 years ago

I'm not sure this is related. Are you sure you don't have something like:

column_list = ('name', 'foo', 'bar')

And there's no name column on the model?

lnielsen commented 8 years ago

Here's an snippet from the configuration that worked in v1.4.0 and now fails in v1.4.1:

class Location(db.Model, Timestamp):
    __tablename__ = 'files_location'
    id = db.Column(db.Integer, primary_key=True)
    name = db.Column(db.String(20), unique=True, nullable=False)
    uri = db.Column(db.String(255), nullable=False)
    default = db.Column(db.Boolean, nullable=False, default=False)

class LocationModelView(ModelView):
    # ... 
    column_formatters = dict(
        bucketslink=link('Buckets', lambda o: url_for('bucket.index_view', flt2_2=o.name))
    )
    column_details_list = ('name', 'uri', 'default', 'bucketslink')
    column_list = ('name', 'uri', 'default', 'bucketslink')
    # ...

We have a column formatter bucketslink which we include in the column_list property. The error it spits out is the same as above (basically anything for the view which hits ModelView. get_column_names with yield the error):

  File "...flask_admin/contrib/sqla/tools.py", line 144, in get_field_with_path
    value = getattr(current_model, attribute)
AttributeError: type object 'Location' has no attribute 'bucketslink'

I will try to see if we can provide a minimal test case showcasing the problem, but it might be a couple of days before I get the time.

mrjoes commented 8 years ago

OK, here's why it is happening - you don't have bucketslink column anymore on your Location model but have it in column_list. Flask-Admin was not as strict about column_list definition before 1.4.1.

Maybe it was a backref that was removed, maybe you had a column on the model and then code was refactored.

Sure, I can change default behavior to show a warning, but it is easy to miss it.

lnielsen commented 8 years ago

Thanks for the feedback. OK, it might have been an unwanted behaviour of v1.4.0. We used the bucketslink with a formatter to stick a link in the list view, and thus never had a bucketslink column on the Location model. Is there another preferred way of doing this? We can naturally stick a property on the model which I think should work (what we liked about the previous solution was that our model didn't get "dirty" with link generation).

ugomeda commented 8 years ago

This makes it difficult to implement some functionalities. For instance, i have a formatter displaying links to download invoices/tickets of an order, it uses the ID of my order. I defined column_list = ('id', 'links') and added a formatter for the links. With this change, it's not possible to use twice the id column with different formatters, and i'll have to generate URLs in my model.

Same for formatters displaying data from several columns, i'll have to create weird getters returning tuples.

It might be possible to move most of the logic to the model and make the formatters as dumb as possible, but it's definitely less flexible with this change.

mrjoes commented 8 years ago

Ah! Now it makes sense. Never thought of using it that way.

OK, I'll restore old behavior and release 1.4.2 with a bug fix today. I can't think of better way of defining list of visible columns in one place even if they're virtual.

mrjoes commented 8 years ago

So, I pushed a fix, can you try with current master? If everything is working as expected, I'm pushing it to PyPI.

ugomeda commented 8 years ago

Maybe create a classe to define virtual columns could make it possible to have flexibility while maintaining strict checks on the columns ?

column_list = ('id', composite_column('link', cols=('id',), formatter='link_formatter'))

(It would also allow us to use different formatter for the same column between the list and details views)

Sonata also have some good ideas about this : https://sonata-project.org/bundles/admin/3-x/doc/reference/action_list.html

ugomeda commented 8 years ago

Yep, i can confirm it works again with master.

mrjoes commented 8 years ago

Pushed to PyPI. Thanks everyone.

lnielsen commented 8 years ago

Awesome. Thanks for the quick fix! Greatly appreciated.

asfaltboy commented 7 years ago

@mrjoes I would just like to mention that this behavior does not seem to be documented; and, in my opinion is extremely useful for setting "virtual" fields. A simple use case:

class Person(db.Model):
    # ...
    first_name = db.Column(db.String(64))
    last_name = db.Column(db.String(64))

    @property
    def full_name(self):
        return '{p.first_name} {p.last_name}'.format(p=self)

class Person(sqla.ModelView):
    column_list = ('full_name', )