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.

Sign up for GitHub

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

Address AT issues after domain registration #10101

Merged
merged 16 commits into from
Jun 28, 2019
Merged

Address AT issues after domain registration #10101

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0a44f51
Adding a 10 seconds delay between completion of AT and fetching of pl…
khaykov Jun 24, 2019
2ee6582
Added retry logic for fetching plugin directory.
khaykov Jun 26, 2019
e40005e
Fixed request code, updated logging, removed redundant code.
khaykov Jun 26, 2019
0f531df
Increased plugin retry delay to 10 seconds.
khaykov Jun 26, 2019
effc139
Added descriptive SSL error message, delay when registering domain name.
khaykov Jun 26, 2019
8fbb918
Naming.
khaykov Jun 26, 2019
4093d05
Changed action after domain registration and dispatching plugin confi…
khaykov Jun 26, 2019
79797c1
Removed dedicated message for no ssl error in favor of more general one.
khaykov Jun 26, 2019
d845303
Manually setting domain as a primary after it registered.
khaykov Jun 26, 2019
58baf9e
Updated tests.
khaykov Jun 26, 2019
0491d03
Fixed checkstyle issues, naming.
khaykov Jun 26, 2019
5d469e1
Removed multispaces.
khaykov Jun 26, 2019
afa7ffe
Null guard for DomainRegistrationPurpose passed with intent, private …
khaykov Jun 27, 2019
28b3905
Updated error message handling.
khaykov Jun 27, 2019
066fd32
Fresh flux-c commit hash.
khaykov Jun 28, 2019
051dba7
Merge branch 'release/12.7' of github.com:wordpress-mobile/WordPress-…
khaykov Jun 28, 2019
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
9 changes: 9 additions & 0 deletions WordPress/src/main/java/org/wordpress/android/ui/ActivityLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.wordpress.android.ui.activitylog.list.ActivityLogListActivity;
import org.wordpress.android.ui.comments.CommentsActivity;
import org.wordpress.android.ui.domains.DomainRegistrationActivity;
import org.wordpress.android.ui.domains.DomainRegistrationActivity.DomainRegistrationPurpose;
import org.wordpress.android.ui.giphy.GiphyPickerActivity;
import org.wordpress.android.ui.history.HistoryDetailActivity;
import org.wordpress.android.ui.history.HistoryDetailContainerFragment;
Expand Down Expand Up @@ -414,6 +415,14 @@ public static void viewDomainRegistrationActivity(Activity activity, SiteModel s
activity.startActivity(intent);
}

public static void viewDomainRegistrationActivityForResult(Activity activity, SiteModel site,
DomainRegistrationPurpose purpose) {
Intent intent = new Intent(activity, DomainRegistrationActivity.class);
intent.putExtra(WordPress.SITE, site);
intent.putExtra(DomainRegistrationActivity.DOMAIN_REGISTRATION_PURPOSE_KEY, purpose);
activity.startActivityForResult(intent, RequestCodes.DOMAIN_REGISTRATION);
}

