Skip to content
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

Move WordPress Analytics from GravatarApi to WPAndroid #19948

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import com.google.android.material.dialog.MaterialAlertDialogBuilder;
import com.gravatar.GravatarApi;
import com.gravatar.GravatarApi.ErrorType;
import com.yalantis.ucrop.UCrop;
import com.yalantis.ucrop.UCropActivity;

Expand Down Expand Up @@ -737,8 +738,8 @@ protected void startGravatarUpload(final String filePath) {
new GravatarApi.GravatarUploadListener() {
@Override
public void onSuccess() {
// FIXME: log analytics
endProgress();
AnalyticsTracker.track(Stat.ME_GRAVATAR_UPLOADED);
mPhotoUrl = GravatarUtils.fixGravatarUrl(mAccount.getAccount().getAvatarUrl(),
getResources().getDimensionPixelSize(R.dimen.avatar_sz_large));
loadAvatar(mPhotoUrl, filePath);
Expand All @@ -747,10 +748,12 @@ public void onSuccess() {
}

@Override
public void onError(@NonNull String exceptionClass, @NonNull String exceptionMessage) {
public void onError(@NonNull ErrorType errorType) {
endProgress();
showErrorDialogWithCloseButton(getString(R.string.signup_epilogue_error_avatar));
// FIXME: log analytics
Map<String, Object> properties = new HashMap<>();
properties.put("error_type", errorType);
AnalyticsTracker.track(AnalyticsTracker.Stat.ME_GRAVATAR_UPLOAD_EXCEPTION, properties);
AppLog.e(T.NUX, "Uploading image to Gravatar failed");
}
});
Expand Down Expand Up @@ -836,18 +839,15 @@ public void run() {
new GravatarApi.GravatarUploadListener() {
@Override
public void onSuccess() {
// FIXME: log analytics
AppLog.i(T.NUX, "Google avatar download and Gravatar upload succeeded.");
AnalyticsTracker.track(AnalyticsTracker.Stat.ME_GRAVATAR_UPLOAD_UNSUCCESSFUL);
AnalyticsTracker.track(Stat.ME_GRAVATAR_UPLOADED);
}

@Override
public void onError(String exceptionClass, String exceptionMessage) {
public void onError(@NonNull ErrorType errorType) {
AppLog.i(T.NUX, "Google avatar download and Gravatar upload failed.");
// FIXME: Don't track exceptions caused by poor internet connectivity
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously some exceptions were ignored. I wasn't sure what to do, this could actually hide some underlying problems like a too short default timeout, so I decided to drop the filter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks better this way. We don't want to hide any exceptions, at least in the app.

Map<String, Object> properties = new HashMap<>();
properties.put("network_exception_class", exceptionClass);
properties.put("network_exception_message", exceptionMessage);
properties.put("error_type", errorType);
AnalyticsTracker.track(AnalyticsTracker.Stat.ME_GRAVATAR_UPLOAD_EXCEPTION, properties);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import androidx.fragment.app.viewModels
import androidx.lifecycle.ViewModelProvider
import com.google.android.material.dialog.MaterialAlertDialogBuilder
import com.google.android.material.snackbar.Snackbar
import com.gravatar.GravatarApi
import com.gravatar.GravatarApi.GravatarUploadListener
import com.yalantis.ucrop.UCrop
import com.yalantis.ucrop.UCrop.Options
import com.yalantis.ucrop.UCropActivity
Expand All @@ -36,15 +38,14 @@ import org.wordpress.android.analytics.AnalyticsTracker.Stat.ME_GRAVATAR_GALLERY
import org.wordpress.android.analytics.AnalyticsTracker.Stat.ME_GRAVATAR_SHOT_NEW
import org.wordpress.android.analytics.AnalyticsTracker.Stat.ME_GRAVATAR_TAPPED
import org.wordpress.android.analytics.AnalyticsTracker.Stat.ME_GRAVATAR_UPLOADED
import org.wordpress.android.analytics.AnalyticsTracker.Stat.ME_GRAVATAR_UPLOAD_EXCEPTION
import org.wordpress.android.databinding.MeFragmentBinding
import org.wordpress.android.fluxc.Dispatcher
import org.wordpress.android.fluxc.store.AccountStore
import org.wordpress.android.fluxc.store.AccountStore.OnAccountChanged
import org.wordpress.android.fluxc.store.PostStore
import org.wordpress.android.fluxc.store.SiteStore
import org.wordpress.android.models.JetpackPoweredScreen
import com.gravatar.GravatarApi
import com.gravatar.GravatarApi.GravatarUploadListener
import org.wordpress.android.ui.ActivityLauncher
import org.wordpress.android.ui.RequestCodes
import org.wordpress.android.ui.about.UnifiedAboutActivity
Expand Down Expand Up @@ -672,12 +673,12 @@ class MeFragment : Fragment(R.layout.me_fragment), OnScrollToTopListener {
GravatarApi.uploadGravatar(file, accountStore.account.email, accountStore.accessToken!!,
object : GravatarUploadListener {
override fun onSuccess() {
// FIXME: log analytics
AnalyticsTracker.track(ME_GRAVATAR_UPLOADED)
EventBus.getDefault().post(GravatarUploadFinished(filePath, true))
}

override fun onError(exceptionClass: String, exceptionMessage: String) {
// FIXME: log analytics
override fun onError(errorType: GravatarApi.ErrorType) {
AnalyticsTracker.track(ME_GRAVATAR_UPLOAD_EXCEPTION, mapOf("error_type" to errorType.name))
EventBus.getDefault().post(GravatarUploadFinished(filePath, false))
}
})
Expand All @@ -689,7 +690,6 @@ class MeFragment : Fragment(R.layout.me_fragment), OnScrollToTopListener {
fun onEventMainThread(event: GravatarUploadFinished) {
binding?.showGravatarProgressBar(false)
if (event.success) {
AnalyticsTracker.track(ME_GRAVATAR_UPLOADED)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note here, I removed this one because ME_GRAVATAR_UPLOADED is already tracked on line 676

binding?.loadAvatar(event.filePath)
} else {
ToastUtils.showToast(
Expand Down
Loading