WordPress-Android icon indicating copy to clipboard operation
WordPress-Android copied to clipboard

Navigate to correct stats page for Jetpack sites using XML-RPC API

Open oguzkocer opened this issue 7 years ago • 10 comments
trafficstars

We currently have the following logic for when the stats button is tapped on the MySiteFragment:

SiteModel selectedSite = getSelectedSite();
if (selectedSite != null) {
    if (!mAccountStore.hasAccessToken() && selectedSite.isJetpackConnected()) {
        // If the user is not connected to WordPress.com, ask him to connect first.
        startWPComLoginForJetpackStats();
    } else if (selectedSite.isWPCom() || (selectedSite.isJetpackInstalled() && selectedSite
            .isJetpackConnected())) {
        ActivityLauncher.viewBlogStats(getActivity(), selectedSite);
    } else {
        ActivityLauncher.viewConnectJetpackForStats(getActivity(), selectedSite);
    }
}

I don't think that's entirely correct because it doesn't check which API a site is using. For example, I have a site that's connected to Jetpack on my main account. If I login to the app with my other account and add my Jetpack site as self-hosted site, tapping on the Stats button shows the actual Stats page and telling me that it couldn't be refreshed at this point.

I don't know what exactly we want to see happen, but I think we should be utilizing selectedSite.isUsingWpComRestApi() to check the API the site is using to make better informed decisions. If I am not missing anything, here are some different scenarios we might want to handle separately:

  1. WordPress.com site
  2. Self-hosted site with Jetpack uninstalled
  3. Self-hosted site with Jetpack installed, but not connected to any account
  4. Jetpack site connected to the logged in account
  5. Jetpack site, but the user is not logged into WordPress.com in the app (added as a self-hosted site and using xml-rpc api)
  6. Jetpack site, but the user is logged into a different WordPress.com (added as a self-hosted site and using xml-rpc api)

I believe we currently convert self-hosted sites to Jetpack sites when the user logs in and the information matches, but I might be wrong about that. If it's possible to have duplicate sites, one added as self-hosted and one added as Jetpack site, I think we should fix that first because that'd make it much harder to handle this issue.

@aerych Since NDE is working on Stats refresh, would this be something that NDE could look into? If not, I am happy to work on this, but I'd still need help deciding what exactly we want to see happen in each of these cases, could NDE help with that?

P.S: I am not adding Stats Refresh label to this, because it's just Stats related unless it's done as part of that project.

oguzkocer avatar Nov 15 '18 00:11 oguzkocer

This issue has been marked as stale because:

  • It has been inactive for the past year.
  • It isn't in a project or a milestone.
  • It hasn’t been labeled [Pri] Blocker, [Pri] High, or good first issue.

Please comment with an update if you believe this issue is still valid or if it can be closed. This issue will also be reviewed for validity and priority (cc @designsimply).

stale[bot] avatar Nov 20 '19 12:11 stale[bot]

Deleted comment spam

aerych avatar Nov 20 '19 16:11 aerych

@planarvoid Do you know if this is is still relevant or if its something we could close given the stats refresh?

aerych avatar Nov 20 '19 16:11 aerych

@aerych I think this is still relevant and it's not connected to the Stats refresh. It might be related to the Jetpack remote install but I think at least the main item is valid (even though it's an edge case).

planarvoid avatar Nov 21 '19 15:11 planarvoid

This issue has been marked as stale because:

  • It has been inactive for the past year.
  • It isn't in a project or a milestone.
  • It hasn’t been labeled [Pri] Blocker, [Pri] High, or good first issue.

Please comment with an update if you believe this issue is still valid or if it can be closed. This issue will also be reviewed for validity and priority during regularly scheduled triage sessions.

stale[bot] avatar Nov 21 '20 07:11 stale[bot]

Still valid. Noting we should circle back and test the scenarios Oguz mentioned so it's more clear to see what's happening.

designsimply avatar Dec 09 '20 18:12 designsimply

This issue has been marked as stale because:

  • It has been inactive for the past year.
  • It isn't in a project or a milestone.
  • It hasn’t been labeled [Pri] Blocker, [Pri] High, or good first issue.

Please comment with an update if you believe this issue is still valid or if it can be closed. This issue will also be reviewed for validity and priority during regularly scheduled triage sessions.

stale[bot] avatar Jan 09 '22 10:01 stale[bot]

Thanks for reporting! 👍

dangermattic avatar Mar 31 '24 14:03 dangermattic

@irfano need some more information here. Do stats not work on XML-RPC API? Is the json API required?

notandyvee avatar Apr 12 '24 21:04 notandyvee

Do stats not work on XML-RPC API

Yeah, this is the underlying issue. The XML-RPC API doesn't have stats endpoints. https://codex.wordpress.org/XML-RPC_WordPress_API

@SiobhyB's description in https://github.com/wordpress-mobile/WordPress-Android/issues/18485 is a good read for more context and options on what a better experience might be. :)

aerych avatar Apr 12 '24 21:04 aerych