Skip to content
13 changes: 10 additions & 3 deletions src/SettingsServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Backpack\Settings\app\Models\Setting as Setting;
use Config;
use Illuminate\Routing\Router;
use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Schema;
use Illuminate\Support\ServiceProvider;
use Route;
Expand Down Expand Up @@ -36,9 +37,15 @@ public function boot()
$this->setupRoutes($this->app->router);

// only use the Settings package if the Settings table is present in the database
if (!\App::runningInConsole() && count(Schema::getColumnListing('settings'))) {
// get all settings from the database
$settings = Setting::all();
$tableExists = Cache::remember('backpack_settings_table_exists', config('backpack.base.cache_time', 60), function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you've used config('backpack.base.cache_time', 60). Is this a new config value you want to introduce? Since this is only used in Settings, I think it's better for it to be in a settings.php config file, rather than Base.

But I'm wondering - isn't it easier to just use the default cache time in the app? We're using the default cache store, so I think it's intuitive to use the default cache time too. Then we wouldn't need a settings config file at all 😄

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points, I did add that in my instance but I'll patch it.

return count(Schema::getColumnListing('settings'));
});

if (!\App::runningInConsole() && $tableExists) {
// get all settings from the database if they're not in the database.
$settings = Cache::remember('backpack_settings_cache', config('backpack.base.cache_time', 60), function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the same concern here - related to config('backpack.base.cache_time', 60)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've adjusted this and updated the PR to merge with current code. I couldn't find a default cache time in Laravel and have added this setting in the backpack.settings file.

It's also been updated with the suggested code from here: #90 (comment)

return Setting::all();
});

// bind all settings to the Laravel config, so you can call them like
// Config::get('settings.contact_email')
Expand Down