[1.x] Allow retrieving all features for a scope when some features are defined for differing scopes

[cosmastech]

This should resolve https://github.com/laravel/pennant/issues/112

We are building a feature flags endpoint for our mobile and web consumers. We need to be able to get all features for 3-4 scopes which are related to the user, but not the user object itself.

I ran into this exact issue earlier today. I saw that this issue had been open for a few weeks without a PR so I figured I'd take a stab at it.

---

Given we have features defined for different scopes

Feature::define('feature-for-user', fn(User $u) => true);
Feature::define('feature-for-team', fn(Team $t) => true);

When we attempt to load all features for a particular scope

$features = Feature::for($user)->all()

Then we expect to receive feature flag definitions ONLY for that apply to Users.

dd($features);
/*
array:1 [
  "feature-for-user" => true
]
*/

Currently, however, an exception is thrown: TypeError: Tests\Feature\DatabaseDriverTest::Tests\Feature{closure}(): Argument #1 ($t) must be of type Workbench\App\Models\Team, Workbench\App\Models\User given, called in /Users/luke/Projects/laravel-pennant/src/Drivers/Decorator.php on line 173

Non-goals of this PR

One outstanding question is: do we expect to allow for union or intersection types in the resolving function? Like Feature::define('team-or-user-feature', fn(Team|User $v) => true); Seems like this probably wouldn't work with how scopes are set up in general. I am sure that my change will not work in this case.

Also, you can still call Feature::for($user)->activate('team-scoped-feature'); I believe that ideally there would be a guard for this, but it seems like it's beyond the scope of this PR.

[github-actions[bot]]

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.

[taylorotwell]

Drafting pending review from @timacdonald

[timacdonald]

@cosmastech, I have re-worked this one. There were a few problems with the original implementation. The main issue was when a value was resolved for a bad type, it was saved to the database.

The new implementation does the check before attempting to resolve. Supports union and intersection types.

What do you think?

Feature::for($scope)->all() and Feature::for($scope)->loadAll() will now only return or load the features for that scope type.

[cosmastech]

@cosmastech, I have re-worked this one. There were a few problems with the original implementation. The main issue was when a value was resolved for a bad type, it was saved to the database.

The new implementation does the check before attempting to resolve. Supports union and intersection types.

What do you think?

Feature::for($scope)->all() and Feature::for($scope)->loadAll() will now only return or load the features for that scope type.

Thanks for working on this @timacdonald. Sad you removed my given-when-then 😆 but otherwise, nice.

I didn't run any local testing on this, but it seems to solve the problem. Good catch on the DB queries, didn't consider that fact.

[erikaraujo]

Hey all,

I'm still trying to figure out the exact reason (since I've never looked into this codebase before), but this PR definitely has breaking changes that should be noted in the release notes (ideally should've been release in a version 2, I think).

Again, I didn't dig too deep into the issue yet, but after this change was merged when I do \Laravel\Pennant\Feature::all() I get no results back.

I was using null scopes for a "global" FeatureFlag that is not tied to any users.

[timacdonald]

@erikaraujo, if you could open an issue with reproduction steps, I would be happy to look at the issue for you and see if we can get it resolved.

[erikaraujo]

@erikaraujo, if you could open an issue with reproduction steps, I would be happy to look at the issue for you and see if we can get it resolved.

Hey @timacdonald ! Thanks for the reply. I'll open an Issue later with more details, but basically up to version 1.12 if you had any Feature Flags created you could list them by doing Feature::all().

Now, that same call returns just an empty array.

[timacdonald]

The Feature::all() method should continue to return the available features for the current scope.

Would appreciate some replication steps to see the issue you are currently facing to understand it better.