immich
immich copied to clipboard
feat(web): upload json config
Adds "Import from JSON" button as discussed here. My background is in embedded software, not web programming, so a review is very appreciated.
An open question is how to handle saving of the configuration. Currently the data imported from the JSON file is just put on the UI, but not saved. I believe this is counterintuitive and we should directly save after importing. In that case, it seems what we should do is call handleSave in admin-settings.svelte, but at that point my lack of knowledge about how Svelte works shows up and I'd appreciate some guidance.
From what I see here, you are just reading the JSON file and have not uploaded the configuration to the server yet, correct?
Correct, currently it only automates setting the values on the web UI from the JSON file.
Thanks!
I'd prefer if it'll automatically make a request to the server and update the config though.
I agree. I can either copy the code from handleSave (not very elegant) or call the function, but I couldn't figure that out (again, never used Svelte) in my limited time this morning. I'll try to have a look again later.
Thanks!
I'd prefer if it'll automatically make a request to the server and update the config though.
I agree. I can either copy the code from
handleSave(not very elegant) or call the function, but I couldn't figure that out (again, never used Svelte) in my limited time this morning. I'll try to have a look again later.
FWIW I don't think there's much you need to do. You should be able to just call the API function and wrap it in a try catch, there isn't much else to it since what you get is already in JSON. So I'd be fine with just having that API call here as well
It seemed to me like handleSave was doing more than just calling the API (ex. showing a notification, etc), so I just figured out how to call it instead.
that probably won't work. the config file might be a partial but the update method requires the entire thing
that probably won't work. the config file might be a partial but the update method requires the entire thing
I believe that's what this code in handleSave does:
const newConfig = await updateConfig({
systemConfigDto: {
...savedConfig,
...update,
},
});
that's not a nested deep merge, that only overrides entire config sections
I'm fine if we just want to post the whole config for now though. At least export full config and import full config would work correctly. It is quite a bit more complicated to do anything else at the moment.
that only overrides entire config sections
Ah yes, that's a good point. Handling that requires changing that code in the handleSave function.
When you need to use bind that usually shows bad data modelling
Right, I'm not familiar with Svelte best practices but it did seem like this functionality would be better outside of the child component. On the other hand a bind for the config variable is also how the copy to clipboard and the download JSON are implemented and I wasn't going for large changes.
Is there anything else you'd like me to change on this PR?
Is there anything else you'd like me to change on this PR?
No, I will do some functional testing and will get it merged