this runs through the background and removes any experiments that are… #302
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… not in a fresh datafile
mikeproeng37
suggested changes
Aug 31, 2018
There was a problem hiding this comment.
Same comment as @msohailhussain , please add unit tests for these methods.
mikeproeng37
suggested changes
Sep 5, 2018
|
|
||
| - (void)removeInvalidExperimentsForAllUsers:(NSArray<NSString *> *)validExperimentIds { | ||
|
|
||
| NSMutableDictionary*userProflieService = [[self.dataStore getUserDataForType:OPTLYDataStoreDataTypeUserProfileService] mutableCopy]; |
There was a problem hiding this comment.
nit: typo in userProflieService. Also need a space before it
| - (void)cleanUserProfileService:(NSArray<OPTLYExperiment> *)experiments { | ||
| if (experiments == nil) return; | ||
|
|
||
| if (_userProfileService != nil && [(NSObject *)_userProfileService respondsToSelector:@selector(removeInvalidExperimentsForAllUsers:)]) { |
There was a problem hiding this comment.
This seems dangerous, as in, should we wrap it in a try/catch?
There was a problem hiding this comment.
respondsToSelector is a test. I can't think of a possible reason an exception could be thrown here.
| @@ -52,4 +52,8 @@ __attribute((deprecated("Use OPTLYManager initWithBuilder method instead."))); | |||
| **/ | |||
| - (void)removeAllUserExperimentRecords; | |||
|
|
|||
| /** | |||
| * Cleanup and remove experiments that are not in the valid experiment list passed in. | |||
| @@ -67,6 +69,29 @@ @interface OPTLYManagerBase() | |||
|
|
|||
| @implementation OPTLYManagerBase | |||
|
|
|||
| - (void)cleanUserProfileService:(NSArray<OPTLYExperiment> *)experiments { | |||
There was a problem hiding this comment.
Can you also add some unit tests for this method, particularly around failure scenarios
There was a problem hiding this comment.
@mikeng13 done
…rofileService. fix typos
…izely/objective-c-sdk into cleanup/userprofileservice
mikeproeng37
approved these changes
Sep 6, 2018
|
|
||
| [manager cleanUserProfileService:client.optimizely.config.experiments]; | ||
|
|
||
|
|
There was a problem hiding this comment.
nit: exceess new lines?
| @@ -53,7 +53,11 @@ __attribute((deprecated("Use OPTLYManager initWithBuilder method instead."))); | |||
| - (void)removeAllUserExperimentRecords; | |||
|
|
|||
| /** | |||
| * Cleanup and remove experiments that are not in the valid experiment list passed in. | |||
| * Clean up and remove experiments that are not in the valid experiment list passed in. | |||
| * This is called when initialzied from a remote datafile to ensure that the UserProfileService | |||
There was a problem hiding this comment.
nit: typo --> initialized
AbdurRafay
pushed a commit
that referenced
this pull request
Sep 10, 2018
* master: this runs through the background and removes any experiments that are… (#302) Updating libraries based on SourceClear recommendation (#304) add credits for third party libraries (#299) # Conflicts: # OptimizelySDKUniversal/generated-frameworks/Release-iOS-universal-SDK/OptimizelySDKiOS.framework.zip # OptimizelySDKUniversal/generated-frameworks/Release-iOS-universal-SDK/OptimizelySDKiOS.framework/OptimizelySDKiOS
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
… not in a fresh datafile