-
Notifications
You must be signed in to change notification settings - Fork 4
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
Badly Architected Resource CRUD handlers makes it impossible to customize them #14
Comments
So in summary you want to move the |
yeah, broken upstream as well. Like I said scenario is having the ability to class Model1Detail(ResourceDetail):
def before_get(self, view_kwargs):
if view_kwargs.get("display_name") is not None:
model1 = Model1.find_by(display_name=view_kwargs.get("display_name"))
view_kwargs["id"] = model1.id
# Doesn't work but included as example
# def before_patch(self, view_args, view_kwargs):
# if view_kwargs.get("display_name") is not None:
# model1 = Model1.find_by(display_name=view_kwargs.get("display_name"))
# view_kwargs["id"] = model1.id
schema = Model1Schema
data_layer = {
"session": ext.db.session,
"model": Model1,
"methods": {"before_get": before_get},
} with this as a view api.route(
Model1Detail,
"model1_detail",
"/api/model1/<int:id>",
"/api/model1/<string:display_name>",
) so now I can
I'd have to handle the backend of doing the look-up to enable this to function but at the end of the day, I save a good amount of work by enabling string buckets thru flapison and not having to manually define a thousand endpoints for specific use cases |
I think this is breaking unit tests tho, if you have a better way of approaching this, let me know and I can work on it in some spare time |
If you want to access resources using a name rather than an ID, could you not just override Having said this, I still think this is a relevant issue, because there might be use-cases where there is no alternative approach. |
Man, those functions aren't really well documented as existing, are they? I honestly keep getting more functionality from src diving than reading the docs |
You're right though, but I do also agree, the before_x should be ran before x as expected because some extra work might want to be done before said function is called and not rely on data that can't be modified within said functions |
I would welcome a documentation PR. Also I really need to create a documentation website build like they have upstream. |
For sure, so looking into this issue more, it doesnt look like any before_x_object or before_x_relationship functions will solve this as its throwing the error before those are called in the stack and is pulling the data its checking directly from the request.json() so even if it was being called before, I wouldn't have the ability to modify it Was super pumped to resolve the block haha |
Can you post an actual trace of the error you're getting? Also, would |
Planning on submitting a PR to resolve this but I wanted to make this a possible function of the library:
GET /api/label/<string:label_name>
along with the ability to create relationships this way too to cut down on my frontend needing to make an extra get request to match up IDs to functions.
The issue comes in ResourceDetail's
patch
function:https://github.com/TMiguelT/flapison/blob/master/flapison/resource.py#L327
The
get
function allows the kwargs to be modified in thebefore_get
function but its broken inpatch
if you're trying to modify the kwargs in eitherbefore_patch
orbefore_marshmallow
due to this function (for some reason) relying solely on data from the request and now enabling it to be modified before doing queries on it.https://github.com/TMiguelT/flapison/blob/master/flapison/resource.py#L353
Proposal:
Move
before_patch
to the top of the file as it should run before the patch function does.Note:
This is the same issue for ResourceList's
post
function so I'll be submitting a fix for bothhttps://github.com/TMiguelT/flapison/blob/master/flapison/resource.py#L222
The text was updated successfully, but these errors were encountered: