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

feat: Add support for client-side prerequisite events. #137

Conversation

kinyoklion
Copy link
Member

@kinyoklion kinyoklion commented Oct 22, 2024

BEGIN_COMMIT_OVERRIDE
feat: Add support for client-side prerequisite events.
feat: Add support for client-side visibility for all_flags_state.
END_COMMIT_OVERRIDE

SDK-803

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating this test data to have consistent clientSideAvailability with the clientSide field.

with_reasons => boolean()
%% client_side_only => boolean(), % TODO: Support.
with_reasons => boolean(),
client_side_only => boolean()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed reasonable to implement this at the same time, and that allows the contract tests to be passed as well.

@kinyoklion kinyoklion force-pushed the rlamb/SDK-694/add-client-side-prerequisite-events-support-to-erlang-server-sdk branch from 8d938aa to f1d2e58 Compare October 22, 2024 16:35
@kinyoklion kinyoklion force-pushed the rlamb/SDK-694/add-client-side-prerequisite-events-support-to-erlang-server-sdk branch from f1d2e58 to 2e75104 Compare October 22, 2024 16:37
@@ -810,6 +812,101 @@ rule_match_rollout_not_in_experiment(_) ->
ExpectedEvents = ActualEvents.

all_flags_state(_) ->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was supposed to be without reasons and the other test with reasons, but it originally had a mistake. So both tests were with reasons.

all_flags_state_offline(_) ->
#{
<<"$flagsState">> := #{},
<<"$valid">> := false
} = ldclient_eval:all_flags_state(#{key => <<"userKeyA">>, kind => <<"user">>}, #{with_reasons => true}, offline).

all_flags_state_client_side_only(_) ->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New test for client-side visibility.

FeatureStore = ldclient_config:get_value(Tag, feature_store),
AllFlags = [Flag || Flag = {_, FlagValue} <- FeatureStore:all(Tag, features), is_not_deleted(FlagValue)],
AllFlags = [Flag || Flag = {_, FlagValue} <- FeatureStore:all(Tag, features), is_not_deleted(FlagValue), is_visible(FlagValue, Options)],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the list comprehension to filter flags based on visibility.

@@ -113,6 +113,26 @@ all_flags_state(Context, Options, Tag) ->
is_not_deleted(#{deleted := true}) -> false;
is_not_deleted(_) -> true.

-spec is_prereq_of(FlagKey :: ldclient_flag:key(), Event :: ldclient_event:event()) -> boolean().
is_prereq_of(FlagKey, #{data := #{prereq_of := PrereqOf}} = _Event) ->
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If prereq_of is set to the flag we are evaluating, then that flag was evaluated as a direct prerequisite.

@kinyoklion kinyoklion marked this pull request as ready for review October 22, 2024 16:44
@kinyoklion kinyoklion requested a review from a team as a code owner October 22, 2024 16:44
@kinyoklion kinyoklion merged commit 167308a into main Oct 24, 2024
4 checks passed
@kinyoklion kinyoklion deleted the rlamb/SDK-694/add-client-side-prerequisite-events-support-to-erlang-server-sdk branch October 24, 2024 19:54
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.

2 participants