Proposal: version control for BREAD configuration (WIP)

joshuadegier commented 6 years ago

Laravel Version: Any compatible
Voyager Version: 1.x.x
PHP Version: Any compatible

Also mentioned in:

And probably a few others

Problem:

When creating a BREAD configuration for a table, the settings are stored in the database. This makes it hard for teams to work together on a project without exporting/importing these tables settings or maybe sharing full database back-ups.

Presented solutions in the issues above used migrations and seeds to share this data. While this indeed is a valid solution to share the data, I don't think this is the best solution for people working in teams. Why? As long as you are developing locally or maybe even on the staging/testing environment it's perfectly fine to drop a table or truncate a table and re-create or re-seed these. But when the project is in production we can't drop a table and rerun a migration holding the full table since we could lose the data this table is holding.

Development

I'm developing the solution presented below, you can find it at https://github.com/PendoNL/voyager. As long as I haven't said it's ready for testing here, use it at your own risk and until it's properly tested: don't use it in any of your live projects.

Solution:

I've started working out an idea, roughly what I'm going to try to do is:

(1) Change the BreadAdded and BreadEditted events to receive $request->all() besides the $dataType and $data
(2) Use the BreadChanged event to dispatch a SaveBreadConfigurationToFilesystem listener
(3) The json_encode($event) (I guess I also need serialization here) is then saved to {$event->dataType->slug}.json in a new directory (database/bread/) which is published when installing voyager

The above is working so far. Next steps are to create a new refresh method on the VoyagerDatabaseController which deletes the current data (safely) and then adds the data from within the configuration files. This is all wrapped within Database Transactions to prevent false data when something goes wrong along the process. Also I created a setting enabled_bread_configuration_version_control which can be disabled if there's no need to save the configuration to the file system.

There still are a few things I need to be aware of:

I need a way to compare the filemtime of the newest saved configuration to the updated_at timestamp of the last editted record in data_types. This can be used to determine wether a user needs to refresh the BREAD configurations to stay in sync with his/her team. Also, I have to take timezones into account I guess.
To prevent duplicate configurations being saved when the slug is changed for a BREAD configuration. I need to find a better solution which can keep track of this change and remove the old file from the database/bread directory. Solution: the $dataType->name field holds the database name (and is not changeable). There still is a situation where a BREAD configuration can be duplicate, but I suppose we can check the existance of a table before adding the configuration.
When refreshing the configuration I need to use the delete method of VoyagerDatabaseController (also need to use storeBread and updateBread of that same controller), some of the logic of these methods need to be extracted to a function to be used in the refreshBread method. I found a way to check the existence of a DataType and then add or update accordingly, this way there is no need (yet or for ever?) for deleting a DataType from the database.
In order to prevent updates of the BREAD in production, the setting enabled_bread_configuration_version_control can be used as wel. Being true, this would disable the ability to EAD the BREAD configurations in production.
A new commando voyager:refresh-bread-config can be run from within the console in production in order to have the latest configuration loaded.
A separate, but in similar, problem for version control exists for the Admin menu.
In order to have valid migrations in version control, I propose to disable the database add/edit methods when enabled_bread_configuration_version_control is set to true. This forces the developers to use the native make:migration which can hold changes to tables as well.

I can see that this might add a few steps to development since you can't always use the graphical interface of Voyager. But I don't think there is another way to keep these settings in version control and shareable between teams. Besides, having the enabled_bread_configuration_version_control setting you can still unleash full control using the GUI if set to false.

joshuadegier commented 6 years ago

I've got this working, it still needs fine tuning and the correct checks but the setup works for now. So far what I did is:

Added new event Listener to save BREAD configuration

I added a new Listener (SaveBreadConfigurationToFilesystem) which receives all data as received by the BreadChanged event. I had to alter the BreadAdded and BreadEditted Events as well to have them receive the full Request a user sends.

The new Listener then saves the data ($request->all() and a serialized object of $dataType) in a json file which is stored under database_path("bread/{$dataType->name}"), for instance: database/bread/pages.json.

Refactored VoyagerDatabaseController.php

To prevent duplicate code in the controller I refactored this file a bit, I extracted the code used by storeBread and updateBread which do the actual saving into a seperate method. Then I added a new refreshBread method. This method does the following:

First it checks permissions to browse_database
It then finds all .json files in the database_path('bread') folder
It decodes and unserializes the data from the json's contents
It checks if the database (still) exists, if not nothing happens
It then checks if the dataType exists (using $dataType->exists() from the unserialized model)
Based on existence it either updates or adds the BREAD DataType and all attributes to the database

Demo proof

The above set-up is demo proof, since it doesn't delete all records from the data_rows table but it checks for existence and updates or adds accordingly. This way the default settings are safe when refreshing the BREAD configurations, only configurations made or editted after installing Voyager end up in the database/bread folder as restoreable configuration.

Still to do

[ ] Find a valid way to compare the timestamps to determine if a refresh is necessary or not
[ ] Decide whether or not we want the configuration files to be delete if the table wasn't found anymore (alternative is to just skip this configuration and keep it in version control)
[ ] Disabling editting BREAD configuration in production
[ ] Disabling using the database add/update when version control for BREAD is enabled
[ ] Create the voyager:refresh-bread-config command for use on the CLI
[ ] What to do when a BREAD gets deleted

How to test this code

To test this code, my repository should be added to your composer.json file as such:

"repositories": {
        "0": {
            "type": "VCS",
            "url": "git@github.com:PendoNL/voyager.git"
        }
    },

After adding the repository, you can use the following command to pull the latest version (made a little mistake with the branch name, but it works):

composer require "tcg/voyager:dev-dev-pendo"

Any feedback is welcome! :-)

theduke commented 6 years ago

Just want to provide big support for this.

It's exactly what every project that is shared between developers or has a proper CI setup needs.

tobz-nz commented 6 years ago

Me too, massive support for this.

The lack of version control of project config/settings is why I moved away from Expressionegine, CraftCMS and a few others. This feature is critical in my opinion.

fletch3555 commented 6 years ago

Fair warning with developing this (sorry, not sure how I missed it last month...). We're in the process of rebuilding the BREAD builder (and a number of other related features) from the ground up, so it's entirely possible this won't port over well. I just don't want the work to be in vain.

tobz-nz commented 6 years ago

Your rebuilding it? I'd hope it'd be file based by default in that case - saving this stuff in the DB makes CI/CD or any sane deployment incredibly hard. I've been doing it with Seeders, but a clunky hack. Flat file would be MUCH better.

</two-cents>

bbrala commented 6 years ago

I have to agree here, having an export/import possibility for bread configuration is essential to get things running properly on CI and making it realistic to use in teams.

joshuadegier commented 6 years ago

I've stopped developing this since you already told me that on Slack @fletch3555, feel free to take anything I made for your benefit if it's usable. I've stopped using Voyager because it simply wasn't what I was looking for after all and we're going to build something custom. Thumbs up for all your efforts tho!

terion-name commented 6 years ago

@fletch3555 will this new BREAD builder use file-based configuration, usable for version control and proper deploys?

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have further questions please ask in our Slack group.

thedevdojo / voyager