Skip to content

[FSSDK-11494] fix UserAttributes type #1042

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

Merged
merged 2 commits into from
May 6, 2025
Merged

[FSSDK-11494] fix UserAttributes type #1042

merged 2 commits into from
May 6, 2025

Conversation

raju-opti
Copy link
Contributor

Summary

  • Current type does not work when there are control attributes. This PR fixes that

Test plan

Issues

  • FSSDK-11494

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses the issue FSSDK-11494 by updating types for UserAttributes to support control attributes and by refactoring references to control attribute constants.

  • Removed the FORCED_DECISION_NULL_RULE_KEY entry from CONTROL_ATTRIBUTES and introduced a new constant.
  • Updated UserAttributeValue union to include undefined and ExperimentBucketMap, and updated tests and type usages accordingly.
  • Modified buildVisitorAttributes to early exit on object types, which may affect processing of ExperimentBucketMap values.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/utils/enums/index.ts Removed FORCED_DECISION_NULL_RULE_KEY from control attributes.
lib/shared_types.ts Updated UserAttributeValue type and UserAttributes structure.
lib/optimizely_user_context/index.ts Replaced usage of CONTROL_ATTRIBUTES.FORCED_DECISION_NULL_RULE_KEY with new constant.
lib/optimizely_user_context/index.tests.js Updated tests to reference the new forced decision key constant.
lib/event_processor/event_builder/user_event.ts Modified buildVisitorAttributes to add an early return for object types.
lib/event_processor/event_builder/log_event.ts Updated BOT_FILTERING key reference using CONTROL_ATTRIBUTES.
lib/core/decision_service/index.spec.ts Updated test attribute types to use UserAttributes.

@coveralls
Copy link

coveralls commented May 6, 2025

Coverage Status

coverage: 80.725% (+0.008%) from 80.717%
when pulling 06bf2fe on raju/user_attr
into 32f857e on master.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fixes the UserAttributes type to support control attributes by updating type definitions and replacing references to the forced decision key constant.

  • Removed FORCED_DECISION_NULL_RULE_KEY from CONTROL_ATTRIBUTES and replaced its usage with the standalone constant.
  • Extended UserAttributeValue to include undefined and ExperimentBucketMap.
  • Updated the user context, tests, and attribute building logic to align with the new types.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lib/utils/enums/index.ts Removed FORCED_DECISION_NULL_RULE_KEY from CONTROL_ATTRIBUTES.
lib/shared_types.ts Updated UserAttributeValue type and made control attribute keys optional.
lib/optimizely_user_context/index.ts Replaced references from CONTROL_ATTRIBUTES to the new constant.
lib/optimizely_user_context/index.tests.js Updated tests to use the new forced decision key constant.
lib/event_processor/event_builder/user_event.ts Refactored buildVisitorAttributes for clearer attribute handling.
lib/event_processor/event_builder/log_event.ts Adjusted BOT_FILTERING attribute to use CONTROL_ATTRIBUTES constant.
lib/core/decision_service/index.spec.ts Updated test attribute types to UserAttributes for consistency.
Comments suppressed due to low confidence (2)

lib/shared_types.ts:75

  • Ensure that 'ExperimentBucketMap' is declared or properly imported; its inclusion in the union type may lead to unexpected type conflicts if not defined.
export type UserAttributeValue = string | number | boolean | null | undefined | ExperimentBucketMap;

lib/event_processor/event_builder/user_event.ts:266

  • [nitpick] Verify that excluding attribute values of type 'object' does not omit valid attribute data (e.g., arrays) that should be logged, in accordance with log endpoint requirements.
if (typeof attributeValue === 'object' || typeof attributeValue === 'undefined') {

@raju-opti raju-opti merged commit 299e1d7 into master May 6, 2025
17 checks passed
@raju-opti raju-opti deleted the raju/user_attr branch May 6, 2025 19:47
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.

3 participants