-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
FEATURE: Stronger data validation + defaults on PUT /api/settings
#1885
Comments
PUT /api/settings
Hello, |
I am using Ansible for this usecase for sending HTTP requests. Here is what I am actually doing :
Please notice that I MUST redeclare defaults like |
Does this happens with only SMPT and bounce use case? Or it can happen with any field |
Any field, really. Like with my example above, if i run this script minus the |
Got it. |
I assume that while updating settings, frontend is providing all the attributes(regardless its new updated values only). So this type of bug won't happen through UI. The problem arises when we try to update this manually and misses out some field which are required by UI. For ex. in bounce case if we miss out |
Seems legit for a PUT verb. What would be nice for most usescases regarding updating settings, is to enable POST verb, and allow atomic properties replacement through it. With this, we could just let actual settings as it and only update those that we wish to change. |
Yes, we could actually make this change to PUT method only, as this API will use to updating setting. Thought of this as my second approach but will require lots of code change. I'm fine with both the approaches. |
@Amphaal can you confirm this ? Will start with development |
Just do what makes the more sense to you, as long at it makes the usage safer and documented :) |
Actually, there is only PUT verb enabled on this path, which will result in replacing the whole setting file: some settings in body are explicit requirements (as the API tells us if we do not set them), but some more that are not required by the API are actually required for the UI to keep functionning !
For example, if you call
PUT /api/settings
and missing some parameters configuring an SMTP server, the Settings's SMTP tab goes completely blank, same for bounces.Can we have defaults values where required if expected properties are missing in the body ? If not, telling us with
HTTP 500
that some properties are missing and need to be set, with somehow documented possibles values in Swagger ?The text was updated successfully, but these errors were encountered: