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

Allow to enable feature flag based on request parameters #13

Merged
merged 1 commit into from
Sep 21, 2016

Conversation

filipgolonka
Copy link
Contributor

Described in #12

@wsromek
Copy link

wsromek commented Jul 28, 2016

👍

@richardfullmer
Copy link
Contributor

I took a look at this implementation today and wonder if we couldn't use existing functionality to accomplish a similar effect.

Example:

<?php

use Opensoft\Rollout\Rollout;
use Opensoft\Rollout\Storage\ArrayStorage;
use Opensoft\Rollout\RolloutUserInterface;

require_once __DIR__ . '/vendor/autoload.php';

class RolloutUser implements RolloutUserInterface
{
    public function getRolloutIdentifier()
    {
        return 1;
    }
}

$requestVars = ['FF_facebook'];
//$requestVars = [];

$rollout = new Rollout(new ArrayStorage());
$rollout->defineGroup('isInRequest', function(RolloutUserInterface $user) use ($requestVars) {
    return in_array('FF_facebook', $requestVars);
});

$rollout->activateGroup('chat', 'isInRequest');

if ($rollout->isActive('chat', new RolloutUser())) {
    print_r('yes');
} else {
    print_r('no');
}

Pros:

  • Doesn't require any new code
  • Leaves implementation details about how request variables drive feature toggling to implementors

Cons:

  • Requires implementors to handle request variables themselves.
  • Requires a RolloutUser. Groups only make sense in the context of a rollout user. Weird, cause the above implementation doesn't even look at the user while doing the "isInGroup" check.

Feature toggling based on query parameters is something we've considered internally, but doesn't it have additional requirements attached to it?

  1. Usually, when a query parameter drives a feature, it should be set on the user's session.
  2. Feature selection is often done in context of A/B testing and this library provides no reporting/measuring in this context. Internally, we're using Optimizely for that.

Thoughts?

@filipgolonka
Copy link
Contributor Author

@richardfullmer thank you for response.

  1. What if there are no users in system? With your solution it is impossible (I guess) to achieve the same effect.
  2. Measuring feature selection in A/B testing context should be done on users side IMHO, so it shouldn't have influence on library.
  3. My solution can handle parameters not only from query string, but also from another sources, like $_POST array, $_SESSION, $_COOKIES, headers and stuff like that. I.e. in Symfony bundle I can grab all data from Request object and pass to Rollout library. It might be useful for pages with hardcoded urls - for testing features user can set header to contain proper value and then it is independent on the other values.

@filipgolonka
Copy link
Contributor Author

@richardfullmer any response on it?

@richardfullmer richardfullmer merged commit b93d48c into opensoft:master Sep 21, 2016
@richardfullmer
Copy link
Contributor

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.

3 participants