Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Draft
wants to merge 23 commits into
base: 1.x
Choose a base branch
from

Conversation

cosmastech
Copy link
Contributor

@cosmastech cosmastech commented Aug 17, 2024

This should resolve #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.

Copy link

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.

@cosmastech cosmastech force-pushed the no-error-when-feature-does-not-apply-to-scope branch from 5663d44 to c89423f Compare August 17, 2024 02:07
@cosmastech cosmastech marked this pull request as ready for review August 17, 2024 02:36
@cosmastech cosmastech changed the title No error when feature does not apply to scope [1.x] Allow for retrieving all features for a scope when some features are defined for differing scopes Aug 17, 2024
@cosmastech cosmastech marked this pull request as draft August 17, 2024 10:41
@cosmastech cosmastech marked this pull request as ready for review August 17, 2024 12:35
@cosmastech cosmastech changed the title [1.x] Allow for retrieving all features for a scope when some features are defined for differing scopes [1.x] Allow retrieving all features for a scope when some features are defined for differing scopes Aug 17, 2024
@taylorotwell
Copy link
Member

Drafting pending review from @timacdonald

@taylorotwell taylorotwell marked this pull request as draft August 17, 2024 20:59
@timacdonald timacdonald self-assigned this Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using multiple scopes across features disables use of all() and values()
3 participants