Only using request-level cache if the helper has a request

[ponny]

Firstly, thanks for building such a great gem. The interface is quite intuitive and the source code is very clear. It's really been useful for our team in adding A/B tests (and sometimes feature toggling). We've introduced many improvements based on the results in the last few months. 🙏

This is my first open source pull request so please let me know if I'm 'doing it wrong' 😄

We had an issue where we were using FieldTest::Helpers in a service with class methods.

class SomeService
  extend FieldTest::Helpers

  class << self
    def variant!(user)
      field_test("my_experiment_name", participant: user)
    end
end

Unfortunately, this meant that our app cached the result in @field_test_cache per experiment permanently (until the app restarts anyway). Looking at the comments inside FieldTest::Helpers#field_test, it seems to be setting the cache for the request outside of the check for the request. This fix means that it will only be using the cache when a request is present.

I also added a tweak to the assignment of params_variant because it could be unassigned. Happy to roll it back if you don't like doing it that way.

[ankane]

Hey @ponny, thanks for the PR (and congrats 🎉)! For non-requests, there's a different method that's preferred that skips caching (as the Helper module itself isn't meant to be public): https://github.com/ankane/field_test#jobs

For requests, it looks like the cache key needs to take participants and possibly a few other options that affect the variant like exclude into account. Feel free to update the PR if that's something you'd like to tackle.

Re params_variant: it's generally a good practice to keep the changeset to a minimum, as it makes it easier to review/merge and each author has their own preferred style.

[ponny]

Fair enough. Thanks for the advice 👍