Skip to content

Commit fb2408f

Browse files
Address PR review feedback: remove unused user_id_source, consolidate exclusion logic, include targeting_key
- Remove unused user_id_source variable and assignments (addresses code clutter concern) - Consolidate user ID exclusion logic into single efficient statement - Include targeting_key in exclusion logic to prevent it from being added to custom data - Update comment to mention all three user ID types for clarity - Add comprehensive test coverage for targeting_key exclusion from attributes
1 parent 12c04b2 commit fb2408f

File tree

2 files changed

+16
-8
lines changed

2 files changed

+16
-8
lines changed

devcycle_python_sdk/models/user.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,18 +107,14 @@ def create_user_from_context(
107107
:return: A DevCycleUser instance
108108
"""
109109
user_id = None
110-
user_id_source = None
111110

112111
if context:
113112
if context.targeting_key:
114113
user_id = context.targeting_key
115-
user_id_source = "targeting_key"
116114
elif context.attributes and "user_id" in context.attributes.keys():
117115
user_id = context.attributes["user_id"]
118-
user_id_source = "user_id"
119116
elif context.attributes and "userId" in context.attributes.keys():
120117
user_id = context.attributes["userId"]
121-
user_id_source = "userId"
122118

123119
if not user_id or not isinstance(user_id, str):
124120
raise TargetingKeyMissingError(
@@ -130,10 +126,8 @@ def create_user_from_context(
130126
private_custom_data: Dict[str, Any] = {}
131127
if context and context.attributes:
132128
for key, value in context.attributes.items():
133-
# Skip user_id and userId - these are reserved for user ID mapping
134-
if key == "user_id":
135-
continue
136-
if key == "userId":
129+
# Skip user_id, userId, and targeting_key - these are reserved for user ID mapping
130+
if key in ("user_id", "userId", "targeting_key"):
137131
continue
138132

139133
if value is not None:

test/models/test_user.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,20 @@ def test_create_user_from_context_user_id_fields_always_excluded(self):
125125
self.assertNotIn("userId", user.customData)
126126
self.assertEqual(user.customData["other_field"], "value")
127127

128+
def test_create_user_from_context_targeting_key_excluded_from_attributes(self):
129+
targeting_key_id = "targeting-12345"
130+
131+
# When targeting_key appears in attributes, it should be excluded from custom data
132+
context = EvaluationContext(
133+
targeting_key=targeting_key_id,
134+
attributes={"targeting_key": "should-be-excluded", "other_field": "value"}
135+
)
136+
user = DevCycleUser.create_user_from_context(context)
137+
self.assertEqual(user.user_id, targeting_key_id)
138+
self.assertIsNotNone(user.customData)
139+
self.assertNotIn("targeting_key", user.customData)
140+
self.assertEqual(user.customData["other_field"], "value")
141+
128142
def test_create_user_from_context_userId_excluded_when_used(self):
129143
userId = "userId-12345"
130144

0 commit comments

Comments
 (0)