statamic / eloquent-driver

Provides support for storing your Statamic data in a database, rather than flat files.
https://statamic.dev/tips/storing-content-in-a-database
MIT License
104 stars 71 forks source link

Globals not editable if entry is missing in database #303

Closed rrelmy closed 1 week ago

rrelmy commented 2 weeks ago

Bug description

Our goal is to keep globals definitions in the repository (driver: file) but store the global variables in a database.

When a new global set is added and deployed to a machine with a db that does not contain an entry in the global_set_variables table for it. Content manager have no possibility in the CP to edit, they only can access the edit page of the global set, the /cp/globals/{slug} page shows a 404.

The only way to make it editable seems to be running php please eloquent:import-globals but that overwrites all other variables in DB with data from the files which is undesired. There seem to be no way to populate only missing ones without overwriting data.

Am I missing something or do I use the driver in a wrong way?

How to reproduce

Standard config, with only variables stored with eloquent driver

    'global_sets' => [
        'driver' => 'file',
        'model' => \Statamic\Eloquent\Globals\GlobalSetModel::class,
    ],

    'global_set_variables' => [
        'driver' => 'eloquent',
        'model' => \Statamic\Eloquent\Globals\VariablesModel::class,
    ],

Create a global set

// delete item from table
php artisan migrate:fresh

Logs

No response

Environment

Application Name: Statamic
Laravel Version: 11.10.0
PHP Version: 8.3.8
Composer Version: 2.7.7
Environment: local
Debug Mode: ENABLED
URL: localhost
Maintenance Mode: OFF

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: log
Cache: statamic
Database: sqlite
Logs: stack / single
Mail: log
Queue: sync
Session: file

Statamic
Addons: 1
Sites: 1
Stache Watcher: Enabled
Static Caching: Disabled
Version: 5.8.0 Solo

Statamic Addons
statamic/eloquent-driver: 4.3.0

Statamic Eloquent Driver
Asset Containers: file
Assets: file
Blueprints: file
Collection Trees: file
Collections: file
Entries: file
Forms: file
Global Sets: file
Global Variables: eloquent
Navigation Trees: file
Navigations: file
Revisions: file
Taxonomies: file
Terms: file
Tokens: file

Additional details

No response

ryanmitchell commented 2 weeks ago

Yes unfortunately this is the case in single site setups as in file based mode the global and variables is one file.

We will take another look at it but it’s not easily solved to be honest.

ryanmitchell commented 2 weeks ago

Ok so if you create one through the CP it does automatically create a new variables row, so for me this is a devops issue in that you need to write something to verify you have rows in your db (or make a migration/seeder) for any newly added globals. I don't think this is something we should be solving in this driver.

rrelmy commented 2 weeks ago

So this only affects single site but not multi-site setups?

ryanmitchell commented 2 weeks ago

@rrelmy no, with your workflow it will affect all setups as you arent seeding the database with the new global variable row

rrelmy commented 2 weeks ago

I looked into how I would seed the database with missing entries.

The easiest to me seems to add an option (--no-overwrite-variables) to the import-globals which would disable the ->fill( call when the variable is created https://github.com/statamic/eloquent-driver/blob/cad4c5194d05f00c8085b1f0bf5129c9ff073b3f/src/Globals/Variables.php#L41

This would allow running php artisan statamic:eloquent:import-globals --only-global-variables --no-overwrite-variables to seed the database with missing entries without overwriting any existing content in the database.

Would you accept a pull requests which would implement such an option?

ryanmitchell commented 2 weeks ago

It seems pretty niche so I’d be inclined to say no - your usage is not what the imports are intended for.

Can you not just make a seeder that gets any global (using the facade) then checks if a variables model for that handle exists and if not creates it?

rrelmy commented 2 weeks ago

I am not sure if I miss something, but isn't everyone that has a site live on a server, and then adds another global set locally during development hitting this problem?

The addon already supports a similar use-case with the sync-assets command, I could also create a separate command to be consistent with the concept.


Implement a seeder outside the addon is certainly possible, but I thought it would be worth asking first before I do almost 1:1 re-implementation of the ImportGlobals.

I did not investigate if I could not seed and prevent the 404 in CP by doing it at runtime.

ryanmitchell commented 2 weeks ago

The sync assets is a regular running task designed to keep a regularly updating store up to date, it's not quite the same as what you are wanting.

Something like this should do the trick - just run it on deploy. You'll see its quite different to the import command.

<?php

namespace Statamic\Eloquent\Commands;

use Illuminate\Console\Command;
use Illuminate\Support\Facades\Facade;
use Statamic\Console\RunsInPlease;
use Statamic\Eloquent\Globals\VariablesModel;
use Statamic\Facades\GlobalSet;
use Statamic\Facades\Site;

class SyncGlobalVariables extends Command
{
    use RunsInPlease;

    /**
     * The name and signature of the console command.
     *
     * @var string
     */
    protected $signature = 'statamic:eloquent:sync-global-variables';

    /**
     * The console command description.
     *
     * @var string
     */
    protected $description = 'Create models for global sets that don\'t exist.';

    /**
     * Execute the console command.
     *
     * @return int
     */
    public function handle()
    {
        $sets = GlobalSet::all();

        $this->withProgressBar($sets, function ($set) {
            if (VariablesModel::where('handle', $set->handle())->first()) {
                return;                
            }

            $set->makeLocalization(Site::default()->handle())
                ->data([])
                ->save();
        });

        $this->newLine();
        $this->info('Variables synced');
    }
}
rrelmy commented 1 week ago

Thank you very much, it works perfectly.