spatie
Make missing default values check on save configurable
Based on https://github.com/spatie/laravel-settings/pull/313#issuecomment-2829593696, this PR adds a config option to skip checking non-migrated default values when saving settings.
Reasoning
The idea is to allow using saving settings even if migrations take a while to complete, or when one does not want to use settings migrations at all.
The latter may sound odd at first, but when Settings classes declare default values, it would allow adding new settings without creating migrations. The new settings with their default values would be "migrated" just-in-time, when the user first saves the settings. This is especially helpful in multi-tenancy apps, where each tenant (user) has their own set of settings.
Backgrounnd
I don't really see a reason to throw an exception when trying to save settings with default values, if there's nothing in the database 🤔 ... Perhaps I am missing something, but this PR does not change the default behaviour, it only provides a lever for those of us who like to live on the edge 😂
Naming and docs
Please note that there's no docs yet, as I wanted to gauge if a PR like this would even be considered, first. If the general approach is acceptable, we can discuss naming and implementation details, and then write the docs.
Misc
At first, I thought of using a generic throw_on_missing_settings config value, but realized that would mean if a settings class does not have default values defined, then accessing such a property, or converting the class to an array would still throw a runtime error, so it wouldn't really buy anything.
I use this package together with filament/spatie-laravel-settings-plugin. In my case, most settings were always null by default. Before v3.4.3, everything worked and did not require extra time spent on creating and managing migrations.
Making throwing an exception optional would be very useful. Perhaps without it, many, like me, have to stay on v3.4.2.
@freekmurze This is really helpful for Filament with multi-tenant projects, hope this PR gets merged
While probably controversial, I'm not going to merge this.
The whole idea behind the package was that you defined a settings class with properties which should map 1:1 to your database. I don't want to lose that strictness and actually see the possibility of saving settings without migrations more as a bug than a feature.
The docs clearly state that every settings property needs a migrated default value.
As for multi tenant setups, I would guess you always need to setup a tenant, this feels like an excellent place to seed this data. Even if it takes a long time for migrations to run on a multi tenant single database app. I think if such things happen you would actually need to put the tenant in maintenance mode in order to not create conflicts in state.
Either way, if we would merge this other bug reports may also pop up since allowing saving values without running migrations may cause that migrations (taking a long time) later on may crash due to settings already saved.
