search

laravel / pennant

A simple, lightweight library for managing feature flags.
https://laravel.com/docs/pennant
MIT License
478 stars 49 forks source link

Add support for plain objects as scope #98

Closed jkatruska closed 7 months ago

jkatruska commented 7 months ago

Fixes https://github.com/laravel/pennant/issues/97

taylorotwell commented 7 months ago

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If possible, please consider releasing your code as a package so that the community can still take advantage of your contributions!

If you feel absolutely certain that this code corrects a bug in the framework, please "@" mention me in a follow-up comment with further explanation so that GitHub will send me a notification of your response.

jkatruska commented 6 months ago

Hi @taylorotwell, thank you for your response.

I still think this is a bug because according to the documentation Pennant's built-in array and database storage drivers know how to properly store scope identifiers for all PHP data types as well as Eloquent models. you should be able to use any data type as a scope, but as I mentioned in issue https://github.com/laravel/pennant/issues/97, only null|int|string|Model are valid scopes.

timacdonald commented 6 months ago

Please see my comment: https://github.com/laravel/pennant/issues/97#issuecomment-2031011469

denisvanmorgan commented 6 months ago

hi, any chance this gets more attention? IMO the proposed solution is more robust, without need to fetch user multiple times if you don't use eloquent model. not every app is necessarily using DB, e.g. BFF in microservice architecture, etc.