[FSSDK-9533] Log error & reject on track event with null, empty or whitespace event key #362
Conversation
|
This will need to be part of a cherry-pick and release for a v3.11.4 along with #361 |
| var config = ProjectConfigManager?.GetConfig(); | ||
|
|
||
| if (config == null) |
There was a problem hiding this comment.
can we remove this config ready check?
There was a problem hiding this comment.
If we're going to call .GetEvent(eventKey) later (L365 below on this older version) to check if the provided eventKey is in the datafile, I think we'd probably still need this check, right?
| var eevent = config.GetEvent(eventKey); | ||
|
|
||
| if (eevent.Key == null) |
There was a problem hiding this comment.
I think these 2 lines will filter out null or empty eventKey cases.
There was a problem hiding this comment.
I think I see what you're talking about: The eventKey must be in the datafile before it can be tracked anyway, regardless of whether it's not null, empty, or whitespace.
| @@ -338,54 +339,60 @@ private bool ValidateInputs(string datafile, bool skipJsonValidation) | |||
| /// <summary> | |||
| /// Sends conversion event to Optimizely. | |||
| /// </summary> | |||
| /// <param name="eventKey">Event key representing the event which needs to be recorded</param> | |||
| /// <param name="eventKey">Event key representing the event (must not be null, empty, or whitespace)</param> | |||
There was a problem hiding this comment.
I guess this should read: "Event key representing the event in the datafile which must not be null, empty, or whitespace"
| var config = ProjectConfigManager?.GetConfig(); | ||
|
|
||
| if (config == null) | ||
| if (eventKey == null || Regex.IsMatch(eventKey, @"^\s*$")) |
There was a problem hiding this comment.
I added an earlier eventKey check since it's cheaper before digging into the ProjectConfig to see if the eventKey is present in the datafile since it's required per the docs.
Summary
Test plan
Issues