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

Model preffix is not lowercased in TortoiseCRUDRouter #64

Closed
marcoaaguiar opened this issue Apr 22, 2021 · 3 comments · Fixed by #65
Closed

Model preffix is not lowercased in TortoiseCRUDRouter #64

marcoaaguiar opened this issue Apr 22, 2021 · 3 comments · Fixed by #65
Labels
bug Something isn't working

Comments

@marcoaaguiar
Copy link

marcoaaguiar commented Apr 22, 2021

When using TortoiseCRUDRouter, the CRUD routers generate a base prefix that has the same casing as the db_model.

For instance, for a router with prefix "/api", and an object CRUD router defined by

router.include_router(TortoiseCRUDRouter(schema=UserPydantic, db_model=User))

The generated CRUD route that appears in the /doc is /api/User, which is quite odd for APIs.

This issue seems to come from the interaction of these 2 lines

prefix=prefix or db_model.describe()["name"].replace("None.", ""),
and
prefix = str(prefix if prefix else self.schema.__name__.lower())

Since the prefix is defined in the TortoiseCRUDRouter, it is never lowercased

This might be intended. Anyway, this behavior can be circumvented by passing prefix to the TortoiseCRUDRouter.

@awtkns
Copy link
Owner

awtkns commented Apr 22, 2021

Hi @marcoaaguiar thanks for reporting this.

In #65 I've changed it so that all prefixes are lowercase. Even if you specify prefix="MyModel" it would turn into "mymodel". That being said, this could cause problems if someone wanted an uppercase path prefix (for whatever reason). Alternatively, I can just fix the tortoise issue,

Thoughts?

@awtkns awtkns added the bug Something isn't working label Apr 22, 2021
@marcoaaguiar
Copy link
Author

Hi Adam, I believe that would fix the issue.

I started using this package this morning, and it's great to see that it is under development/maintenance!
The only feature that seems to be missing is filtering, but it seems that soon it will be available (#61).

When #65 is merged, feel free to close this issue.

awtkns added a commit that referenced this issue Apr 23, 2021
@awtkns
Copy link
Owner

awtkns commented Apr 23, 2021

Awesome! Thanks for the feedback, filtering will be released soon 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants