-
-
Notifications
You must be signed in to change notification settings - Fork 29
Fix isFeatureEnabled returns false when multiple strategies are configured #33
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
Fix isFeatureEnabled returns false when multiple strategies are configured #33
Conversation
Thanks for the PR @DanTheDJ. I just had a couple of comments but need to test this out. |
Change order of strategies returned by mock unleash server response
@DanTheDJ I'll get round to a review when I can, pretty busy at the moment with work (and other things, believe it or not) and I'd still like to test this end-to-end. |
@DanTheDJ Just getting round to test this e2e but struggling to think about how to set this up on the Unleash side. Working backwards, how are you detecting whether your user should see the toggle in your app? |
Hi @mikefrancis The way we have this configured is that we have a feature toggle which can be made available to users by either:
Here is the setup on our unleash server: This means that any of the individual strategies evaluating to true causes the user to see the toggle within the application. The way this is configured on the Laravel side (running with the PR branch) is as follows: The When the Unleash feature is evaluated, it will check both Let me know if this clears it up, if not I can provide some further examples |
@DanTheDJ Thanks for this. I'm looking for how you are checking whether to show the feature in your app, are you using Blade, the |
@mikefrancis, currently when checking in view files, I have a custom blade directive which then passes through to a helper class I have for tenancy. In Blade::directive('hasFeature', function ($featureCode) {
return "<?php if (\Domain\Tenancy\Helpers\PlanFeatureHelpers::hasFeatureCode($featureCode)): ?>";
});
Blade::directive('endHasFeature', function () {
return '<?php endif; ?>';
}); The helper class just uses the <?php
namespace Domain\Tenancy\Helpers;
use MikeFrancis\LaravelUnleash\Unleash;
class PlanFeatureHelpers {
static function hasFeatureCode($featureCode)
{
$unleash = app(Unleash::class);
$enabled = $unleash->isFeatureEnabled($featureCode);
return $enabled;
}
} I also use this helper around my application to check the eligibility of features via the static method of the Helper, so I guess you could say I only ever use the Unleash facade for my checks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this one @DanTheDJ
Hi there, firstly thanks for a great package. I am currently implementing Unleash to manage feature access in my application.
I have come across an issue when a feature has been configured with multiple different activation strategies. In my app, there are multiple scenarios under which I'd like a user to gain access to a feature e.g:
What I am experiencing is that if any of these strategy evaluations fail, then the function returns the feature is not available.
From the unleash documentation (https://docs.getunleash.io/user_guide/important-concepts)
I have found by modifying the isFeatureEnabled function, I can achieve the expected behaviour according to the unleash docs. I have also added a new test which now passes.