comocode / laravel-ab

Laravel 5 A/B experiment testing using with Blade
https://packagist.org/packages/comocode/laravel-ab
MIT License
31 stars 14 forks source link

Instance Metadata #4

Closed JonDickson20 closed 8 years ago

JonDickson20 commented 8 years ago

Hey! Great package.

I noticed the metadata field on the instance table which seems to be unused.

I would love a way to identify my users if info is available in the GET variables.

What about something like this on line 63-69 of Ab.php?:

        if (empty(self::$session)){
            self::$session = Instance::firstOrCreate([
                'instance'=>Session::get("laravel_ab_user"),
                'identifier'=>$this->request->getClientIp(),
                'metadata'=>json_encode($this->request->all())
            ]);
        }
82rules commented 8 years ago

I think that's a great idea, I would like to see that implemented optionally thou, as request->all() can get large for some apps. Perhaps with a callback defined in a config. That way different apps can capture whatever metadata they need in with some control over what and how it gets stored.

Sorry I took a while to get to the issue I didnt get notification for some reason.

82rules commented 8 years ago

so I'm not terribly happy with my eventual solution but I wanted to get something included in the meantime. https://github.com/comocode/laravel-ab/blob/master/src/App/Ab.php#L66 I created a convention to allow users to execute logic based on the session initiation, personally I think it's better placed in an event rather than instance, however defining this function in your app will pass additional array into the instance model which will serialize and return your array.