search

burningmantech / ranger-clubhouse-api

Ranger Secret Clubhouse API Service
Apache License 2.0
12 stars 8 forks source link

Too many settings in the DB #76

Closed wsanchez closed 6 months ago

wsanchez commented 5 years ago

It appears that we have put every possible setting into the DB.

I think that's a bad idea, and that we should only put settings into the DB when they actually override the defaults in code. Otherwise, we're just duplicating the settings, which is not great, and edits in code never apply in any deployment, which then implies that they shouldn't be in code at all.

As it stands, in order for a change to config/clubhouse.php to matter, a corresponding database migration (ew!) or manual settings change (argh!) is required.

As an example, this change should be sufficient, but I suspect is not.

wsanchez commented 5 years ago

Note "when they actually override the defaults in code" may be too strict; we should use the production Lambase server in our production environment, regardless of the default in code, so perhaps that's an exception, since the code shouldn't really know about production defaults anyway.

wsanchez commented 5 years ago

Actually, I think that any variables set in any environment should be done manually, and so setting any in a DB migration step seems like it's stepping on the app admin. Perhaps we can un-migrate? I'd like a fresh DB not to have pre-inserted settings and leave that to be done by the admins. Does that make sense?

mikeburg commented 4 years ago

This pull request addresses the concerns: https://github.com/burningmantech/ranger-clubhouse-api/pull/263

The config/clubhouse.php file was cleaned up (gutted) and only two variables remain - DeploymentEnvironment and GroundhogDayServer - each is directly related to the platform the website is deployed on, and should not be editable by an application admin interface.

The settings definitions and migration has been relocated into the app/Models/Setting.php which holds the definition of each setting (type, description, available option values) but does not hold the setting value.

The settings defaults now live in a seeder file (database/seeds/SettingsTableSeeder.php) which may be used to populate a test database if so desired Note: the seeders are NOT used for unit testing. The unit tests want a current schema and an unpopulated database.

The config/clubhouse.php was only meant to be a temporary bridge between the CH1 configuration scheme, and the CH2 settings scheme. The commit should clear up the confusion of where a setting is pulled from.

mikeburg commented 6 months ago

Closing out. Any critical settings related to where the Clubhouse is running, i.e. on playa versus off playa, is handled through environment variables. Examples are SMTP creds, database credits, photo location, etc. Other settings stored in the database are only viewable by the Tech Ninja permission, and will be blanked out with any database redactions.