Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What about Router per View? #771

Open
1 task done
hasansezertasan opened this issue May 16, 2024 · 2 comments
Open
1 task done

What about Router per View? #771

hasansezertasan opened this issue May 16, 2024 · 2 comments

Comments

@hasansezertasan
Copy link
Contributor

hasansezertasan commented May 16, 2024

Checklist

  • There are no similar issues or pull requests for this yet.

Is your feature related to a problem? Please describe.

Introduction

At #767, I talked about building a RedisCLI view like in the Flask Admin.

I started building this view, but I realized that routing in the view is a bit different from the Flask Admin.

Flask Admin uses one Blueprint (Router equivalent in Flask) per view but SQLAdmin registers exposed methods directly to the main Starlette application instance (BaseAdmin.admin), not a dedicated Router instance of the view.

So in my case, if I do this:

from sqladmin import BaseView, expose

class RedisCLIView(BaseView):
    name = "Redis CLI"
    redis_instance = None
    identity = "rediscli"

    @expose("/rediscli", methods=["GET", "POST"], identity="index")
    async def index_page(self, request):
        # Do something with redis_instance
        return await self.templates.TemplateResponse(request, "sqladmin/redis/index.html")

admin.add_view(RedisCLIView)

The url_for method won't work as expected.

url_for("admin:rediscli:index") will raise an error because the view is not registered as a Router with a name ("rediscli") and the exposed method is not registered as a Route with a name ("index").

url_for("admin:index") will work because exposed methods are registered as a Route to the main Starlette application instance.

What is the problem?

In general, every view is expected to have an entry point (like a home page) and other pages. This is generally the index and other pages.

What happens if we have multiple views with the same entry point name?

from starlette.applications import Starlette
from sqladmin import Admin, BaseView, expose

class RedisCLIView(BaseView):
    name = "Redis CLI"
    redis_instance = None
    identity = "rediscli"

    @expose("/rediscli", methods=["GET", "POST"], identity="index")
    async def index_page(self, request):
        # Do something with redis_instance
        return await self.templates.TemplateResponse(request, "sqladmin/redis/index.html")

class RedisCLI1(RedisCLIView):
    name = "Redis CLI 1"
    redis_instance = None

class RedisCLI2(RedisCLIView):
    name = "Redis CLI 2"
    redis_instance = None

app = Starlette()
admin = Admin(app)
admin.add_view(RedisCLI1)
admin.add_view(RedisCLI2)

Even though RedisCLI1 and RedisCLI2 are different views, they both have the same entry point (index) with the same URL (/rediscli).

Thinking RedisCLIView comes from SQLAdmin, will cause unexpected behavior because the index method is already registered.

Describe the solution you would like.

Solution

Every view should have its own Router and its exposed methods should be registered to that Router.

Describe alternatives you considered

I'm actively using Starlette Admin, it doesn't have a built-in expose method and it has a similar structure to SQLAdmin. The problem that exists here also exists there.

I tried to adopt Flask Admin to work with Starlette but that's not an easy job 😆.

Additional context

Other Problems

Since the index, list, details, delete, create, edit, export, and ajax_lookup endpoints are registered directly to the main Starlette application instance, we can't override them for each view.

routes = [
Mount("/statics", app=statics, name="statics"),
Route("/", endpoint=self.index, name="index"),
Route("/{identity}/list", endpoint=self.list, name="list"),
Route(
"/{identity}/details/{pk:path}", endpoint=self.details, name="details"
),
Route(
"/{identity}/delete",
endpoint=self.delete,
name="delete",
methods=["DELETE"],
),
Route(
"/{identity}/create",
endpoint=self.create,
name="create",
methods=["GET", "POST"],
),
Route(
"/{identity}/edit/{pk:path}",
endpoint=self.edit,
name="edit",
methods=["GET", "POST"],
),
Route(
"/{identity}/export/{export_type}", endpoint=self.export, name="export"
),
Route(
"/{identity}/ajax/lookup", endpoint=self.ajax_lookup, name="ajax_lookup"
),
Route("/login", endpoint=self.login, name="login", methods=["GET", "POST"]),
Route("/logout", endpoint=self.logout, name="logout", methods=["GET"]),

The user might need to override the list method in the ModelView but might require different logic per model. So the user must override Admin directly.

@hasansezertasan
Copy link
Contributor Author

hasansezertasan commented May 17, 2024

I'm not entirely sure and I didn't think thoroughly but this approach might solve #681.

@hasansezertasan
Copy link
Contributor Author

What I learned so far

Flask Admin Routing

As I said, Flask Admin creates a dedicated blueprint for each view and they are all registered directly to the application.

Consider two views: UserView and PostView. UserView has a blueprint named user_view and PostView has a blueprint named post_view.

Flask Admin also registers an AdminIndexView by default and its blueprint is named admin.

In Flask Admin, every view has an exposed method named index_view.

So the URL-building process is like this:

  • /admin/: url_for("admin.index_view")
  • /admin/user: url_for("user_view.index_view")
  • /admin/user/new: url_for("user_view.create_view")

Flask Admin registers AdminIndexView before all other views. That's why I failed when I tried to adopt its core to use Starlette. AdminIndexView occupies all URLs starting with /admin/* when I use Mount.

Starlette Routing

Mount builds an application (if not provided), and the URL that is used while mounting an application, router, or group of routes is occupied by this application.

Route naming: we can name our routes with a colon separator.

admin_routes = [
  Route(..., name="admin:index_view"),
  Route(..., name="admin:user_view:index_view"),
  Route(..., name="admin:rediscli1:index_view"),
  Route(..., name="admin:rediscli2:index_view"),
]

Conclusion

Each view gets registered (mounted) to the main application. Not to an application to be mounted to the main application.

Each view has a dedicated blueprint (Router or Mount instance) and each exposed function gets registered to this blueprint.

Questions

  • Do we need something like AdminIndexView?
  • Do we need a separate application instance to mount admin to the main application?
  • We can name expose functions like rediscli:index_view and they'll be directly registered to the SQLAdmin application and will be accessible by doing url_for("admin:rediscli:index_view") but what about the URLs?
  • How does moving list, details, delete, create, edit, export, and ajax_lookup methods to the views as exposed methods sound like? This is the Flask Admin approach to the problem.

In the end problem remains: View can not have a name prefix... So if the user has two custom views with the same exposed URLs, there will be conflicts...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant