feat: update to new audience logic #330
Conversation
BrandonStalnaker
changed the title
BrandonStalnaker
changed the title
BrandonStalnaker
force-pushed
the
feat/6150-Update-To-New-Audience-Logic
branch
2 times, most recently
from
e05b693 to
1fc7a59
Compare
| /** | ||
| The mParticle id associated with this user (MPID) | ||
| */ |
There was a problem hiding this comment.
this comment looks outdated. please fix
| @@ -550,6 +551,10 @@ - (void)testUploadWithOptOutMessage { | |||
| [self waitForExpectationsWithTimeout:DEFAULT_TIMEOUT handler:nil]; | |||
| } | |||
|
|
|||
| - (void)testAudiences { | |||
There was a problem hiding this comment.
There's a lot of code changes but just this 1 test. At a minimum, i feel lik there shoudl be tests for:
- ensuring the url endpoint for audience is created properly
- ensuring the feature flag works if
Trueor error messages ifFalse
| @@ -89,6 +90,13 @@ NS_ASSUME_NONNULL_BEGIN | |||
| */ | |||
| - (void)removeUserAttribute:(NSString *)key; | |||
|
|
|||
| #pragma mark - User Audiences | |||
| /** | |||
| Retrieves user audiences from mParticle's servers and returns the result as two arrays of MPAudience objects | |||
There was a problem hiding this comment.
is reeturns result as two arrays accurate? shouldn't it just be a single array of user audiences? this screenshot of a live example on we which hits the same endpoint returns a single array which contains 2 audiences
| if (supportedKits) { | ||
| kits = [supportedKits componentsJoinedByString:@","]; | ||
| } | ||
|
|
||
| NSString *environment = [NSString stringWithFormat:@"%d", (int)[MPStateMachine environment]]; | ||
| [urlRequest setValue:environment forHTTPHeaderField:@"x-mp-env"]; | ||
|
|
||
| MPIUserDefaults *userDefaults = [MPIUserDefaults standardUserDefaults]; | ||
| NSString *eTag = userDefaults[kMPHTTPETagHeaderKey]; | ||
| NSDictionary *config = [userDefaults getConfiguration]; | ||
| if (eTag && config) { | ||
| [urlRequest setValue:eTag forHTTPHeaderField:@"If-None-Match"]; | ||
| } | ||
|
|
||
| NSString *query = [_url.defaultURL query]; | ||
| signatureMessage = [NSString stringWithFormat:@"%@\n%@\n%@?%@", _httpMethod, date, relativePath, query]; |
There was a problem hiding this comment.
should this be in this PR? i'm unclear as to what is going on here. zoom might be easier.
There was a problem hiding this comment.
So when I was testing this existing code with the endpoint it wasn't working because of a url with capitalization issues and because the the signature was malformed. There was also several headers being sent in the audience call that weren't necessary. So since this was the only ticket I had for this functionality, I fixed it all in this ticket though some of this is also from the changes in that feature branch.
This reverts commit 1c04262.
BrandonStalnaker
force-pushed
the
feat/6150-Update-To-New-Audience-Logic
branch
from
88b1c03 to
f2d0f30
Compare
Summary
Testing Plan
Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)