public static void viewActivityLogList(Activity activity, SiteModel site) {
if (site == null) {
ToastUtils.showToast(activity, R.string.blog_not_found, ToastUtils.Duration.SHORT);
Expand Down
3 changes: 3 additions & 0 deletions WordPress/src/main/java/org/wordpress/android/ui/RequestCodes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,7 @@ public class RequestCodes {
public static final int QUICK_START_REMINDER_NOTIFICATION = 4001;

public static final int GIPHY_PICKER = 3200;

// Domain Registration
public static final int DOMAIN_REGISTRATION = 5000;
}
41 changes: 31 additions & 10 deletions WordPress/src/main/java/org/wordpress/android/ui/domains/DomainRegistrationActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,33 @@
package org.wordpress.android.ui.domains

import android.app.Activity
import android.os.Bundle
import android.view.MenuItem
import androidx.appcompat.app.AppCompatActivity
import kotlinx.android.synthetic.main.toolbar.*
import org.wordpress.android.R
import org.wordpress.android.ui.domains.DomainRegistrationActivity.DomainRegistrationPurpose.CTA_DOMAIN_CREDIT_REDEMPTION

class DomainRegistrationActivity : AppCompatActivity(), DomainRegistrationStepsListener {
enum class DomainRegistrationPurpose {
AUTOMATED_TRANSFER,
CTA_DOMAIN_CREDIT_REDEMPTION
}

companion object {
const val DOMAIN_REGISTRATION_PURPOSE_KEY = "DOMAIN_REGISTRATION_PURPOSE_KEY"
}

var domainRegistrationPurpose: DomainRegistrationPurpose? = null
khaykov marked this conversation as resolved.
Show resolved Hide resolved

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

setContentView(R.layout.activity_domain_suggestions_activity)

domainRegistrationPurpose = intent.getSerializableExtra(DOMAIN_REGISTRATION_PURPOSE_KEY)
as DomainRegistrationPurpose
Copy link
Contributor

@planarvoid planarvoid Jun 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this is correct. This will work if the DOMAIN_REGISTRATION_PURPOSE_KEY is in the intent, if it isn't, it will try to cast null to DomainRegistrationPurpose and it will fail because DomainRegistrationPurpose is not nullable. I think we have to cast it to DomainRegistrationPurpose? or use as? DomainRegistrationPurpose. This assumption is based on https://kotlinlang.org/docs/reference/typecasts.html#unsafe-cast-operator

Do you really need to keep the org.wordpress.android.ui.ActivityLauncher#viewDomainRegistrationActivity? It is unused so the DomainRegistrationPurpose should always be there. You can change var domainRegistrationPurpose: DomainRegistrationPurpose? = null to lateinit var domainRegistrationPurpose: DomainRegistrationPurpose so it doesn't have to be nullable.

Copy link
Member Author

@khaykov khaykov Jun 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch! I added a null guard using as? and left it nullable for now, since it will be called using viewDomainRegistrationActivity from develop branch, and I want to reduce conflicts when merging this back into it.

I'll change it to lateinit in next domain related PR 👍


setSupportActionBar(toolbar)
supportActionBar?.let {
it.setHomeButtonEnabled(true)
Expand Down Expand Up @@ -50,15 +66,20 @@ class DomainRegistrationActivity : AppCompatActivity(), DomainRegistrationStepsL
}

override fun onDomainRegistered(domainName: String) {
supportFragmentManager.beginTransaction()
.setCustomAnimations(
R.anim.activity_slide_in_from_right, R.anim.activity_slide_out_to_left,
R.anim.activity_slide_in_from_left, R.anim.activity_slide_out_to_right
)
.replace(
R.id.fragment_container,
DomainRegistrationResultFragment.newInstance(domainName)
)
.commit()
if (domainRegistrationPurpose == null || domainRegistrationPurpose == CTA_DOMAIN_CREDIT_REDEMPTION) {
supportFragmentManager.beginTransaction()
.setCustomAnimations(
R.anim.activity_slide_in_from_right, R.anim.activity_slide_out_to_left,
R.anim.activity_slide_in_from_left, R.anim.activity_slide_out_to_right
)
.replace(
R.id.fragment_container,
DomainRegistrationResultFragment.newInstance(domainName)
)
.commit()
} else {
setResult(Activity.RESULT_OK)
finish()
}
}
}
82 changes: 64 additions & 18 deletions WordPress/src/main/java/org/wordpress/android/ui/plugins/PluginDetailActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package org.wordpress.android.ui.plugins;

import android.animation.ObjectAnimator;
import android.app.Activity;
import android.app.Dialog;
import android.app.ProgressDialog;
import android.content.Context;
import android.content.DialogInterface;
import android.content.Intent;
import android.os.Bundle;
import android.os.Handler;
import android.text.Html;
Expand Down Expand Up @@ -71,6 +73,8 @@
import org.wordpress.android.fluxc.store.SiteStore.OnPlansFetched;
import org.wordpress.android.fluxc.store.SiteStore.OnSiteChanged;
import org.wordpress.android.ui.ActivityLauncher;
import org.wordpress.android.ui.RequestCodes;
import org.wordpress.android.ui.domains.DomainRegistrationActivity.DomainRegistrationPurpose;
import org.wordpress.android.util.AniUtils;
import org.wordpress.android.util.AppLog;
import org.wordpress.android.util.AppLog.T;
Expand Down Expand Up @@ -119,6 +123,11 @@ public class PluginDetailActivity extends AppCompatActivity implements OnDomainR
= "KEY_IS_SHOWING_AUTOMATED_TRANSFER_PROGRESS";
private static final String KEY_IS_SHOWING_DOMAIN_CREDIT_CHECK_PROGRESS
= "KEY_IS_SHOWING_DOMAIN_CREDIT_CHECK_PROGRESS";
private static final String KEY_PLUGIN_RECHECKED_TIMES = "KEY_PLUGIN_RECHECKED_TIMES";

private static final int MAX_PLUGIN_CHECK_TRIES = 10;
private static final int DEFAULT_RETRY_DELAY_MS = 3000;
private static final int PLUGIN_RETRY_DELAY_MS = 10000;

private SiteModel mSite;
private String mSlug;
Expand Down Expand Up @@ -162,6 +171,8 @@ public class PluginDetailActivity extends AppCompatActivity implements OnDomainR
protected boolean mIsShowingInstallFirstPluginConfirmationDialog;
protected boolean mIsShowingAutomatedTransferProgress;

private int mPluginReCheckTimer = 0;

// These flags reflects the UI state
protected boolean mIsActive;
protected boolean mIsAutoUpdateEnabled;
Expand Down Expand Up @@ -226,6 +237,7 @@ public void onCreate(Bundle savedInstanceState) {
.getBoolean(KEY_IS_SHOWING_AUTOMATED_TRANSFER_PROGRESS);
isShowingDomainCreditCheckProgress = savedInstanceState
.getBoolean(KEY_IS_SHOWING_DOMAIN_CREDIT_CHECK_PROGRESS);
mPluginReCheckTimer = savedInstanceState.getInt(KEY_PLUGIN_RECHECKED_TIMES, 0);
}

setContentView(R.layout.plugin_detail_activity);
Expand Down Expand Up @@ -310,7 +322,19 @@ public void onPlansFetched(OnPlansFetched event) {

@Override
public void onDomainRegistrationRequested() {
ActivityLauncher.viewDomainRegistrationActivity(this, mSite);
ActivityLauncher.viewDomainRegistrationActivityForResult(this, mSite,
DomainRegistrationPurpose.AUTOMATED_TRANSFER);
}

@Override
protected void onActivityResult(int requestCode, int resultCode, @Nullable Intent data) {
super.onActivityResult(requestCode, resultCode, data);
if (requestCode == RequestCodes.DOMAIN_REGISTRATION) {
if (resultCode != Activity.RESULT_OK || isFinishing()) {
return;
}
confirmInstallPluginForAutomatedTransfer();
}
}

public static class DomainRegistrationPromptDialog extends DialogFragment {
Expand Down Expand Up @@ -412,6 +436,7 @@ public void onSaveInstanceState(Bundle outState) {
outState.putBoolean(KEY_IS_SHOWING_AUTOMATED_TRANSFER_PROGRESS, mIsShowingAutomatedTransferProgress);
outState.putBoolean(KEY_IS_SHOWING_DOMAIN_CREDIT_CHECK_PROGRESS,
mCheckingDomainCreditsProgressDialog != null && mCheckingDomainCreditsProgressDialog.isShowing());
outState.putInt(KEY_PLUGIN_RECHECKED_TIMES, mPluginReCheckTimer);
}

// UI Helpers
Expand Down Expand Up @@ -1353,6 +1378,7 @@ private void automatedTransferCompleted() {
AnalyticsUtils.trackWithSiteDetails(Stat.AUTOMATED_TRANSFER_FLOW_COMPLETE, mSite);
cancelAutomatedTransferDialog();
refreshPluginFromStore();
dispatchConfigurePluginAction(true);
refreshViews();
showSuccessfulInstallSnackbar();
invalidateOptionsMenu();
Expand Down Expand Up @@ -1483,7 +1509,7 @@ public void run() {
// Wait 3 seconds before checking the status again
mDispatcher.dispatch(SiteActionBuilder.newCheckAutomatedTransferStatusAction(mSite));
}
}, 3000);
}, DEFAULT_RETRY_DELAY_MS);
}
}
}
Expand Down Expand Up @@ -1514,10 +1540,9 @@ public void onSiteChanged(OnSiteChanged event) {
// We try to fetch the site after Automated Transfer is completed so that we can fetch its plugins. If
// we are still showing the AT progress and the site is AT site, we can continue with plugins fetch
if (mSite.isAutomatedTransfer()) {
AppLog.v(T.PLUGINS, "Site is successfully fetched after Automated Transfer, fetching the site plugins "
+ "to complete the process...");
mDispatcher.dispatch(PluginActionBuilder.newFetchPluginDirectoryAction(new PluginStore
.FetchPluginDirectoryPayload(PluginDirectoryType.SITE, mSite, false)));
AppLog.v(T.PLUGINS, "Site is successfully fetched after Automated Transfer, fetching"
+ " the site plugins to complete the process...");
fetchPluginDirectory(0);
} else {
// Either an error occurred while fetching the site or Automated Transfer is not yet reflected in the
// API response. We need to keep fetching the site until we get the updated site. Otherwise, any changes
Expand All @@ -1532,7 +1557,7 @@ public void run() {
// Wait 3 seconds before fetching the site again
mDispatcher.dispatch(SiteActionBuilder.newFetchSiteAction(mSite));
}
}, 3000);
}, DEFAULT_RETRY_DELAY_MS);
}
}
}
Expand All @@ -1551,26 +1576,38 @@ public void onPluginDirectoryFetched(OnPluginDirectoryFetched event) {
if (isFinishing()) {
return;
}

refreshPluginFromStore();

if (event.isError()) {
if (mIsShowingAutomatedTransferProgress) {
AppLog.e(T.PLUGINS, "Fetching the plugin directory after Automated Transfer has failed with error type"
+ event.error.type + " and message: " + event.error.message);
// Although unlikely, fetching the plugins after a successful Automated Transfer can result in an error.
// This should hopefully be an edge case and fetching the plugins again should
mHandler.postDelayed(new Runnable() {
@Override
public void run() {
AppLog.v(T.PLUGINS, "Fetching the site plugins again after Automated Transfer since the"
+ " changes are not yet reflected");
// Wait 3 seconds before fetching the site plugins again
mDispatcher.dispatch(PluginActionBuilder.newFetchPluginDirectoryAction(new PluginStore
.FetchPluginDirectoryPayload(PluginDirectoryType.SITE, mSite, false)));
}
}, 3000);
AppLog.v(T.PLUGINS, "Fetching the site plugins again after Automated Transfer since the"
+ " changes are not yet reflected");
fetchPluginDirectory(PLUGIN_RETRY_DELAY_MS);
}
// We are safe to ignore the errors for this event unless it's for Automated Transfer since that's the only
// one triggered in this page and only one we care about.
return;
} else if (!mPlugin.isInstalled()) {
// it sometimes take a bit of time for plugin to get marked as installed, especially when
// Automated Transfer is performed right after domain registration
if (mIsShowingAutomatedTransferProgress) {
if (mPluginReCheckTimer < MAX_PLUGIN_CHECK_TRIES) {
AppLog.v(T.PLUGINS, "Targeted plugin is not marked as installed after Automated Transfer."
+ " Fetching the site plugins to reflect the changes.");
fetchPluginDirectory(PLUGIN_RETRY_DELAY_MS);
mPluginReCheckTimer++;
return;
} else {
// if plugin is still not marked as installed, we ask user to check back later, and proceed to
// finish Automated Transfer
ToastUtils.showToast(this, R.string.plugin_fetching_error_after_at, Duration.LONG);
}
}
}
if (event.type == PluginDirectoryType.SITE && mIsShowingAutomatedTransferProgress) {
// After Automated Transfer flow is completed, we fetch the site and then it's plugins. The only way site's
Expand All @@ -1580,11 +1617,20 @@ public void run() {
} else {
// Although it's unlikely that a directory might be fetched while we are in the plugin detail page, we
// should be safe to refresh the plugin and the view in case the plugin we are showing has changed
refreshPluginFromStore();
refreshViews();
}
}

private void fetchPluginDirectory(int delay) {
mHandler.postDelayed(new Runnable() {
@Override
public void run() {
mDispatcher.dispatch(PluginActionBuilder.newFetchPluginDirectoryAction(new PluginStore
.FetchPluginDirectoryPayload(PluginDirectoryType.SITE, mSite, false)));
}
}, delay);
}

private String getEligibilityErrorMessage(String errorCode) {
int errorMessageRes;
switch (errorCode) {
Expand Down
Loading