feat: Add support for before_request hooks

[benjreinhart]

Description

This PR adds support for a before_request hook that can be used in FAB's view and API classes to execute some code just before the route handler is invoked. It has the same semantics as Flask's before_request, but it is scoped only to the routes within a given FAB class and can be scoped even further to a subset of handlers within the class.

Examples

@before_request
def ensure_feature_is_enabled(self):
    if self.feature_is_disabled:
        return self.response_404()
    return None

@before_request(only=["create", "update", "delete"])
def ensure_write_mode_enabled(self):
    if self.read_only:
        return self.response_400()
    return None

Motivation

It seems like we could implement the conditional logic directly in route handlers. Why do we need a new pattern?

In many cases, implementing the conditional logic in the handler itself is preferred. To justify this change, consider the following scenario:

You've implemented many classes that inherit from ModelView or ModelRestAPI and have relied on FAB to implement the endpoint logic for you. Some of these classes you only want enabled if some dynamic condition is true at request time (e.g., feature flags, A/B testing). Since you have not implemented the handlers yourself, you have nowhere to put the conditional logic... that is, unless you manually override/implement each one.

In other classes, you have a large number of endpoints that you have implemented. Some need to be gated by a condition. The before_request hook provides a way to DRY up your condition handling that is consistent with classes whose handlers are implemented by FAB.

While technically possible to workaround these issues, the hook provides an elegant pattern for developers that enables consistency across classes, a way to keep some endpoint logic DRY and decoupled from handlers, and prevents the need to override/implement FAB's automatic route handling in some cases.

FAB Menus/Separators

FAB supports constructing UI menu items and separators, which is registered with its view. If the before_request hook in a view is able to dynamically enabled/disable a route, wouldn't we want the same for the menu item?

I would imagine that in most (all?) cases, if a route was disabled the menu item should be unavailable too. However, I believe that exposing one pattern that handles both Flask endpoints/responses and UI component availability is hard to do elegantly.

Therefore, I propose treating the request handling and menu item separately. This PR implements the request piece, and a subsequent PR I plan to open will implement an option for executing a callable to determine if menu item is available.

[nytai]

LGTM

[dpgaspar]

Looks really good!

• Let's also add tests for ModelView classes with before_request
• Add a reference on the docs for the ModelView also

This supports multiple before_request is there a use case for it? Should we add tests with multiple before_request ?

[dpgaspar]

totally optional, a list comprehension could look good here and can be slightly more efficient.

[dpgaspar]

.

[dpgaspar]

seems like tests began to fail. let me take a look at that

[benjreinhart]

I don't see any failing tests, where do you see them failing?

[dpgaspar]

I don't see any failing tests, where do you see them failing?

No worries, was a temporary failure on packages microsoft

[benjreinhart]

@dpgaspar I added more comprehensive tests and updated to add type hints. As for the docs, I am happy to make more updates, but where I added it is the API reference section, which AFAICT is general API reference, not tied to FAB API vs FAB view. Let me know what you think.

[dpgaspar]

@benjreinhart update to 37
https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/tests/test_mvc.py#L586

[dpgaspar]

optional: we could do a for loop here to DRY these tests a bit

[benjreinhart]

Personally, I like keeping test code "damp" rather than DRY because I think that writing out each case explicitly is easier to read, serving as documentation. Also, less likely to introduce a bug into tests if you avoid "programming" in tests.

That being said, I am happy to change it if you feel strongly.