Issue: Flask-Admin Peewee Contribution Issue
Author: mkmoisen
State: closed
Created: 2022-02-25 18:35:30

Sorry if this is the wrong repo to put this in, I wasn't sure if creating this issue in flask-admin made sense.

The Peewee contribution to Flask-Admin assumes that a connection to the database has already been opened in `app.before_request`.  If a connection has not been opened in `app.before_request`, and the db's `autoconnect=False`, then this will result in a lot of `peewee.InterfaceError: Error, database connection not opened.` errors.

The only way to prevent this is to subclass `ModelView` and wrap `get_one`, `get_list`, `create_model`, `update_model`, `delete_model`, and `action_delete` with code that opens/closes the connection and calls `super()`. This is easy, not a big deal. 

However handling foreign key select drop downs and ajax call backs is a bit harder. Because the `QueryAjaxModelLoader`, `SelectQueryField`, and `ModelSelectField` classes are hard coded and not polymorphic, I had to subclass and copy and paste some code before I could wrap it with code that opens/closes the connection. This isn't great because whenever there is an upgrade I have manually diff it and update the code.

I have two suggestions. Would you please take a look and let me know what you think?

----

Would it make sense to wrap all the calls to the database from flask-admin/peewee contribution with code that opens/closes the connection? I'm not sure if this poses any kind of backwards compatibility issues for people who open connections in `app.before_request`. For example the following opens a save point:

    with db: 
        with db:  # opens save point here
            list(Tweet.select())

---

If that is not a good approach, could we make the `ModelView` more polymorphic? I think there will be more benefits to this approach aside from my use case, as it would make it much easier to customize the behavior. Today there is no simple way to customize the `ModelSelectField`, for example, due to this lack of polymorphism.

The `ModelConverter` class could instead look like this:

```
class ModelConverter:
    select_query_field_class = SelectQueryField  # make this polymorphic
    model_select_field_class = ModelSelectField  # make this polymorphic 

    def handle_foreign_key(self, model, field, **kwargs):
        if field.null:
            kwargs['allow_blank'] = True
        if field.choices is not None:
            field_obj = self.select_query_field_class(query=field.choices, **kwargs)  # polymorphic call
        else:
            field_obj = self.model_select_field_class(model=field.rel_model, **kwargs)  # polymorphic call
        return FieldInfo(field.name, field_obj)
```

If it was polymorphic, then I wouldn't need to copy paste this whole function just to swap out the class names. The only thing I would need to do is subclass ModelSelect/SelectQueryField and wrap with calls to open/close the db. What I have to do today is copy paste the whole `handle_foreign_key` over and then change only the class names.

In addition, if you do it this way, someone could subclass ModelSelectField to restrict the query based on user or route or something. In general the polymorphic approach opens up more avenues for customization.

Thank you.

This belongs on the flask-admin repo.

@coleifer Thanks, I will add it there. Would you be able to comment on this part:

Would there be any harm to wrapping calls from inside of flask-admin with code that opens/closes a connection, if the user is opening a connection from `app.before_request`?

E.g.

```
def @app.get('/tweets/'):
    with db:
        return jsonify(Tweet.select()), 200

@app.before_request
def before():
    db.connect()

@app.after_request
def after():
    db.close()


```