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. #606

Merged
merged 14 commits into from
Oct 15, 2024

Conversation

kinyoklion
Copy link
Member

@kinyoklion kinyoklion commented Oct 3, 2024

This PR contains the server-side implementation for allFlagsState evaluation for bootstrap, server-side LDEvaluationDetail, client-side LDEvaluationDetail, and client-side events for prerequisites.

This version only includes direct pre-requisites, and the client-side evaluation uses variation methods versus directly sending events.

BEGIN_COMMIT_OVERRIDE
feat: Add support for client-side prerequisite events.
feat: Add support for prerequisite details to evaluation detail.
feat: Add prerequisite information to server-side allFlagsState.
END_COMMIT_OVERRIDE

SDK-686
SDK-682

@kinyoklion kinyoklion requested a review from a team as a code owner October 3, 2024 16:54
@kinyoklion kinyoklion marked this pull request as draft October 3, 2024 16:59
@kinyoklion kinyoklion changed the title feat: Add support for client-side prerequisite events (B). feat: Add support for client-side prerequisite events. Oct 14, 2024
Copy link
Member Author

Choose a reason for hiding this comment

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

Just linting changes in this file.

value,
variationIndex: variationIndex ?? null,
reason: reason ?? null,
};
if (prerequisites) {
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 isn't in the object construction because I don't want every eval to have a specified, but undefined, prerequisite field.

prerequisites: undefined.

},
eventFactory,
);
this.evaluateCb(flag, context, resolve, eventFactory);
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated to primary changes, just removing leftover code duplication.

@kinyoklion kinyoklion marked this pull request as ready for review October 14, 2024 17:41
@kinyoklion kinyoklion requested a review from keelerm84 October 14, 2024 17:42
@kinyoklion kinyoklion marked this pull request as draft October 14, 2024 17:53
@kinyoklion kinyoklion marked this pull request as ready for review October 14, 2024 18:09
@@ -18,9 +18,10 @@ export function createSuccessEvaluationDetail(
variationIndex?: number,
reason?: LDEvaluationReason,
): LDEvaluationDetail {
return {
const res: LDEvaluationDetail = {
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 was an intermediate change as I was sorting detail. It could be removed, but it doesn't hurt anything.

@tanderson-ld tanderson-ld self-requested a review October 15, 2024 19:50
@kinyoklion kinyoklion merged commit 8c84e01 into main Oct 15, 2024
21 checks passed
@kinyoklion kinyoklion deleted the rlamb/client-prereq-proto-2 branch October 15, 2024 20:06
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