-
Notifications
You must be signed in to change notification settings - Fork 410
Added Support for ISOCurrency in complete registration #67
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
Conversation
|
Please open an issue before creating a PR. It's unclear to me if this is a bug in the plugin or a hack to fix a problem with the Facebook SDK. Could I please ask you to create an issue where you give full context of the Facebook SDK requirement for this parameter and/or context to official sources claiming that this is the fix that need to be done. If it's a bug on the Facebook SDK, it's important that the solution in the plugin is just handling the bug with grace and not implementing an pattern that will become a bug on the plugin once the SDK bug is fixed 🙏 |
|
Will this be merged? really need this |
Hi @juliavi please see the request for feedback in my comment and perhaps you can help the OP with that information if you also know the background and circumstances of this "fix". A PR should be linked with an issue describing the problem. The PR should declare what issues of the plugin that are being fixed. The issue report should state
It is a lot of work for us as maintainers to read the PR, try to assume the expectations and then search for information that might back up the assumed expectations. The source-check that I did seems to be unrelated. I was not able to confirm that there is a missing currency parameter on the event
I am happy to be guided to the expected behavior and how this event is being used on your side. Because, although I am maintaining the plugin, I am not an expert on Facebook Analytics. I have merely developed the plugin based on the documentation that I could find 🙏 |
|
This is a very common problem people are facing and the fix I have provided has fixed the problem with this particular error. If you need to read further I found this article for your perusal. |
|
Interesting issue. I'll try to take a look at it. As a workaround in the meanwhile, you could use the generic flutter_facebook_app_events/lib/facebook_app_events.dart Lines 67 to 71 in 9adf151
And pass the parameters in the map fbAppEvents.logEvent(
name: FacebookAppEvents.eventNameCompletedRegistration,
parameters: {
FacebookAppEvents.paramNameCurrency: 'USD',
},
),Apologies for the delay, I just want to wrap my head around that it's an issue dating back to 2019 and still haunting the SDK 🤔 |
|
Ok, will wait for a merge 🤞🏽 |
|
Could you give me some context of how you have configured the facebook project, the usecase etc. Because I keep finding this very strange. In the meanwhile, please see the implementation of flutter_facebook_app_events/lib/facebook_app_events.dart Lines 167 to 174 in 9adf151
So if it is causing a lot of pain for now, you could technically create an extension method of your own as a workaround/fix. extension FBAppEventsExtension on FacebookAppEvents {
Future<void> logCompletedRegistrationWithCurrency({
String registrationMethod,
String currency,
}) {
return logEvent(
name: FacebookAppEvents.eventNameCompletedRegistration,
parameters: {
FacebookAppEvents.paramNameRegistrationMethod: registrationMethod,
FacebookAppEvents.paramNameCurrency: currency,
},
);
}
}Because I'm just failing to find any solid and clear information about it that is related to Facebook App Events. I find a number or results related to Facebook Pixel. And I have checked my own Facebook projects that are using this flutter package, and I don't have any error reports regarding the missing currency. So I'm a bit lost regarding how to reproduce it. Would you mind giving me some more context regarding your app, how the Facebook project is configured and the circumstances around getting the error? Could you shine some light how it relates to Facebook Pixel vs Facebook App Events. Because I'm far from expert on how these analytics tools and platforms relate to each other 🙏 |
|
No more information or clarity if this is really a bug on the plugin, a bug in the Facebook SDK or something else. Will close for now as the plugin implements according to documentation: https://developers.facebook.com/docs/analytics/send_data/events/ |


Fixed this issue above.