Enhancement: Restorable Rich Feature Values

[inmanturbo]

As discussed with @timacdonald in #133 this adds a boolean column for the database driver to allow marking a feature flag inactive without erasing its previous value. This means that after a temporary rollback of a feature, a user's value for that feature can be restored upon reactivation, using the following new api:

Feature::activate('custom-font', 'Agu Display');

Feature::deactivate('custom-font'); // remembers 'Agu Display'

Feature::restore('custom-font', fallback: 'Arial'); // will restore to 'Agu Display'

This PR also allows restoring features for all scopes, in the same manner as Feature::restoreForEveryone()

Feature::activateForEveryone('custom-background', 'default-background');

Feature::for('robin')->activate('custom-background', 'sunny-afternoon');

Feature::deactivateForEveryone('custom-background');

Feature::restoreForEveryone('custom-background', fallback: 'default-background');

Feature::for('robin')->value('custom-background'); // sunny-afternoon

Includes tests, and events FeatureRestored and FeatureRestoredForAllScopes which are fired upon Feature::restore and Feature::restoreForEveryone, respectively.

Major changes:

• The major change in behavior here is that deactivating a feature will no longer change its value; it will simply mark it inactive. However this is done transparently and the same value as before is returned for inactive features (false).
• Also notable is that this adds two methods to a public interface. Maintainers of custom drivers will need to implement those methods in order to upgrade.
• This will require running a migration. The column is nullable and isset checks are used for a smooth upgrade; however there are no checks for the column's existence. Let me know if you want me to wrap those (queries) in a flag, to allow user code to continue run as is after upgrade but prior to running the migration.
• After upgrading and running the migration, existing usage of the original API will continue to function as is, without any code changes.

[github-actions]

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

[inmanturbo]

@timacdonald sorry #135 was a bit more of a POC so I've closed it. I've had some time to consider and I think this is a better approach. I'll start working on a PR for the docs if it shows any promise. There's a lot to review here I know, and it will likely require a version bump so I'm not expecting any rush.

I'm already using in my own project via an extension (of database driver) and a couple of macros which don't feel like much of a burden, so no worries at all on my end.

[timacdonald]

Appreciate it. I'm gonna mark this one as a draft so I have time to review before we push it to Taylor for final review and consideration. I have some ideas on how this could work since and just wanna make sure we align on how to make it happen. Will try and get to this next week.

[inmanturbo]

@timacdonald apologies for the ping, I know I said no rush but is there any news on this? I know you've been busy with wayfinder and I'm sure there are many other things as well. I Just ran laravel new again for another but related app that will need this and I'm debating whether I should re-implement my extension/macro combo, or just start pinning/tracking patches here until I get closer to a release point. I'm curious how/whether your ideas/changes will affect usage.

[timacdonald]

Hey! I'm still interested in making this happen. We are leading up to the Nightwatch release, so my time is limited right now.

After we get that out the door I will circle back to this one.

Sorry for the delay.

[inmanturbo]

No worries! Thanks for the update. Also looking forward to the nightwatch release! 👍

[inmanturbo]

@timacdonald in typical fashion, turns out what I wanted wasn't what I thought I wanted.

While I still believe this is a great feature, let me show you what it turns out I actually wanted:

Stateless features unless overridden by stateful features.

So how I achieved this is by using the array driver as my default driver, but checking with the database driver in the closure definition, something like so:

    public function boot()
    {
        Feature::driver('array')->define('custom-font', function (User $scope) {
            try {
                if (Feature::driver('database')->for($scope)->active('custom-font')) {
                    return Feature::driver('database')->for($scope)->value('custom-font');
                }

            } catch (QueryException $e) {
               //
            }
      
            return false;
        });
    }

• This allows me to remove or disable the feature by simply removing its definition (or wrapping the definition in a config check or business rule) without touching the database.

• Since I'm using the array driver as the default, regular feature checks (without specifying the driver) won't check the database unless the feature is defined in the manner above.

• If the feature gets defined again in the future, the users' preferences (set by the database driver) will still be intact.

• I can also do a "system reset" for one or all scopes with something like the following:

    public function resetDefaults(mixed $scope = null, ?string $name = null)
    {
        $query = DB::connection(config('pennant.stores.database.connection'))
            ->table('features');

        if ($scope) {
            $query->where('scope', Feature::serializeScope($scope));
        }

        if ($name) {
            $query->where('name', $name);
        }

        $query->delete();
    }