-
Notifications
You must be signed in to change notification settings - Fork 247
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
Analytics - Update Methods , Clean Code #21
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,59 +48,55 @@ class AmplifyAnalyticsBridge { | |
|
||
fun flushEvents(@NonNull result: MethodChannel.Result) { | ||
try { | ||
Amplify.Analytics.flushEvents(); | ||
result.success(true); | ||
Amplify.Analytics.flushEvents() | ||
result.success(true) | ||
} catch (e: Exception) { | ||
result.error("AmplifyException", "Error", e.message) | ||
} | ||
} | ||
|
||
fun registerGlobalProperties(@NonNull arguments: Any, @NonNull result: MethodChannel.Result) { | ||
try { | ||
val globalProperties = arguments as HashMap<String, Any>; | ||
val globalProperties = arguments as HashMap<String, Any> | ||
Amplify.Analytics.registerGlobalProperties( | ||
AmplifyAnalyticsBuilder.createAnalyticsProperties(globalProperties)); | ||
result.success(true); | ||
AmplifyAnalyticsBuilder.createAnalyticsProperties(globalProperties)) | ||
result.success(true) | ||
} catch (e: Exception) { | ||
result.error("AmplifyException", "Error", e.message) | ||
} | ||
} | ||
|
||
fun unregisterGlobalProperties(@NonNull arguments: Any, @NonNull result: MethodChannel.Result) { | ||
try { | ||
val propertyNames = (arguments as ArrayList<String>).toSet<String>(); | ||
val propertyNames = (arguments as ArrayList<String>).toSet<String>() | ||
|
||
for (name in propertyNames) { | ||
Amplify.Analytics.unregisterGlobalProperties(name); | ||
if(propertyNames.size == 0){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
Amplify.Analytics.unregisterGlobalProperties() | ||
} | ||
result.success(true); | ||
} catch (e: Exception) { | ||
result.error("AmplifyException", "Error", e.message) | ||
} | ||
} | ||
|
||
fun unregisterAllGlobalProperties(@NonNull result: MethodChannel.Result) { | ||
try { | ||
Amplify.Analytics.unregisterGlobalProperties() | ||
result.success(true); | ||
else{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
for (name in propertyNames) { | ||
fjnoyp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Amplify.Analytics.unregisterGlobalProperties(name) | ||
} | ||
} | ||
result.success(true) | ||
} catch (e: Exception) { | ||
result.error("AmplifyException", "Error", e.message) | ||
} | ||
} | ||
|
||
fun enable(@NonNull result: MethodChannel.Result) { | ||
try { | ||
Amplify.Analytics.enable(); | ||
result.success(true); | ||
Amplify.Analytics.enable() | ||
result.success(true) | ||
} catch (e: Exception) { | ||
result.error("AmplifyException", "Error", e.message) | ||
} | ||
} | ||
|
||
fun disable(@NonNull result: MethodChannel.Result) { | ||
try { | ||
Amplify.Analytics.disable(); | ||
result.success(true); | ||
Amplify.Analytics.disable() | ||
result.success(true) | ||
} catch (e: Exception) { | ||
result.error("AmplifyException", "Error", e.message) | ||
} | ||
|
@@ -110,11 +106,11 @@ class AmplifyAnalyticsBridge { | |
try { | ||
val argumentsMap = arguments as HashMap<*, *> | ||
val userId = argumentsMap["userId"] as String | ||
val userProfileMap = argumentsMap["userProfileMap"] as HashMap<String, Object>; | ||
val userProfileMap = argumentsMap["userProfileMap"] as HashMap<String, Object> | ||
|
||
Amplify.Analytics.identifyUser(userId, | ||
AmplifyAnalyticsBuilder.createUserProfile(userProfileMap)); | ||
result.success(true); | ||
AmplifyAnalyticsBuilder.createUserProfile(userProfileMap)) | ||
result.success(true) | ||
} catch (e: Exception) { | ||
result.error("AmplifyException", "Error", e.message) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,10 +44,14 @@ class _MyAppState extends State<MyApp> { | |
amplifyInstance.addPlugin( | ||
authPlugins: [authPlugin], analyticsPlugins: [analyticsPlugin]); | ||
|
||
await amplifyInstance.configure(amplifyconfig); | ||
setState(() { | ||
_amplifyConfigured = true; | ||
}); | ||
try { | ||
await amplifyInstance.configure(amplifyconfig); | ||
setState(() { | ||
_amplifyConfigured = true; | ||
}); | ||
} catch (e) { | ||
print(e); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the configuration fails, that probably means you won't be able to use Analytics, right? So, log-n-gobble may not be the best strategy here. Should you not have the exception bubble back up to the user, so they know not to proceed? (Or, can make the decision themselves, as to whether or not they want to, at least. Right now, you make that choice for them.) |
||
} | ||
} | ||
|
||
void _recordEvent() async { | ||
|
@@ -81,11 +85,10 @@ class _MyAppState extends State<MyApp> { | |
print("unregister global properties: " + _globalProp); | ||
|
||
Amplify.Analytics.unregisterGlobalProperties(propertyNames: [_globalProp]); | ||
; | ||
} | ||
|
||
void _unregisterAllGlobalProperties() async { | ||
Amplify.Analytics.unregisterAllGlobalProperties(); | ||
Amplify.Analytics.unregisterGlobalProperties(); | ||
} | ||
|
||
void _enable() async { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,11 +43,14 @@ public class AmplifyAnalyticsBridge { | |
|
||
public static func unregisterGlobalProperties(arguments: Any?, result: @escaping FlutterResult){ | ||
let propertyNames = Set<String>(arguments as! Array<String>) | ||
Amplify.Analytics.unregisterGlobalProperties(propertyNames) | ||
result(true); | ||
} | ||
public static func unregisterAllGlobalProperties(result: @escaping FlutterResult){ | ||
Amplify.Analytics.unregisterGlobalProperties() | ||
|
||
if(propertyNames.count == 0){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Meh, yea -- these are driving me nuts though. I don't know any language where whitespace isn't used around control flow keywords. The Dart style guide does not contain any examples of this. https://dart.dev/guides/language/effective-dart/style |
||
Amplify.Analytics.unregisterGlobalProperties() | ||
} | ||
else{ | ||
Amplify.Analytics.unregisterGlobalProperties(propertyNames) | ||
} | ||
|
||
result(true); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have some lint tooling available that can enforce this as part of the build/PR process?