Skip to content

Use feature name instead of id #16

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 13, 2022
Merged

Use feature name instead of id #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c7d78c
Use feature name instead of id
matthewelwell Jul 13, 2022
563033f
formatting
matthewelwell Jul 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions lib/flagsmith_client_analytics_processor.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ defmodule Flagsmith.Client.Analytics.Processor do
#################################

@doc """
Given a `t:Flagsmith.Schemas.Features.FeatureState.t/0` or
Given a `t:Flagsmith.Schemas.Features.FeatureState.t/0` or
`t:Flagsmith.Schemas.Features.Feature.t/0` or `t:Flagsmith.Schemas.Flag.t/0` and an
`t:Flagsmith.Schemas.Environment.t/0` or `t:Flagsmith.Configuration.t/0` add or
increment the call count for feature id to be reported to the analytics endpoint.
increment the call count for feature name to be reported to the analytics endpoint.
If `:enable_analytics` in the configuration value of the environment isn't true
it's a no op and returns (:noop), otherwise if the feature/flag doesn't have an
id it returns an error.
Expand All @@ -55,7 +55,7 @@ defmodule Flagsmith.Client.Analytics.Processor do
to_track,
%Configuration{enable_analytics: true} = config
) do
case extract_feature_id(to_track) do
case extract_feature_name(to_track) do
id when is_binary(id) or is_integer(id) ->
do_track(id, config)

Expand All @@ -66,11 +66,11 @@ defmodule Flagsmith.Client.Analytics.Processor do

def track(_, _), do: :noop

defp extract_feature_id(feature) do
defp extract_feature_name(feature) do
case feature do
%Schemas.Features.FeatureState{feature: %{id: id}} -> id
%Schemas.Features.Feature{id: id} -> id
%Schemas.Flag{feature_id: id} -> id
%Schemas.Features.FeatureState{feature: %{name: name}} -> name
%Schemas.Features.Feature{name: name} -> name
%Schemas.Flag{feature_name: name} -> name
_ -> :invalid_feature
end
end
Expand Down Expand Up @@ -111,7 +111,7 @@ defmodule Flagsmith.Client.Analytics.Processor do
end

@doc """
Starts and links a gen_server represented by this module, using a
Starts and links a gen_server represented by this module, using a
`t:Flagsmith.Configuration.t/0` as the basis to derive its registration name and
inner details.
"""
Expand All @@ -133,7 +133,7 @@ defmodule Flagsmith.Client.Analytics.Processor do
end

#################################
########### Statem Implementation / Internal
########### Statem Implementation / Internal
#################################

@doc false
Expand Down Expand Up @@ -200,10 +200,10 @@ defmodule Flagsmith.Client.Analytics.Processor do
def handle_event({:timeout, :dump}, nil, _, _data),
do: {:keep_state_and_data, [{:next_event, :internal, :dump}]}

# a cast for tracking a feature_id simply sets or updates that feature id track
# a cast for tracking a feature_name simply sets or updates that feature name track
# in the tracking map
def handle_event(:cast, {:track, feature_id}, _, %{tracking: tracking} = data) do
new_tracking = Map.update(tracking, feature_id, 1, &(&1 + 1))
def handle_event(:cast, {:track, feature_name}, _, %{tracking: tracking} = data) do
new_tracking = Map.update(tracking, feature_name, 1, &(&1 + 1))
{:keep_state, %{data | tracking: new_tracking}, []}
end

Expand Down
35 changes: 20 additions & 15 deletions test/flagsmith_analytics_processor_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ defmodule Flagsmith.Client.Analytics.Processor.Test do
@api_url Flagsmith.Configuration.default_url()
@api_paths Flagsmith.Configuration.api_paths()

# setup Mox to verify any expectations
# setup Mox to verify any expectations
setup :verify_on_exit!

# we start the supervisor as a supervised process so it's shut down on every test
Expand Down Expand Up @@ -77,7 +77,7 @@ defmodule Flagsmith.Client.Analytics.Processor.Test do
:ok
end

test "dosn't start if enable_analytics is false", %{config: config} do
test "doesn't start if enable_analytics is false", %{config: config} do
config = %{config | enable_analytics: false}
{:ok, environment} = Flagsmith.Client.get_environment(config)

Expand Down Expand Up @@ -105,21 +105,21 @@ defmodule Flagsmith.Client.Analytics.Processor.Test do
{:ok, environment} = Flagsmith.Client.get_environment(config)

# get a flag from that environment
assert %Schemas.Flag{feature_id: id, enabled: true} =
Flagsmith.Client.get_flag(environment, "secret_button")
feature_name_1 = "secret_button"
assert %Schemas.Flag{enabled: true} = Flagsmith.Client.get_flag(environment, feature_name_1)

# assert that now there's a processor for the environment key we've been using
pid = Flagsmith.Client.Analytics.Processor.whereis("test_key")
assert is_pid(pid)

# assert it's tracking correctly:
# - the tracking map should have 1 key being the id of the flag we retrieved
# - the tracking map should have 1 key being the name of the flag we retrieved
# with the value being 1
assert {:on,
%Flagsmith.Client.Analytics.Processor{
configuration: ^config,
dump: 60_000,
tracking: %{^id => 1} = tracking_map_1
tracking: %{^feature_name_1 => 1} = tracking_map_1
}} = :sys.get_state(pid)

# assert there's only 1 key on the tracking map
Expand All @@ -129,32 +129,34 @@ defmodule Flagsmith.Client.Analytics.Processor.Test do
assert Flagsmith.Client.is_feature_enabled(environment, "secret_button")

# assert that the processor is still alive and that now the tracking for
# that feature id is at 2
# that feature name is at 2
assert {:on,
%Flagsmith.Client.Analytics.Processor{
configuration: ^config,
dump: 60_000,
tracking: %{^id => 2} = tracking_map_2
tracking: %{^feature_name_1 => 2} = tracking_map_2
}} = :sys.get_state(pid)

# assert there's still only 1 key on the tracking map
assert map_size(tracking_map_2) == 1

# assert other features track correctly too
# get another flag from that environment
assert %Schemas.Flag{feature_id: id_2, enabled: false} =
Flagsmith.Client.get_flag(environment, "header_size")
feature_name_2 = "header_size"

assert %Schemas.Flag{enabled: false} =
Flagsmith.Client.get_flag(environment, feature_name_2)

refute Flagsmith.Client.is_feature_enabled(environment, "header_size")
refute Flagsmith.Client.is_feature_enabled(environment, feature_name_2)

# assert that the processor is still alive and that now the tracking for
# the previous feature id is still at 2, and for the new one, id_2, is at
# the previous feature is still at 2, and for the new one, is at
# 2 too
assert {:on,
%Flagsmith.Client.Analytics.Processor{
configuration: ^config,
dump: 60_000,
tracking: %{^id => 2, ^id_2 => 2} = tracking_map_3
tracking: %{^feature_name_1 => 2, ^feature_name_2 => 2} = tracking_map_3
}} = :sys.get_state(pid)

# assert there's now 2 keys on the tracking map
Expand All @@ -172,13 +174,16 @@ defmodule Flagsmith.Client.Analytics.Processor.Test do
# change the dump timeout
assert :ok = :gen_statem.call(pid, {:update_dump_rate, 1})

# use an example feature name that we know exists
feature_name = "secret_button"

# we need to set an additional expectation since it will call the analytics
# endpoint

expect(Tesla.Adapter.Mock, :call, fn tesla_env, _options ->
assert_request(
tesla_env,
body: "{\"17985\":1}",
body: "{\"#{feature_name}\":1}",
query: [],
headers: [{@environment_header, "test_key"}],
url: Path.join([@api_url, @api_paths.analytics]) <> "/",
Expand All @@ -196,7 +201,7 @@ defmodule Flagsmith.Client.Analytics.Processor.Test do
# attempts to dump, meaning this inadvertently tests that after a dump the
# tracking map is effectively empty

assert Flagsmith.Client.is_feature_enabled(environment, "secret_button")
assert Flagsmith.Client.is_feature_enabled(environment, feature_name)

assert Flagsmith.Test.Helpers.wait_until(
fn ->
Expand Down