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

Use FastAPI validation tricks #10

Open
hbusul opened this issue Mar 8, 2021 · 2 comments · May be fixed by #34
Open

Use FastAPI validation tricks #10

hbusul opened this issue Mar 8, 2021 · 2 comments · May be fixed by #34

Comments

@hbusul
Copy link
Owner

hbusul commented Mar 8, 2021

In backend/apps/task/semester_routers.py, line 42 we have

  if (
        semester["dLesson"] < 0
        or semester["dBreak"] < 0
        or semester["slotCount"] < 3
        or semester["slotCount"] > 15
    ):

We should always validate user inputs before we process them. But it might be cleaner to do it using FastAPI features.
Here is the page describing validating body fields.
So instead of validating things in the function, we can validate things in the model and delegate FastAPI to do that.

In the model, it might be something like

dLesson: int = Field(..., gt=0)

They also mention the same tricks apply to query and path parameters.

@hbusul
Copy link
Owner Author

hbusul commented Mar 19, 2021

Why is this issue closed?

In backend/apps/task/semester_routers.py line 44 we still have

 if (
        len(resStartHour) != 2
        or int(resStartHour[0]) < 0
        or int(resStartHour[0]) > 23
        or int(resStartHour[1]) < 0
        or int(resStartHour[1]) > 59
    ):

We are still checking the input in create_semester and not using Fast API validation stuff. I'm reopening this issue, if there are commits that you forgot to push, please push them and close this issue.

@hbusul hbusul reopened this Mar 19, 2021
@hbusul
Copy link
Owner Author

hbusul commented Mar 19, 2021

I see that problem happens because hour input is a string but I guess we can handle this later. So closing the issue again.

@hbusul hbusul closed this as completed Mar 19, 2021
@emrdagkusu emrdagkusu reopened this Dec 31, 2021
@emrdagkusu emrdagkusu linked a pull request Mar 17, 2022 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants