-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
fix(ProvisioningAPI): set typed config values by via API #46991
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a ok solution for now. We probably need a better way to fix this in the future as you said.
a634b01
to
37cde7b
Compare
Hi, I'm impacted by #45083 so I'm eagerly watching this PR. I get the same type conflict error when I do a
@kesselb recommended that I mention this particular case here because it might not be solved by this PR in its current form. Is there any chance you could fix that case as well? |
Hm, on occ there is should be another entrypoint somewhere. |
6025810
to
c747aff
Compare
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
c747aff
to
a3cd963
Compare
rebased, adjusted and added tests (no prod code change since) |
/backport to stable30 |
/backport to stable29 |
Not scope of this PR, requires a follow up. |
Please no backports. this is a breaking change 👎 |
Fix in #47409 |
conflict between new type (mixed) and old type (string)
#45083Summary
On Javascript side we have one method at hand to set an app config value:
CP.AppConfig.setValue()
. As value it takes astring
and sends this over the Provisioning API to the server.Meanwhile we have typed configuration entries. All existing entries are default to the
IAppConfig::VALUE_MIXED
type. New entries however are bound to their type. For instance a new boolean value cannot be set via JavaScript anymore.This fix solves this on API level. All clients may still send strings, and the Provisioning API's Controller tries to cast the provided value into the target type.
I see this sort of critical myself as there is not good validation of the casted value. On the other hand I do not think it can do much harm, only fail in a different way. Still, an alternative would be to have also dedicated setter on the JS side, and endpoints for each acceptable type. Personally I don't have the time for this approach today anymore. I am totally fine to discard this PR in favor of a better replacement :)
Todo
Checklist