New: expose menu data as serializable objects #1116
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1116 +/- ##
==========================================
+ Coverage 68.71% 68.83% +0.12%
==========================================
Files 45 45
Lines 6223 6248 +25
==========================================
+ Hits 4276 4301 +25
Misses 1947 1947
Continue to review full report at Codecov.
|
nytai
force-pushed
the
tai/expose-menu
branch
2 times, most recently
from
8566d2e to
a97e6ca
Compare
flask_appbuilder/tests/test_menu.py
Outdated
| sess = PSSession() | ||
|
|
||
| class PSView(ModelView): | ||
| datamodel = GenericInterface(PSModel, sess) |
flask_appbuilder/tests/test_menu.py
Outdated
| rv = client.get(uri) | ||
| eq_(rv.status_code, 200) | ||
| data = rv.data.decode('utf-8') | ||
| eq_(data, '{"data":[]}\n') |
There was a problem hiding this comment.
let's use assertEqual I'm changing everything on another PR
nytai
force-pushed
the
tai/expose-menu
branch
from
955537b to
53bee68
Compare
flask_appbuilder/menu.py
Outdated
| class MenuApi(BaseApi): | ||
| resource_name = "menu" | ||
|
|
||
| @expose('/data', methods=["GET"]) |
There was a problem hiding this comment.
I personally prefer just GET /api/v1/menu
flask_appbuilder/menu.py
Outdated
| - label | ||
| - icon | ||
| """ | ||
| return self.response(200, data=current_app.appbuilder.menu.get_data()) |
There was a problem hiding this comment.
Currently FAB uses result key instead of data
| $ref: \ | ||
| '#/paths/menu/data/get/reponses/200/content/application/json/schema/properties/result' | ||
| required: | ||
| - name |
There was a problem hiding this comment.
name and url are required. label, icon are optional
| class MenuApi(BaseApi): | ||
| resource_name = "menu" | ||
|
|
||
| @expose('/', methods=["GET"]) |
There was a problem hiding this comment.
public endpoint, is this intentional? it really does not return any data when no user is authenticated
There was a problem hiding this comment.
Correct, it returns an empty array, currently. However if a user decides to expose a menu for the public role we'd still want to serve that [limited] menu, right?
There was a problem hiding this comment.
Right, yet we do have a public role for this. It's actually a decision between exposing an unauthenticated EP that anonymously return an empty list, or exposing an authenticated EP that returns 401
flask_appbuilder/menu.py
Outdated
|
|
||
| class MenuApiManager(BaseManager): | ||
| def register_views(self): | ||
| self.appbuilder.add_api(MenuApi) |
There was a problem hiding this comment.
Let's create a new option key for this, take a look at: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/security/manager.py#L581
There was a problem hiding this comment.
You mean expose a config to enable/disable this endpoint? something like FAB_EXPOSE_MENU_API?
nytai
force-pushed
the
tai/expose-menu
branch
from
2eb41a2 to
baf06c8
Compare
nytai
force-pushed
the
tai/expose-menu
branch
from
baf06c8 to
e6e103e
Compare
nytai
force-pushed
the
tai/expose-menu
branch
from
a67f551 to
606c2c9
Compare
2 participants
exposes a method on the menu class to output the menu as list of dictionaries that can be converted to json.